From 8d57e0fa0f7d1539722fa85bd22f6b4cc0300845 Mon Sep 17 00:00:00 2001 From: Sergey Chernyshev Date: Thu, 25 Apr 2024 04:21:20 +0200 Subject: [PATCH] url: validate `pathToFileURL(path)` argument as string PR-URL: https://github.com/nodejs/node/pull/49161 Reviewed-By: Joyee Cheung Reviewed-By: Antoine du Hamel --- graal-nodejs/lib/internal/url.js | 4 +- graal-nodejs/lib/url.js | 11 +++++- .../test/parallel/test-url-pathtofileurl.js | 37 ++++++++++++++++++- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/graal-nodejs/lib/internal/url.js b/graal-nodejs/lib/internal/url.js index a363a789d4c..8040aebf53a 100644 --- a/graal-nodejs/lib/internal/url.js +++ b/graal-nodejs/lib/internal/url.js @@ -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', ); diff --git a/graal-nodejs/lib/url.js b/graal-nodejs/lib/url.js index c99bf384962..5b30f11677e 100644 --- a/graal-nodejs/lib/url.js +++ b/graal-nodejs/lib/url.js @@ -54,7 +54,7 @@ const { domainToASCII, domainToUnicode, fileURLToPath, - pathToFileURL, + pathToFileURL: _pathToFileURL, urlToHttpOptions, unsafeProtocol, hostlessProtocol, @@ -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, diff --git a/graal-nodejs/test/parallel/test-url-pathtofileurl.js b/graal-nodejs/test/parallel/test-url-pathtofileurl.js index 068a04e6613..d18b5a41fdf 100644 --- a/graal-nodejs/test/parallel/test-url-pathtofileurl.js +++ b/graal-nodejs/test/parallel/test-url-pathtofileurl.js @@ -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; @@ -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', + }); + } +}