diff --git a/README.md b/README.md index 4c3f9bc..c0de3f4 100644 --- a/README.md +++ b/README.md @@ -8,12 +8,12 @@ Parses package name and specifier passed to commands like `npm install` or ## EXAMPLES ```javascript -var assert = require("assert") -var npa = require("npm-package-arg") +const assert = require("assert") +const npa = require("npm-package-arg") // Pass in the descriptor, and it'll return an object try { - var parsed = npa("@bar/foo@1.2") + const parsed = npa("@bar/foo@1.2") } catch (ex) { … } @@ -21,9 +21,9 @@ try { ## USING -`var npa = require('npm-package-arg')` +`const npa = require('npm-package-arg')` -### var result = npa(*arg*[, *where*]) +### const result = npa(*arg*[, *where*]) * *arg* - a string that you might pass to `npm install`, like: `foo@1.2`, `@bar/foo@1.2`, `foo@user/foo`, `http://x.com/foo.tgz`, @@ -34,7 +34,7 @@ part, eg `foo` then the specifier will default to `latest`. **Throws** if the package name is invalid, a dist-tag is invalid or a URL's protocol is not supported. -### var result = npa.resolve(*name*, *spec*[, *where*]) +### const result = npa.resolve(*name*, *spec*[, *where*]) * *name* - The name of the module you want to install. For example: `foo` or `@bar/foo`. * *spec* - The specifier indicating where and how you can get this module. Something like: @@ -45,7 +45,7 @@ included then the default is `latest`. **Throws** if the package name is invalid, a dist-tag is invalid or a URL's protocol is not supported. -### var purl = npa.toPurl(*arg*, *reg*) +### const purl = npa.toPurl(*arg*, *reg*) Returns the [purl (package URL)](https://github.com/package-url/purl-spec) form of the given package name/spec. @@ -79,9 +79,9 @@ keys: specification. Mostly used when making requests against a registry. When `name` is `null`, `escapedName` will also be `null`. * `rawSpec` - The specifier part that was parsed out in calls to `npa(arg)`, - or the value of `spec` in calls to `npa.resolve(name, spec). + or the value of `spec` in calls to `npa.resolve(name, spec)`. * `saveSpec` - The normalized specifier, for saving to package.json files. - `null` for registry dependencies. + `null` for registry dependencies. See note below about how this is (not) encoded. * `fetchSpec` - The version of the specifier to be used to fetch this resource. `null` for shortcuts to hosted git dependencies as there isn't just one URL to try with them. @@ -94,3 +94,11 @@ keys: `npa.resolve(name, spec)` then this will be `name + '@' + spec`. * `subSpec` - If `type === 'alias'`, this is a Result Object for parsing the target specifier for the alias. + +## SAVE SPECS + +TLDR: `file:` urls are NOT uri encoded. + +Historically, npm would uri decode file package args, but did not do any uri encoding for the `saveSpec`. This meant that it generated incorrect saveSpecs for directories with characters that *looked* like encoded uri characters, and also that it could not parse directories with some unencoded uri characters (such as `%`). + +In order to fix this, and to not break all existing versions of npm, this module now parses all file package args as not being uri encoded. And in order to not break all of the package.json files npm has made in the past, it also does not uri encode the saveSpec. This includes package args that start with `file:`. This does mean that npm `file:` package args are not RFC compliant, and making them so constitutes quite a breaking change. diff --git a/lib/npa.js b/lib/npa.js index e926058..d409b7f 100644 --- a/lib/npa.js +++ b/lib/npa.js @@ -1,23 +1,24 @@ 'use strict' -module.exports = npa -module.exports.resolve = resolve -module.exports.toPurl = toPurl -module.exports.Result = Result -const { URL } = require('url') +const isWindows = process.platform === 'win32' + +const { URL } = require('node:url') +// We need to use path/win32 so that we get consistent results in tests, but this also means we need to manually convert backslashes to forward slashes when generating file: urls with paths. +const path = isWindows ? require('node:path/win32') : require('node:path') +const { homedir } = require('node:os') const HostedGit = require('hosted-git-info') const semver = require('semver') -const path = global.FAKE_WINDOWS ? require('path').win32 : require('path') const validatePackageName = require('validate-npm-package-name') -const { homedir } = require('os') const { log } = require('proc-log') -const isWindows = process.platform === 'win32' || global.FAKE_WINDOWS const hasSlashes = isWindows ? /\\|[/]/ : /[/]/ const isURL = /^(?:git[+])?[a-z]+:/i const isGit = /^[^@]+@[^:.]+\.[^:]+:.+$/i -const isFilename = /[.](?:tgz|tar.gz|tar)$/i +const isFileType = /[.](?:tgz|tar.gz|tar)$/i const isPortNumber = /:[0-9]+(\/|$)/i +const isWindowsFile = /^(?:[.]|~[/]|[/\\]|[a-zA-Z]:)/ +const isPosixFile = /^(?:[.]|~[/]|[/]|[a-zA-Z]:)/ +const defaultRegistry = 'https://registry.npmjs.org' function npa (arg, where) { let name @@ -31,13 +32,14 @@ function npa (arg, where) { return npa(arg.raw, where || arg.where) } } - const nameEndsAt = arg[0] === '@' ? arg.slice(1).indexOf('@') + 1 : arg.indexOf('@') + const nameEndsAt = arg.indexOf('@', 1) // Skip possible leading @ const namePart = nameEndsAt > 0 ? arg.slice(0, nameEndsAt) : arg if (isURL.test(arg)) { spec = arg } else if (isGit.test(arg)) { spec = `git+ssh://${arg}` - } else if (namePart[0] !== '@' && (hasSlashes.test(namePart) || isFilename.test(namePart))) { + // eslint-disable-next-line max-len + } else if (!namePart.startsWith('@') && (hasSlashes.test(namePart) || isFileType.test(namePart))) { spec = arg } else if (nameEndsAt > 0) { name = namePart @@ -54,7 +56,27 @@ function npa (arg, where) { return resolve(name, spec, where, arg) } -const isFilespec = isWindows ? /^(?:[.]|~[/]|[/\\]|[a-zA-Z]:)/ : /^(?:[.]|~[/]|[/]|[a-zA-Z]:)/ +function isFileSpec (spec) { + if (!spec) { + return false + } + if (spec.toLowerCase().startsWith('file:')) { + return true + } + if (isWindows) { + return isWindowsFile.test(spec) + } + // We never hit this in windows tests, obviously + /* istanbul ignore next */ + return isPosixFile.test(spec) +} + +function isAliasSpec (spec) { + if (!spec) { + return false + } + return spec.toLowerCase().startsWith('npm:') +} function resolve (name, spec, where, arg) { const res = new Result({ @@ -65,12 +87,16 @@ function resolve (name, spec, where, arg) { }) if (name) { - res.setName(name) + res.name = name } - if (spec && (isFilespec.test(spec) || /^file:/i.test(spec))) { + if (!where) { + where = process.cwd() + } + + if (isFileSpec(spec)) { return fromFile(res, where) - } else if (spec && /^npm:/i.test(spec)) { + } else if (isAliasSpec(spec)) { return fromAlias(res, where) } @@ -82,15 +108,13 @@ function resolve (name, spec, where, arg) { return fromHostedGit(res, hosted) } else if (spec && isURL.test(spec)) { return fromURL(res) - } else if (spec && (hasSlashes.test(spec) || isFilename.test(spec))) { + } else if (spec && (hasSlashes.test(spec) || isFileType.test(spec))) { return fromFile(res, where) } else { return fromRegistry(res) } } -const defaultRegistry = 'https://registry.npmjs.org' - function toPurl (arg, reg = defaultRegistry) { const res = npa(arg) @@ -128,60 +152,62 @@ function invalidPurlType (type, raw) { return err } -function Result (opts) { - this.type = opts.type - this.registry = opts.registry - this.where = opts.where - if (opts.raw == null) { - this.raw = opts.name ? opts.name + '@' + opts.rawSpec : opts.rawSpec - } else { - this.raw = opts.raw +class Result { + constructor (opts) { + this.type = opts.type + this.registry = opts.registry + this.where = opts.where + if (opts.raw == null) { + this.raw = opts.name ? `${opts.name}@${opts.rawSpec}` : opts.rawSpec + } else { + this.raw = opts.raw + } + this.name = undefined + this.escapedName = undefined + this.scope = undefined + this.rawSpec = opts.rawSpec || '' + this.saveSpec = opts.saveSpec + this.fetchSpec = opts.fetchSpec + if (opts.name) { + this.setName(opts.name) + } + this.gitRange = opts.gitRange + this.gitCommittish = opts.gitCommittish + this.gitSubdir = opts.gitSubdir + this.hosted = opts.hosted } - this.name = undefined - this.escapedName = undefined - this.scope = undefined - this.rawSpec = opts.rawSpec || '' - this.saveSpec = opts.saveSpec - this.fetchSpec = opts.fetchSpec - if (opts.name) { - this.setName(opts.name) - } - this.gitRange = opts.gitRange - this.gitCommittish = opts.gitCommittish - this.gitSubdir = opts.gitSubdir - this.hosted = opts.hosted -} + // TODO move this to a getter/setter in a semver major + setName (name) { + const valid = validatePackageName(name) + if (!valid.validForOldPackages) { + throw invalidPackageName(name, valid, this.raw) + } -Result.prototype.setName = function (name) { - const valid = validatePackageName(name) - if (!valid.validForOldPackages) { - throw invalidPackageName(name, valid, this.raw) + this.name = name + this.scope = name[0] === '@' ? name.slice(0, name.indexOf('/')) : undefined + // scoped packages in couch must have slash url-encoded, e.g. @foo%2Fbar + this.escapedName = name.replace('/', '%2f') + return this } - this.name = name - this.scope = name[0] === '@' ? name.slice(0, name.indexOf('/')) : undefined - // scoped packages in couch must have slash url-encoded, e.g. @foo%2Fbar - this.escapedName = name.replace('/', '%2f') - return this -} - -Result.prototype.toString = function () { - const full = [] - if (this.name != null && this.name !== '') { - full.push(this.name) - } - const spec = this.saveSpec || this.fetchSpec || this.rawSpec - if (spec != null && spec !== '') { - full.push(spec) + toString () { + const full = [] + if (this.name != null && this.name !== '') { + full.push(this.name) + } + const spec = this.saveSpec || this.fetchSpec || this.rawSpec + if (spec != null && spec !== '') { + full.push(spec) + } + return full.length ? full.join('@') : this.raw } - return full.length ? full.join('@') : this.raw -} -Result.prototype.toJSON = function () { - const result = Object.assign({}, this) - delete result.hosted - return result + toJSON () { + const result = Object.assign({}, this) + delete result.hosted + return result + } } // sets res.gitCommittish, res.gitRange, and res.gitSubdir @@ -228,25 +254,67 @@ function setGitAttrs (res, committish) { } } -function fromFile (res, where) { - if (!where) { - where = process.cwd() +// Taken from: EncodePathChars and lookup_table in src/node_url.cc +// url.pathToFileURL only returns absolute references. We can't use it to encode paths. +// encodeURI mangles windows paths. We can't use it to encode paths. +// Under the hood, url.pathToFileURL does a limited set of encoding, with an extra windows step, and then calls path.resolve. +// The encoding node does without path.resolve is not available outside of the source, so we are recreating it here. +const encodedPathChars = new Map([ + ['\0', '%00'], + ['\t', '%09'], + ['\n', '%0A'], + ['\r', '%0D'], + [' ', '%20'], + ['"', '%22'], + ['#', '%23'], + ['%', '%25'], + ['?', '%3F'], + ['[', '%5B'], + ['\\', isWindows ? '/' : '%5C'], + [']', '%5D'], + ['^', '%5E'], + ['|', '%7C'], + ['~', '%7E'], +]) + +function pathToFileURL (str) { + let result = '' + for (let i = 0; i < str.length; i++) { + result = `${result}${encodedPathChars.get(str[i]) ?? str[i]}` + } + if (result.startsWith('file:')) { + return result } - res.type = isFilename.test(res.rawSpec) ? 'file' : 'directory' + return `file:${result}` +} + +function fromFile (res, where) { + res.type = isFileType.test(res.rawSpec) ? 'file' : 'directory' res.where = where - // always put the '/' on where when resolving urls, or else - // file:foo from /path/to/bar goes to /path/to/foo, when we want - // it to be /path/to/bar/foo + let rawSpec = pathToFileURL(res.rawSpec) + + if (rawSpec.startsWith('file:/')) { + // XXX backwards compatibility lack of compliance with RFC 8089 + + // turn file://path into file:/path + if (/^file:\/\/[^/]/.test(rawSpec)) { + rawSpec = `file:/${rawSpec.slice(5)}` + } + + // turn file:/../path into file:../path + // for 1 or 3 leading slashes (2 is already ruled out from handling file:// explicitly above) + if (/^\/{1,3}\.\.?(\/|$)/.test(rawSpec.slice(5))) { + rawSpec = rawSpec.replace(/^file:\/{1,3}/, 'file:') + } + } - let specUrl let resolvedUrl - const prefix = (!/^file:/.test(res.rawSpec) ? 'file:' : '') - const rawWithPrefix = prefix + res.rawSpec - let rawNoPrefix = rawWithPrefix.replace(/^file:/, '') + let specUrl try { - resolvedUrl = new URL(rawWithPrefix, `file://${path.resolve(where)}/`) - specUrl = new URL(rawWithPrefix) + // always put the '/' on "where", or else file:foo from /path/to/bar goes to /path/to/foo, when we want it to be /path/to/bar/foo + resolvedUrl = new URL(rawSpec, `${pathToFileURL(path.resolve(where))}/`) + specUrl = new URL(rawSpec) } catch (originalError) { const er = new Error('Invalid file: URL, must comply with RFC 8089') throw Object.assign(er, { @@ -257,24 +325,6 @@ function fromFile (res, where) { }) } - // XXX backwards compatibility lack of compliance with RFC 8089 - if (resolvedUrl.host && resolvedUrl.host !== 'localhost') { - const rawSpec = res.rawSpec.replace(/^file:\/\//, 'file:///') - resolvedUrl = new URL(rawSpec, `file://${path.resolve(where)}/`) - specUrl = new URL(rawSpec) - rawNoPrefix = rawSpec.replace(/^file:/, '') - } - // turn file:/../foo into file:../foo - // for 1, 2 or 3 leading slashes since we attempted - // in the previous step to make it a file protocol url with a leading slash - if (/^\/{1,3}\.\.?(\/|$)/.test(rawNoPrefix)) { - const rawSpec = res.rawSpec.replace(/^file:\/{1,3}/, 'file:') - resolvedUrl = new URL(rawSpec, `file://${path.resolve(where)}/`) - specUrl = new URL(rawSpec) - rawNoPrefix = rawSpec.replace(/^file:/, '') - } - // XXX end RFC 8089 violation backwards compatibility section - // turn /C:/blah into just C:/blah on windows let specPath = decodeURIComponent(specUrl.pathname) let resolvedPath = decodeURIComponent(resolvedUrl.pathname) @@ -288,13 +338,21 @@ function fromFile (res, where) { if (/^\/~(\/|$)/.test(specPath)) { res.saveSpec = `file:${specPath.substr(1)}` resolvedPath = path.resolve(homedir(), specPath.substr(3)) - } else if (!path.isAbsolute(rawNoPrefix)) { + } else if (!path.isAbsolute(rawSpec.slice(5))) { res.saveSpec = `file:${path.relative(where, resolvedPath)}` } else { res.saveSpec = `file:${path.resolve(resolvedPath)}` } res.fetchSpec = path.resolve(where, resolvedPath) + // re-normalize the slashes in saveSpec due to node:path/win32 behavior in windows + res.saveSpec = res.saveSpec.split('\\').join('/') + // Ignoring because this only happens in windows + /* istanbul ignore next */ + if (res.saveSpec.startsWith('file://')) { + // normalization of \\win32\root paths can cause a double / which we don't want + res.saveSpec = `file:/${res.saveSpec.slice(7)}` + } return res } @@ -416,3 +474,8 @@ function fromRegistry (res) { } return res } + +module.exports = npa +module.exports.resolve = resolve +module.exports.toPurl = toPurl +module.exports.Result = Result diff --git a/test/basic.js b/test/basic.js index 9cabdb4..6eb9737 100644 --- a/test/basic.js +++ b/test/basic.js @@ -1,23 +1,19 @@ -const path = require('path').posix -const os = require('os') +const path = require('node:path').posix +const os = require('node:os') const normalizePath = p => p && p.replace(/^[a-zA-Z]:/, '').replace(/\\/g, '/') const cwd = normalizePath(process.cwd()) process.cwd = () => cwd const normalizePaths = spec => { - spec.saveSpec = normalizePath(spec.saveSpec) spec.fetchSpec = normalizePath(spec.fetchSpec) return spec } const t = require('tap') const npa = t.mock('..', { path }) -t.on('bailout', () => process.exit(1)) t.test('basic', function (t) { - t.setMaxListeners(999) - const tests = { 'foo@1.2': { name: 'foo', @@ -635,10 +631,13 @@ t.test('basic', function (t) { } Object.keys(tests).forEach(function (arg) { - const res = normalizePaths(npa(arg, '/test/a/b')) - t.ok(res instanceof npa.Result, arg + ' is a result') - Object.keys(tests[arg]).forEach(function (key) { - t.match(res[key], tests[arg][key], arg + ' [' + key + ']') + t.test(arg, t => { + const res = normalizePaths(npa(arg, '/test/a/b')) + t.ok(res instanceof npa.Result, arg + ' is a result') + Object.keys(tests[arg]).forEach(function (key) { + t.match(res[key], tests[arg][key], arg + ' [' + key + ']') + }) + t.end() }) }) @@ -735,9 +734,49 @@ t.test('basic', function (t) { t.end() }) +t.test('directory with non URI compatible components', t => { + t.has(normalizePaths(npa('/test%dir')), { + type: 'directory', + name: null, + rawSpec: '/test%dir', + fetchSpec: '/test%dir', + saveSpec: 'file:/test%dir', + }) + t.end() +}) + +t.test('file: spec with non URI compatible components', t => { + t.has(normalizePaths(npa('file:/test%dir')), { + type: 'directory', + name: null, + rawSpec: 'file:/test%dir', + fetchSpec: '/test%dir', + saveSpec: 'file:/test%dir', + }) + t.end() +}) + +t.test('directory cwd has non URI compatible components', t => { + // eslint-disable-next-line max-len + const where = '/tmp/ !"$%&\'()*+,-.0123456789:;<=>@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~' + const originalCwd = process.cwd + t.teardown(() => { + process.cwd = originalCwd + }) + process.cwd = () => where + t.has(normalizePaths(npa('./')), { + type: 'directory', + where, + name: null, + rawSpec: './', + fetchSpec: normalizePath(where), + }) + t.end() +}) + t.test('invalid url', t => { const broken = t.mock('..', { - url: { + 'node:url': { URL: class { constructor () { throw new Error('something went wrong') diff --git a/test/windows.js b/test/windows.js index 4406026..ba7c7a7 100644 --- a/test/windows.js +++ b/test/windows.js @@ -1,9 +1,13 @@ -global.FAKE_WINDOWS = true +// redefine process.platform before any requires so that we don't cache a require that got the non-redefined value +const { platform } = process +Object.defineProperty(process, 'platform', { value: 'win32' }) -const npa = require('..') const t = require('tap') +const npa = require('..') -t.on('bailout', () => process.exit(1)) +t.teardown(() => { + Object.defineProperty(process, 'platform', { value: platform }) +}) const cases = { 'C:\\x\\y\\z': { @@ -95,9 +99,12 @@ const cases = { t.test('parse a windows path', function (t) { Object.keys(cases).forEach(function (c) { - const expect = cases[c] - const actual = npa(c, 'C:\\test\\path') - t.has(actual, expect, c) + t.test(c, t => { + const expect = cases[c] + const actual = npa(c, 'C:\\test\\path') + t.has(actual, expect, c) + t.end() + }) }) t.end() })