Skip to content

Commit

Permalink
url: validate pathToFileURL(path) argument as string
Browse files Browse the repository at this point in the history
PR-URL: nodejs/node#49161
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
sercher committed Apr 25, 2024
1 parent be0f17c commit 9a8e69a
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 5 deletions.
4 changes: 2 additions & 2 deletions graal-nodejs/lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -1521,14 +1521,14 @@ function pathToFileURL(filepath) {
const hostnameEndIndex = StringPrototypeIndexOf(filepath, '\\', 2);
if (hostnameEndIndex === -1) {
throw new ERR_INVALID_ARG_VALUE(
'filepath',
'path',
filepath,
'Missing UNC resource path',
);
}
if (hostnameEndIndex === 2) {
throw new ERR_INVALID_ARG_VALUE(
'filepath',
'path',
filepath,
'Empty UNC servername',
);
Expand Down
11 changes: 10 additions & 1 deletion graal-nodejs/lib/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const {
domainToASCII,
domainToUnicode,
fileURLToPath,
pathToFileURL,
pathToFileURL: _pathToFileURL,
urlToHttpOptions,
unsafeProtocol,
hostlessProtocol,
Expand Down Expand Up @@ -1009,6 +1009,15 @@ Url.prototype.parseHost = function parseHost() {
if (host) this.hostname = host;
};

// When used internally, we are not obligated to associate TypeError with
// this function, so non-strings can be rejected by underlying implementation.
// Public API has to validate input and throw appropriate error.
function pathToFileURL(path) {
validateString(path, 'path');

return _pathToFileURL(path);
}

module.exports = {
// Original API
Url,
Expand Down
37 changes: 35 additions & 2 deletions graal-nodejs/test/parallel/test-url-pathtofileurl.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,30 @@ const url = require('url');

// Missing server:
assert.throws(() => url.pathToFileURL('\\\\\\no-server'), {
code: 'ERR_INVALID_ARG_VALUE'
code: 'ERR_INVALID_ARG_VALUE',
});

// Missing share or resource:
assert.throws(() => url.pathToFileURL('\\\\host'), {
code: 'ERR_INVALID_ARG_VALUE'
code: 'ERR_INVALID_ARG_VALUE',
});

// Regression test for direct String.prototype.startsWith call
assert.throws(() => url.pathToFileURL([
'\\\\',
{ [Symbol.toPrimitive]: () => 'blep\\blop' },
]), {
code: 'ERR_INVALID_ARG_TYPE',
});
assert.throws(() => url.pathToFileURL(['\\\\', 'blep\\blop']), {
code: 'ERR_INVALID_ARG_TYPE',
});
assert.throws(() => url.pathToFileURL({
[Symbol.toPrimitive]: () => '\\\\blep\\blop',
}), {
code: 'ERR_INVALID_ARG_TYPE',
});

} else {
// UNC paths on posix are considered a single path that has backslashes:
const fileURL = url.pathToFileURL('\\\\nas\\share\\path.txt').href;
Expand Down Expand Up @@ -144,3 +161,19 @@ const url = require('url');
assert.strictEqual(actual, expected);
}
}

// Test for non-string parameter
{
for (const badPath of [
undefined, null, true, 42, 42n, Symbol('42'), NaN, {}, [], () => {},
Promise.resolve('foo'),
new Date(),
new String('notPrimitive'),
{ toString() { return 'amObject'; } },
{ [Symbol.toPrimitive]: (hint) => 'amObject' },
]) {
assert.throws(() => url.pathToFileURL(badPath), {
code: 'ERR_INVALID_ARG_TYPE',
});
}
}

0 comments on commit 9a8e69a

Please sign in to comment.