url: validate pathToFileURL(path) argument as string

PR-URL: nodejs/node#49161 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
sercher · Apr 25, 2024 · 9a8e69a · 9a8e69a
1 parent be0f17c
commit 9a8e69a
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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',
+    });
+  }
+}