Skip to content

Commit

Permalink
Npm: Fix quering of package details from private / scoped registries
Browse files Browse the repository at this point in the history
Bump the NPM version requirement from 5.7.* to 6.* in order to be able
to use the `npm view` command to query details about a package from
remote registries. This lifts two severe restrictions:

- The original code was only able to query package details from the
  public NPM registry [1]. Private registries were not supported at all.

- Custom registries defined for different NPM scopes were not supported.

As a bonus, `npm view` seems to return more details than querying the
registry API directly even for the public NPM registry.

The downside is that performance is worse now, using `npm view` seems to
be roughly 4 times slower (on Linux); the command unfortunately does not
support batching. However, correctness should be more important than
performance here, and a future improvement could parallelize the
`parsePackage()` calls in general.

[1]: https://registry.npmjs.org

Signed-off-by: Sebastian Schuberth <sebastian.schuberth@bosch.io>
  • Loading branch information
sschuberth committed Mar 18, 2022
1 parent 73f9ca7 commit b38a882
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
70 changes: 14 additions & 56 deletions analyzer/src/main/kotlin/managers/Npm.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -71,17 +68,13 @@ 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("5.7.* - 7.20.*")
override fun getVersionRequirement() = Requirement.buildNPM("6.* - 7.20.*")
}

/**
Expand Down Expand Up @@ -196,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'." }
Expand Down Expand Up @@ -246,34 +239,18 @@ open class Npm(
|| hash == Hash.NONE || vcsFromPackage == VcsInfo.EMPTY

if (hasIncompleteData) {
// 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." }

OkHttpClientHelper.downloadText("$npmRegistry/$encodedName/$version").onSuccess {
val versionInfo = jsonMapper.readTree(it)
val process = NpmCli.run("view", "--json", "$rawName@$version")
val view = jsonMapper.readTree(process.stdoutFile)

if (description.isEmpty()) description = versionInfo["description"].textValueOrEmpty()
if (homepageUrl.isEmpty()) homepageUrl = versionInfo["homepage"].textValueOrEmpty()
if (description.isEmpty()) description = view["description"].textValueOrEmpty()
if (homepageUrl.isEmpty()) homepageUrl = view["homepage"].textValueOrEmpty()

versionInfo["dist"]?.let { dist ->
if (downloadUrl.isEmpty()) downloadUrl = dist["tarball"].textValueOrEmpty()
if (hash == Hash.NONE) 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}"
}
view["dist"]?.let { dist ->
if (downloadUrl.isEmpty()) downloadUrl = dist["tarball"].textValueOrEmpty()
if (hash == Hash.NONE) hash = Hash.create(dist["shasum"].textValueOrEmpty())
}

vcsFromPackage = parseVcsInfo(view)
}
}

Expand Down Expand Up @@ -323,23 +300,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.
Expand All @@ -356,10 +318,6 @@ open class Npm(
NpmCli.checkVersion()
}

override fun afterResolution(definitionFiles: List<File>) {
ortProxySelector.removeProxyOrigin(managerName)
}

override fun createPackageManagerResult(projectResults: Map<File, List<ProjectAnalyzerResult>>) =
PackageManagerResult(projectResults, graphBuilder.build(), graphBuilder.packages())

Expand Down Expand Up @@ -423,7 +381,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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,12 @@ class NpmDependencyHandlerTest : StringSpec({
id shouldBe Identifier("NPM", "", "bonjour", "3.5.0")
declaredLicenses should containExactly("MIT")
authors should containExactly("Thomas Watson Steen")
homepageUrl shouldBe "https://github.com/watson/bonjour/local"
description shouldBe "A Bonjour/Zeroconf implementation in pure JavaScript (local)"
homepageUrl shouldBe "https://github.com/watson/bonjour"
description shouldBe "A Bonjour/Zeroconf implementation in pure JavaScript"
}
}
})

/** The URI to be used for queries to the NPM registry. */
const val NPM_REGISTRY_URL = "https://npm-test.example.org"

/**
* Construct a test identifier with the given [name].
*/
Expand All @@ -100,4 +97,4 @@ private fun createModuleInfo(
/**
* Creates an [NpmDependencyHandler] instance to be used by test cases.
*/
private fun createHandler() = NpmDependencyHandler(NPM_REGISTRY_URL)
private fun createHandler() = NpmDependencyHandler()

0 comments on commit b38a882

Please sign in to comment.