Merge pull request #10939 from dependabot/dev/brettfo/nuget-analyze

only report a package as existing if the actual `.nupkg` can be downloaded

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Analyze/AnalyzeWorkerTests.cs

@@ -835,4 +835,121 @@ await TestAnalyzeAsync(
             }
         );
     }
+
+    [Fact]
+    public async Task AnalysisFailsWhenNewerPackageDownloadIsDenied()
+    {
+        static (int, byte[]) TestHttpHandler(string uriString)
+        {
+            var uri = new Uri(uriString, UriKind.Absolute);
+            var baseUrl = $"{uri.Scheme}://{uri.Host}:{uri.Port}";
+            return uri.PathAndQuery switch
+            {
+                // initial request is good
+                "/index.json" => (200, Encoding.UTF8.GetBytes($$"""
+                    {
+                        "version": "3.0.0",
+                        "resources": [
+                            {
+                                "@id": "{{baseUrl}}/download",
+                                "@type": "PackageBaseAddress/3.0.0"
+                            },
+                            {
+                                "@id": "{{baseUrl}}/query",
+                                "@type": "SearchQueryService"
+                            },
+                            {
+                                "@id": "{{baseUrl}}/registrations",
+                                "@type": "RegistrationsBaseUrl"
+                            }
+                        ]
+                    }
+                    """)),
+                // request for package index is good
+                "/registrations/some.package/index.json" => (200, Encoding.UTF8.GetBytes("""
+                        {
+                            "count": 1,
+                            "items": [
+                                {
+                                    "lower": "1.0.0",
+                                    "upper": "1.1.0",
+                                    "items": [
+                                        {
+                                            "catalogEntry": {
+                                                "listed": true,
+                                                "version": "1.0.0"
+                                            }
+                                        },
+                                        {
+                                            "catalogEntry": {
+                                                "listed": true,
+                                                "version": "1.1.0"
+                                            }
+                                        }
+                                    ]
+                                }
+                            ]
+                        }
+                        """)),
+                // request for versions is good
+                "/download/some.package/index.json" => (200, Encoding.UTF8.GetBytes("""
+                    {
+                        "versions": [
+                            "1.0.0",
+                            "1.1.0"
+                        ]
+                    }
+                    """)),
+                // download of old package is good
+                "/download/some.package/1.0.0/some.package.1.0.0.nupkg" => (200, MockNuGetPackage.CreateSimplePackage("Some.Package", "1.0.0", "net9.0").GetZipStream().ReadAllBytes()),
+                // download of new package is denied
+                "/download/some.package/1.1.0/some.package.1.1.0.nupkg" => (401, Array.Empty<byte>()),
+                // all other requests are not found
+                _ => (404, Encoding.UTF8.GetBytes("{}")),
+            };
+        }
+        using var http = TestHttpServer.CreateTestServer(TestHttpHandler);
+        await TestAnalyzeAsync(
+            extraFiles:
+            [
+                ("NuGet.Config", $"""
+                    <configuration>
+                      <packageSources>
+                        <clear />
+                        <add key="private_feed" value="{http.BaseUrl.TrimEnd('/')}/index.json" allowInsecureConnections="true" />
+                      </packageSources>
+                    </configuration>
+                    """)
+            ],
+            discovery: new()
+            {
+                Path = "/",
+                Projects = [
+                    new()
+                    {
+                        FilePath = "./project.csproj",
+                        TargetFrameworks = ["net9.0"],
+                        Dependencies = [
+                            new("Some.Package", "1.0.0", DependencyType.PackageReference),
+                        ],
+                    }
+                ]
+            },
+            dependencyInfo: new()
+            {
+                Name = "Some.Package",
+                Version = "1.0.0",
+                IgnoredVersions = [],
+                IsVulnerable = false,
+                Vulnerabilities = [],
+            },
+            expectedResult: new()
+            {
+                UpdatedVersion = "1.0.0",
+                CanUpdate = false,
+                VersionComesFromMultiDependencyProperty = false,
+                UpdatedDependencies = [],
+            }
+        );
+    }
 }

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/AnalyzeWorker.cs

@@ -50,7 +50,7 @@ internal async Task<AnalysisResult> RunWithErrorHandlingAsync(string repoRoot, s
         when (ex.StatusCode == HttpStatusCode.Unauthorized || ex.StatusCode == HttpStatusCode.Forbidden)
         {
             var localPath = PathHelper.JoinPath(repoRoot, discovery.Path);
-            var nugetContext = new NuGetContext(localPath);
+            using var nugetContext = new NuGetContext(localPath);
             analysisResult = new AnalysisResult
             {
                 ErrorType = ErrorType.AuthenticationFailure,

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/CompatabilityChecker.cs

@@ -24,11 +24,16 @@ public static async Task<bool> CheckAsync(
         ILogger logger,
         CancellationToken cancellationToken)
     {
-        var (isDevDependency, packageFrameworks) = await GetPackageInfoAsync(
+        var packageInfo = await GetPackageInfoAsync(
             package,
             nugetContext,
             cancellationToken);
+        if (packageInfo is null)
+        {
+            return false;
+        }
 
+        var (isDevDependency, packageFrameworks) = packageInfo.GetValueOrDefault();
         return PerformCheck(package, projectFrameworks, isDevDependency, packageFrameworks, logger);
     }
 
@@ -70,7 +75,7 @@ internal static bool PerformCheck(
         return true;
     }
 
-    internal static async Task<PackageInfo> GetPackageInfoAsync(
+    internal static async Task<PackageReaders?> GetPackageReadersAsync(
         PackageIdentity package,
         NuGetContext nugetContext,
         CancellationToken cancellationToken)
@@ -79,7 +84,21 @@ internal static async Task<PackageInfo> GetPackageInfoAsync(
         var readers = File.Exists(tempPackagePath)
             ? ReadPackage(tempPackagePath)
             : await DownloadPackageAsync(package, nugetContext, cancellationToken);
+        return readers;
+    }
+
+    internal static async Task<PackageInfo?> GetPackageInfoAsync(
+        PackageIdentity package,
+        NuGetContext nugetContext,
+        CancellationToken cancellationToken)
+    {
+        var readersOption = await GetPackageReadersAsync(package, nugetContext, cancellationToken);
+        if (readersOption is null)
+        {
+            return null;
+        }
 
+        var readers = readersOption.GetValueOrDefault();
         var nuspecStream = await readers.CoreReader.GetNuspecAsync(cancellationToken);
         var reader = new NuspecReader(nuspecStream);
 
@@ -127,7 +146,7 @@ internal static PackageReaders ReadPackage(string tempPackagePath)
         return (archiveReader, archiveReader);
     }
 
-    internal static async Task<PackageReaders> DownloadPackageAsync(
+    internal static async Task<PackageReaders?> DownloadPackageAsync(
         PackageIdentity package,
         NuGetContext context,
         CancellationToken cancellationToken)
@@ -179,13 +198,13 @@ internal static async Task<PackageReaders> DownloadPackageAsync(
             var isDownloaded = await downloader.CopyNupkgFileToAsync(tempPackagePath, cancellationToken);
             if (!isDownloaded)
             {
-                throw new Exception($"Failed to download package [{package.Id}/{package.Version}] from [${source.SourceUri}]");
+                continue;
             }
 
             return (downloader.CoreReader, downloader.ContentReader);
         }
 
-        throw new Exception($"Package [{package.Id}/{package.Version}] does not exist in any of the configured sources.");
+        return null;
     }
 
     internal static string GetTempPackagePath(PackageIdentity package, NuGetContext context)

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/VersionFinder.cs

@@ -151,46 +151,17 @@ public static async Task<bool> DoesVersionExistAsync(
         ILogger logger,
         CancellationToken cancellationToken)
     {
-        var includePrerelease = version.IsPrerelease;
-
-        var sourceMapping = PackageSourceMapping.GetPackageSourceMapping(nugetContext.Settings);
-        var packageSources = sourceMapping.GetConfiguredPackageSources(packageId).ToHashSet();
-        var sources = packageSources.Count == 0
-            ? nugetContext.PackageSources
-            : nugetContext.PackageSources
-                .Where(p => packageSources.Contains(p.Name))
-                .ToImmutableArray();
-
-        foreach (var source in sources)
+        // if it can be downloaded, it exists
+        var downloader = await CompatibilityChecker.DownloadPackageAsync(new PackageIdentity(packageId, version), nugetContext, cancellationToken);
+        var packageAndVersionExists = downloader is not null;
+        if (packageAndVersionExists)
         {
-            var sourceRepository = Repository.Factory.GetCoreV3(source);
-            var feed = await sourceRepository.GetResourceAsync<MetadataResource>();
-            if (feed is null)
-            {
-                logger.Log($"Failed to get MetadataResource for [{source.Source}]");
-                continue;
-            }
-
-            try
-            {
-                // a non-compliant v2 API returning 404 can cause this to throw
-                var existsInFeed = await feed.Exists(
-                    new PackageIdentity(packageId, version),
-                    includeUnlisted: false,
-                    nugetContext.SourceCacheContext,
-                    NullLogger.Instance,
-                    cancellationToken);
-                if (existsInFeed)
-                {
-                    return true;
-                }
-            }
-            catch (FatalProtocolException)
-            {
-                // if anything goes wrong here, the package source obviously doesn't contain the requested package
-            }
+            // release the handles
+            var readers = downloader.GetValueOrDefault();
+            (readers.CoreReader as IDisposable)?.Dispose();
+            (readers.ContentReader as IDisposable)?.Dispose();
         }
 
-        return false;
+        return packageAndVersionExists;
     }
 }