Npm: Fix querying of package details from private / scoped registries

analyzer/src/funTest/assets/projects/external/git-repo-expected-output.yml

@@ -4288,7 +4288,7 @@ analyzer:
         declared_licenses_processed:
           spdx_expression: "MIT"
         description: "TypeScript definitions for Node.js"
-        homepage_url: ""
+        homepage_url: "https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/node"
         binary_artifact:
           url: ""
           hash:

analyzer/src/funTest/assets/projects/synthetic/npm-babel-expected-output.yml

@@ -5056,7 +5056,7 @@ packages:
   description: "Returns `true` if the given string looks like a glob pattern or an\
     \ extglob pattern. This makes it easy to create code that only uses external modules\
     \ like node-glob when necessary, resulting in much faster code execution and initialization\
-    \ time, and a bet"
+    \ time, and a better user experience."
   homepage_url: "https://github.com/jonschlinkert/is-glob"
   binary_artifact:
     url: ""

analyzer/src/funTest/assets/projects/synthetic/npm-expected-output.yml

@@ -1165,7 +1165,7 @@ packages:
   declared_licenses_processed:
     spdx_expression: "MIT"
   description: "TypeScript definitions for Node.js"
-  homepage_url: ""
+  homepage_url: "https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/node"
   binary_artifact:
     url: ""
     hash:

analyzer/src/funTest/assets/projects/synthetic/yarn-expected-output.yml

@@ -1134,7 +1134,7 @@ packages:
   declared_licenses_processed:
     spdx_expression: "MIT"
   description: "TypeScript definitions for Node.js"
-  homepage_url: ""
+  homepage_url: "https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/node"
   binary_artifact:
     url: ""
     hash:

analyzer/src/main/kotlin/managers/Npm.kt

@@ -28,7 +28,6 @@ import com.vdurmont.semver4j.Requirement
 
 import java.io.File
 import java.io.IOException
-import java.net.URLEncoder
 import java.util.SortedSet
 
 import org.ossreviewtoolkit.analyzer.AbstractPackageManagerFactory
@@ -39,8 +38,6 @@ import org.ossreviewtoolkit.analyzer.managers.utils.NpmModuleInfo
 import org.ossreviewtoolkit.analyzer.managers.utils.expandNpmShortcutUrl
 import org.ossreviewtoolkit.analyzer.managers.utils.hasNpmLockFile
 import org.ossreviewtoolkit.analyzer.managers.utils.mapDefinitionFilesForNpm
-import org.ossreviewtoolkit.analyzer.managers.utils.readProxySettingsFromNpmRc
-import org.ossreviewtoolkit.analyzer.managers.utils.readRegistryFromNpmRc
 import org.ossreviewtoolkit.analyzer.parseAuthorString
 import org.ossreviewtoolkit.downloader.VcsHost
 import org.ossreviewtoolkit.downloader.VersionControlSystem
@@ -71,12 +68,14 @@ import org.ossreviewtoolkit.utils.common.realFile
 import org.ossreviewtoolkit.utils.common.stashDirectories
 import org.ossreviewtoolkit.utils.common.textValueOrEmpty
 import org.ossreviewtoolkit.utils.common.withoutPrefix
-import org.ossreviewtoolkit.utils.core.OkHttpClientHelper
-import org.ossreviewtoolkit.utils.core.installAuthenticatorAndProxySelector
 import org.ossreviewtoolkit.utils.core.log
 import org.ossreviewtoolkit.utils.spdx.SpdxConstants
 
-const val PUBLIC_NPM_REGISTRY = "https://registry.npmjs.org"
+object NpmCli : CommandLineTool {
+    override fun command(workingDir: File?) = if (Os.isWindows) "npm.cmd" else "npm"
+
+    override fun getVersionRequirement() = Requirement.buildNPM("6.* - 7.20.*")
+}
 
 /**
  * The [Node package manager](https://www.npmjs.com/) for JavaScript.
@@ -86,7 +85,7 @@ open class Npm(
     analysisRoot: File,
     analyzerConfig: AnalyzerConfiguration,
     repoConfig: RepositoryConfiguration
-) : PackageManager(name, analysisRoot, analyzerConfig, repoConfig), CommandLineTool {
+) : PackageManager(name, analysisRoot, analyzerConfig, repoConfig) {
     class Factory : AbstractPackageManagerFactory<Npm>("NPM") {
         override val globsForDefinitionFiles = listOf("package.json")
 
@@ -190,10 +189,10 @@ open class Npm(
 
         /**
          * Construct a [Package] by parsing its _package.json_ file and - if applicable - querying additional
-         * content from the [npmRegistry]. Result is a [Pair] with the raw identifier and the new package.
+         * content via the `npm view` command. The result is a [Pair] with the raw identifier and the new package.
          */
         @Suppress("HttpUrlsUsage")
-        internal fun parsePackage(packageFile: File, npmRegistry: String): Pair<String, Package> {
+        internal fun parsePackage(packageFile: File): Pair<String, Package> {
             val packageDir = packageFile.parentFile
 
             log.debug { "Found a 'package.json' file in '$packageDir'." }
@@ -236,40 +235,32 @@ open class Npm(
                 val vcsFromDirectory = VersionControlSystem.forDirectory(realPackageDir)?.getInfo().orEmpty()
                 vcsFromPackage = vcsFromPackage.merge(vcsFromDirectory)
             } else {
-                // Download package info from registry.npmjs.org.
-                // TODO: check if unpkg.com can be used as a fallback in case npmjs.org is down.
-                val encodedName = if (rawName.startsWith("@")) {
-                    "@${URLEncoder.encode(rawName.substringAfter('@'), "UTF-8")}"
-                } else {
-                    rawName
-                }
-
-                log.debug { "Resolving the package info for '${id.toCoordinates()}' via NPM registry." }
+                val hasIncompleteData = description.isEmpty() || homepageUrl.isEmpty() || downloadUrl.isEmpty()
+                        || hash == Hash.NONE || vcsFromPackage == VcsInfo.EMPTY
 
-                OkHttpClientHelper.downloadText("$npmRegistry/$encodedName/$version").onSuccess {
-                    val versionInfo = jsonMapper.readTree(it)
+                if (hasIncompleteData) {
+                    val process = NpmCli.run("view", "--json", "$rawName@$version")
+                    val view = jsonMapper.readTree(process.stdoutFile)
 
-                    description = versionInfo["description"].textValueOrEmpty()
-                    homepageUrl = versionInfo["homepage"].textValueOrEmpty()
+                    if (description.isEmpty()) description = view["description"].textValueOrEmpty()
+                    if (homepageUrl.isEmpty()) homepageUrl = view["homepage"].textValueOrEmpty()
 
-                    versionInfo["dist"]?.let { dist ->
-                        downloadUrl = dist["tarball"].textValueOrEmpty()
-                            // Work around the issue described at
-                            // https://npm.community/t/some-packages-have-dist-tarball-as-http-and-not-https/285/19.
-                            .replace("http://registry.npmjs.org/", "https://registry.npmjs.org/")
-
-                        hash = Hash.create(dist["shasum"].textValueOrEmpty())
+                    view["dist"]?.let { dist ->
+                        if (downloadUrl.isEmpty() || hash == Hash.NONE) {
+                            downloadUrl = dist["tarball"].textValueOrEmpty()
+                            hash = Hash.create(dist["shasum"].textValueOrEmpty())
+                        }
                     }
 
-                    vcsFromPackage = parseVcsInfo(versionInfo)
-                }.onFailure {
-                    log.info {
-                        "Could not retrieve package information for '$encodedName' from NPM registry $npmRegistry: " +
-                                it.message
-                    }
+                    vcsFromPackage = parseVcsInfo(view)
                 }
             }
 
+            downloadUrl = downloadUrl
+                // Work around the issue described at
+                // https://npm.community/t/some-packages-have-dist-tarball-as-http-and-not-https/285/19.
+                .replace("http://registry.npmjs.org/", "https://registry.npmjs.org/")
+
             val vcsFromDownloadUrl = VcsHost.toVcsInfo(downloadUrl)
             if (vcsFromDownloadUrl.url != downloadUrl) {
                 vcsFromPackage = vcsFromPackage.merge(vcsFromDownloadUrl)
@@ -311,23 +302,8 @@ open class Npm(
         }
     }
 
-    private val ortProxySelector = installAuthenticatorAndProxySelector()
-
-    private val npmRegistry: String
-
-    init {
-        val npmRcFile = Os.userHomeDirectory.resolve(".npmrc")
-        npmRegistry = if (npmRcFile.isFile) {
-            val npmRcContent = npmRcFile.readText()
-            ortProxySelector.addProxies(managerName, readProxySettingsFromNpmRc(npmRcContent))
-            readRegistryFromNpmRc(npmRcContent) ?: PUBLIC_NPM_REGISTRY
-        } else {
-            PUBLIC_NPM_REGISTRY
-        }
-    }
-
     private val graphBuilder: DependencyGraphBuilder<NpmModuleInfo> =
-        DependencyGraphBuilder(NpmDependencyHandler(npmRegistry))
+        DependencyGraphBuilder(NpmDependencyHandler())
 
     /**
      * Array of parameters passed to the install command when installing dependencies.
@@ -336,20 +312,12 @@ open class Npm(
 
     protected open fun hasLockFile(projectDir: File) = hasNpmLockFile(projectDir)
 
-    override fun command(workingDir: File?) = if (Os.isWindows) "npm.cmd" else "npm"
-
-    override fun getVersionRequirement(): Requirement = Requirement.buildNPM("5.7.* - 7.20.*")
-
     override fun mapDefinitionFiles(definitionFiles: List<File>) = mapDefinitionFilesForNpm(definitionFiles).toList()
 
     override fun beforeResolution(definitionFiles: List<File>) {
         // We do not actually depend on any features specific to an NPM version, but we still want to stick to a
         // fixed minor version to be sure to get consistent results.
-        checkVersion()
-    }
-
-    override fun afterResolution(definitionFiles: List<File>) {
-        ortProxySelector.removeProxyOrigin(managerName)
+        NpmCli.checkVersion()
     }
 
     override fun createPackageManagerResult(projectResults: Map<File, List<ProjectAnalyzerResult>>) =
@@ -415,7 +383,7 @@ open class Npm(
         nodeModulesDir.walk().filter {
             it.name == "package.json" && isValidNodeModulesDirectory(nodeModulesDir, nodeModulesDirForPackageJson(it))
         }.forEach { file ->
-            val (id, pkg) = parsePackage(file, npmRegistry)
+            val (id, pkg) = parsePackage(file)
             packages[id] = pkg
         }
 
@@ -659,9 +627,9 @@ open class Npm(
 
         // Install all NPM dependencies to enable NPM to list dependencies.
         val process = if (hasLockFile(workingDir) && this::class.java == Npm::class.java) {
-            run(workingDir, "ci", *installParameters)
+            NpmCli.run(workingDir, "ci", *installParameters)
         } else {
-            run(workingDir, "install", *installParameters)
+            NpmCli.run(workingDir, "install", *installParameters)
         }
 
         // TODO: Capture warnings from npm output, e.g. "Unsupported platform" which happens for fsevents on all

analyzer/src/main/kotlin/managers/Yarn.kt

@@ -28,8 +28,15 @@ import org.ossreviewtoolkit.analyzer.managers.utils.hasYarnLockFile
 import org.ossreviewtoolkit.analyzer.managers.utils.mapDefinitionFilesForYarn
 import org.ossreviewtoolkit.model.config.AnalyzerConfiguration
 import org.ossreviewtoolkit.model.config.RepositoryConfiguration
+import org.ossreviewtoolkit.utils.common.CommandLineTool
 import org.ossreviewtoolkit.utils.common.Os
 
+object YarnCli : CommandLineTool {
+    override fun command(workingDir: File?) = if (Os.isWindows) "yarn.cmd" else "yarn"
+
+    override fun getVersionRequirement() = Requirement.buildNPM("1.3.* - 1.22.*")
+}
+
 /**
  * The [Yarn](https://classic.yarnpkg.com/) package manager for JavaScript.
  */
@@ -53,14 +60,10 @@ class Yarn(
 
     override fun hasLockFile(projectDir: File) = hasYarnLockFile(projectDir)
 
-    override fun command(workingDir: File?) = if (Os.isWindows) "yarn.cmd" else "yarn"
-
-    override fun getVersionRequirement(): Requirement = Requirement.buildNPM("1.3.* - 1.22.*")
-
     override fun mapDefinitionFiles(definitionFiles: List<File>) = mapDefinitionFilesForYarn(definitionFiles).toList()
 
     override fun beforeResolution(definitionFiles: List<File>) =
         // We do not actually depend on any features specific to a Yarn version, but we still want to stick to a
         // fixed minor version to be sure to get consistent results.
-        checkVersion()
+        YarnCli.checkVersion()
 }

analyzer/src/main/kotlin/managers/utils/NodeSupport.kt

@@ -31,9 +31,6 @@ import java.nio.file.PathMatcher
 import org.ossreviewtoolkit.model.readJsonFile
 import org.ossreviewtoolkit.utils.common.collectMessagesAsString
 import org.ossreviewtoolkit.utils.common.toUri
-import org.ossreviewtoolkit.utils.core.AuthenticatedProxy
-import org.ossreviewtoolkit.utils.core.ProtocolProxyMap
-import org.ossreviewtoolkit.utils.core.determineProxyFromURL
 import org.ossreviewtoolkit.utils.core.log
 import org.ossreviewtoolkit.utils.core.showStackTrace
 
@@ -112,46 +109,6 @@ fun expandNpmShortcutUrl(url: String): String {
     }
 }
 
-/**
- * Return all proxies defined in the provided [NPM configuration][npmRc].
- */
-fun readProxySettingsFromNpmRc(npmRc: String): ProtocolProxyMap {
-    val map = mutableMapOf<String, MutableList<AuthenticatedProxy>>()
-
-    npmRc.lines().forEach { line ->
-        val keyAndValue = line.split('=', limit = 2).map { it.trim() }
-        if (keyAndValue.size != 2) return@forEach
-
-        val (key, value) = keyAndValue
-        when (key) {
-            "proxy" -> determineProxyFromURL(value)?.let {
-                map.getOrPut("http") { mutableListOf() } += it
-            }
-
-            "https-proxy" -> determineProxyFromURL(value)?.let {
-                map.getOrPut("https") { mutableListOf() } += it
-            }
-        }
-    }
-
-    return map
-}
-
-/**
- * Return the npm registry defined in the provided [NPM configuration][npmRc] or null.
- */
-fun readRegistryFromNpmRc(npmRc: String): String? {
-    npmRc.lines().forEach { line ->
-        val keyAndValue = line.split('=', limit = 2).map { it.trim() }
-        if (keyAndValue.size != 2) return@forEach
-
-        val (key, value) = keyAndValue
-        if (key == "registry") return value
-    }
-
-    return null
-}
-
 private val NPM_LOCK_FILES = listOf("npm-shrinkwrap.json", "package-lock.json")
 private val YARN_LOCK_FILES = listOf("yarn.lock")
 

analyzer/src/main/kotlin/managers/utils/NpmDependencyHandler.kt

@@ -49,16 +49,13 @@ data class NpmModuleInfo(
 /**
  * A specialized [DependencyHandler] implementation for NPM.
  */
-class NpmDependencyHandler(
-    /** The URL for queries to the NPM registry. */
-    private val npmRegistryUrl: String
-) : DependencyHandler<NpmModuleInfo> {
+class NpmDependencyHandler : DependencyHandler<NpmModuleInfo> {
     override fun identifierFor(dependency: NpmModuleInfo): Identifier = dependency.id
 
     override fun dependenciesFor(dependency: NpmModuleInfo): Collection<NpmModuleInfo> = dependency.dependencies
 
     override fun linkageFor(dependency: NpmModuleInfo): PackageLinkage = PackageLinkage.DYNAMIC
 
     override fun createPackage(dependency: NpmModuleInfo, issues: MutableList<OrtIssue>): Package =
-        Npm.parsePackage(dependency.packageFile, npmRegistryUrl).second
+        Npm.parsePackage(dependency.packageFile).second
 }