Skip to content

test: add more test cases for pathToFileURL#63293

Open
RafaelGSS wants to merge 1 commit into
nodejs:mainfrom
RafaelGSS:test-include-testcases-pathtofileurl
Open

test: add more test cases for pathToFileURL#63293
RafaelGSS wants to merge 1 commit into
nodejs:mainfrom
RafaelGSS:test-include-testcases-pathtofileurl

Conversation

@RafaelGSS
Copy link
Copy Markdown
Member

Just to make sure we won't introduce any regression anytime further (based on H1 reports we received).

Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels May 13, 2026
@RafaelGSS RafaelGSS force-pushed the test-include-testcases-pathtofileurl branch from 4069b78 to 875c813 Compare May 13, 2026 18:58
@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.03%. Comparing base (c24e552) to head (875c813).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63293      +/-   ##
==========================================
- Coverage   90.05%   90.03%   -0.02%     
==========================================
  Files         714      714              
  Lines      225247   225247              
  Branches    42578    42573       -5     
==========================================
- Hits       202842   202804      -38     
- Misses      14181    14229      +48     
+ Partials     8224     8214      -10     

see 39 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment on lines +25 to +35
const forbiddenHostnameChars = [
'\\\\exa mple\\share\\file.txt',
'\\\\host name\\share\\file.txt', // space
'\\\\host#name\\share\\file.txt', // hash
'\\\\host?name\\share\\file.txt', // question mark
'\\\\host@name\\share\\file.txt', // at sign
'\\\\host:name\\share\\file.txt', // colon
'\\\\host/name\\share\\file.txt', // forward slash
'\\\\host[name\\share\\file.txt', // left bracket
'\\\\host]name\\share\\file.txt', // right bracket
];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to https://url.spec.whatwg.org/#host-miscellaneous, there are a few more forbidden char we might as well check for:

Suggested change
const forbiddenHostnameChars = [
'\\\\exa mple\\share\\file.txt',
'\\\\host name\\share\\file.txt', // space
'\\\\host#name\\share\\file.txt', // hash
'\\\\host?name\\share\\file.txt', // question mark
'\\\\host@name\\share\\file.txt', // at sign
'\\\\host:name\\share\\file.txt', // colon
'\\\\host/name\\share\\file.txt', // forward slash
'\\\\host[name\\share\\file.txt', // left bracket
'\\\\host]name\\share\\file.txt', // right bracket
];
const forbiddenHostnameChars = Array.from('\0\t\n\r #:<>?@[]^|', (sep) => `\\\\host${sep}name\\share\\file.txt`);

But it looks like \n, \r, \t are ignored instead of being rejected; # and ? might have a special significant in UNC not sure

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opened ada-url/ada#1142 to address the # and ? cases

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

Labels

needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants