From 5ffa2734fe1ea0078e90991d9d2d2c0bceec2c08 Mon Sep 17 00:00:00 2001 From: Gar Date: Fri, 20 Dec 2024 09:42:29 -0800 Subject: [PATCH] fixup! windows paths and a bunch of encoding nonsense --- README.md | 26 +++++++++++++++++--------- lib/npa.js | 35 ++++++++--------------------------- test/basic.js | 3 --- 3 files changed, 25 insertions(+), 39 deletions(-) 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 bde6e7f..d86e072 100644 --- a/lib/npa.js +++ b/lib/npa.js @@ -3,7 +3,8 @@ const isWindows = process.platform === 'win32' const { URL } = require('node:url') -const path = isWindows ? require('node:path').win32 : require('node:path') +// 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') @@ -276,39 +277,17 @@ function pathToFileURL (str) { for (let i = 0; i < str.length; i++) { result = `${result}${encodedPathChars.get(str[i]) ?? str[i]}` } + if (result.startsWith('file:')) { + return result + } return `file:${result}` } -/* parse file package args: - * - * /posix/path - * ./posix/path - * .dotfile - * .dot/path - * filename - * filename.with.ext - * C:\windows\path - * path/with/no/leading/separator - * - * translates to relative ./path - * - file:{path} - * - * translates to absolute /path - * - file:/{path} - * - file://{path} (this is not RFC compliant, but is supported for backward compatibility) - * - file:///{path} - * - * file: specs are url encoded, bare paths are not - * - */ function fromFile (res, where) { res.type = isFileType.test(res.rawSpec) ? 'file' : 'directory' res.where = where - let rawSpec = res.rawSpec - if (!rawSpec.startsWith('file:')) { - rawSpec = pathToFileURL(rawSpec) - } + let rawSpec = pathToFileURL(res.rawSpec) if (rawSpec.startsWith('file:/')) { // XXX backwards compatibility lack of compliance with RFC 8089 @@ -361,6 +340,8 @@ function fromFile (res, where) { } 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('/') return res } diff --git a/test/basic.js b/test/basic.js index 90d8403..d6adbd6 100644 --- a/test/basic.js +++ b/test/basic.js @@ -6,7 +6,6 @@ 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 } @@ -15,8 +14,6 @@ const t = require('tap') const npa = t.mock('..', { path }) t.test('basic', function (t) { - t.setMaxListeners(999) - const tests = { 'foo@1.2': { name: 'foo',