Skip to content

Conversation

@wraithgar
Copy link
Member

Properly creates file package args that contain characters that need to be url encoded.

There is also a refactor/cleanup in here

  • Removed the magic windows global for testing, fixing the tests to mock process.platform instead.
  • Moved inline regexes up to where the others are defined
  • Renamed a few variables to be more correct (i.e. isFilename to isFileType)
  • Refactored Result to be a proper Class instead of a function w/ prototypes

Closes: #193

@wraithgar wraithgar requested a review from a team as a code owner December 18, 2024 22:16
@wraithgar
Copy link
Member Author

There still appears to be some incorrect behavior here, that was here even before this PR. saveSpec is a file: url but it is not url encoded. Most things this will generate won't cause decodeURI to throw:

> require('.')('/path/br[acket')
Result {
  type: 'directory',
  registry: undefined,
  where: '/Users/wraithgar/Development/npm/npm-package-arg/branches/gar_path-url-encoding',
  raw: '/path/br[acket',
  rawSpec: '/path/br[acket',
  saveSpec: 'file:/path/br[acket',
  fetchSpec: '/path/br[acket',
  gitRange: undefined,
  gitCommittish: undefined,
  gitSubdir: undefined,
  hosted: undefined
}
> decodeURI('file:/path/br[acket')
'file:/path/br[acket'

but some new characters that this now allows (like %) will:

> require('.')('/path/per%cent')
Result {
  type: 'directory',
  registry: undefined,
  where: '/Users/wraithgar/Development/npm/npm-package-arg/branches/gar_path-url-encoding',
  raw: '/path/per%cent',
  rawSpec: '/path/per%cent',
  saveSpec: 'file:/path/per%cent',
  fetchSpec: '/path/per%cent',
  gitRange: undefined,
  gitCommittish: undefined,
  gitSubdir: undefined,
  hosted: undefined
}
> decodeURI('file:/path/per%cent')
Uncaught URIError: URI malformed

I chose not to encode saveSpec in this PR because it's likely a breaking change. Consumers should always be decoding that, but they may not be currently.

@wraithgar
Copy link
Member Author

wraithgar commented Dec 18, 2024

Still some \ shenanigans to work out, evidently. The saveSpec normalization in the tests is likely hiding potential bugs.

@wraithgar wraithgar marked this pull request as draft December 18, 2024 22:46
@wraithgar wraithgar force-pushed the gar/path-url-encoding branch 5 times, most recently from 3e28f1e to ce6c05b Compare December 20, 2024 18:30
Properly creates file package args that contain characters that need to be url encoded.

There is also a refactor/cleanup in here
 - Removed the magic windows global for testing, fixing the tests to mock process.platform instead.
 - Moved inline regexes up to where the others are defined
 - Renamed a few variables to be more correct (i.e. isFilename to isFileType)
 - Refactored Result to be a proper Class instead of a function w/ prototypes

Closes: #193
@wraithgar wraithgar force-pushed the gar/path-url-encoding branch from ce6c05b to 3c876a6 Compare December 20, 2024 18:36
@wraithgar wraithgar marked this pull request as ready for review December 20, 2024 18:39
@wraithgar wraithgar merged commit 14cb8a1 into main Feb 5, 2025
20 checks passed
@wraithgar wraithgar deleted the gar/path-url-encoding branch February 5, 2025 18:41
@github-actions github-actions bot mentioned this pull request Feb 5, 2025
wraithgar pushed a commit that referenced this pull request Feb 5, 2025
🤖 I have created a release *beep* *boop*
---


##
[12.0.2](v12.0.1...v12.0.2)
(2025-02-05)
### Bug Fixes
*
[`14cb8a1`](14cb8a1)
[#200](#200) properly parse
non-url encoded file specs (#200) (@wraithgar)
### Chores
*
[`1343a54`](1343a54)
[#199](#199) bump
@npmcli/template-oss from 4.23.4 to 4.23.5 (#199) (@dependabot[bot],
@npm-cli-bot)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Failed to decode path name with valid symbols

2 participants