From 57e78bc9efc1682f6fb083030df708630ebf3429 Mon Sep 17 00:00:00 2001 From: LiviaMedeiros Date: Mon, 14 Aug 2023 17:19:34 +0800 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 --- lib/internal/url.js | 4 +-- lib/url.js | 11 +++++++- test/parallel/test-url-pathtofileurl.js | 37 +++++++++++++++++++++++-- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/lib/internal/url.js b/lib/internal/url.js index deaf4d4ef75b9b..92d5e84e87c71f 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -1449,14 +1449,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/lib/url.js b/lib/url.js index 1cf27ec371e581..b6b5376c379a6c 100644 --- a/lib/url.js +++ b/lib/url.js @@ -53,7 +53,7 @@ const { domainToASCII, domainToUnicode, fileURLToPath, - pathToFileURL, + pathToFileURL: _pathToFileURL, urlToHttpOptions, unsafeProtocol, hostlessProtocol, @@ -1017,6 +1017,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/test/parallel/test-url-pathtofileurl.js b/test/parallel/test-url-pathtofileurl.js index 068a04e6613b28..d18b5a41fdfc2f 100644 --- a/test/parallel/test-url-pathtofileurl.js +++ b/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', + }); + } +}