Skip to content

Conversation

AgainPsychoX
Copy link
Contributor

@AgainPsychoX AgainPsychoX commented Jul 3, 2022

Allow hash character in paths

Instead file:${...} we need to use pathToFileURL (from node:url) because some paths can contain hash character (#) or other characters (I guess, question mark (?) could also be a problem) that are cut off when the URI (file: schema) points file to be accessed by file system.

The file:${...} instances would need replacing hash character (#) with its URI encoded value (%23) to prevent it being reparsed along the way. My previous idea of using pathToFileURL wouldn't work, as relative paths got pre-resolved before converting to URL, i.e. file:../hello with CWD of S:\\test\\directory would became file://S:\\test\\hello while some instances of code (NPA) expected not unresolved file:..\\hello. Aside of that, simply replacing the char would be faster. By the way, I am almost certain that the hash character is the only "weird" character that could be used in the paths, as question mark (despite being beginning of query params) hardly can't nor should be used for names (Windows disallows).

References

Fixes #5120

Pending work

  • Check other places where file: was used and if similar patch could be applied.
@AgainPsychoX AgainPsychoX marked this pull request as ready for review July 4, 2022 01:06
@AgainPsychoX AgainPsychoX requested a review from a team as a code owner July 4, 2022 01:06
@darcyclarke darcyclarke added the semver:patch semver patch level for changes label Jul 26, 2022
@lukekarrys
Copy link
Contributor

Looks good, thanks for the fix!

It's possible in the future we may want a standardized way of doing this. We've had mixed luck with small utils to do stuff like this, so this is great for now. Just commenting in case we want to DRY it up in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver:patch semver patch level for changes

4 participants