fixup! url: handle "unsafe" characters properly in pathToFileURL

fastest alternative
nodejs · Aug 27, 2024 · 4aa41b2 · 4aa41b2
1 parent 49959c3
commit 4aa41b2
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 34 deletions.
diff --git a/lib/internal/url.js b/lib/internal/url.js
@@ -24,7 +24,6 @@ const {
   StringPrototypeCodePointAt,
   StringPrototypeIncludes,
   StringPrototypeIndexOf,
-  StringPrototypeReplaceAll,
   StringPrototypeSlice,
   StringPrototypeStartsWith,
   StringPrototypeToWellFormed,
@@ -1503,64 +1502,65 @@ function fileURLToPath(path, options = kEmptyObject) {
 
 // RFC1738 defines the following chars as "unsafe" for URLs
 // @see https://www.ietf.org/rfc/rfc1738.txt 2.2. URL Character Encoding Issues
+const percentRegEx = /%/g;
+const newlineRegEx = /\n/g;
+const carriageReturnRegEx = /\r/g;
+const tabRegEx = /\t/g;
+const quoteRegEx = /"/g;
+const hashRegex = /#/g;
+const spaceRegEx = / /g;
+const questionMarkRegex = /\?/g;
+const openSquareBracketRegEx = /\[/g;
 const backslashRegEx = /\\/g;
+const closeSquareBracketRegEx = /]/g;
+const caretRegEx = /\^/g;
+const verticalBarRegEx = /\|/g;
+const tildeRegEx = /~/g;
 
 function encodePathChars(filepath, options = kEmptyObject) {
   if (StringPrototypeIncludes(filepath, '%')) {
-    filepath = StringPrototypeReplaceAll(filepath, '%', '%25');
+    filepath = RegExpPrototypeSymbolReplace(percentRegEx, filepath, '%25');
   }
 
   if (StringPrototypeIncludes(filepath, '\t')) {
-    filepath = StringPrototypeReplaceAll(filepath, '\t', '%09');
+    filepath = RegExpPrototypeSymbolReplace(tabRegEx, filepath, '%09');
   }
   if (StringPrototypeIncludes(filepath, '\n')) {
-    filepath = StringPrototypeReplaceAll(filepath, '\n', '%0A');
+    filepath = RegExpPrototypeSymbolReplace(newlineRegEx, filepath, '%0A');
   }
   if (StringPrototypeIncludes(filepath, '\r')) {
-    filepath = StringPrototypeReplaceAll(filepath, '\r', '%0D');
+    filepath = RegExpPrototypeSymbolReplace(carriageReturnRegEx, filepath, '%0D');
   }
   if (StringPrototypeIncludes(filepath, ' ')) {
-    filepath = StringPrototypeReplaceAll(filepath, ' ', '%20');
+    filepath = RegExpPrototypeSymbolReplace(spaceRegEx, filepath, '%20');
   }
   if (StringPrototypeIncludes(filepath, '"')) {
-    filepath = StringPrototypeReplaceAll(filepath, '"', '%22');
+    filepath = RegExpPrototypeSymbolReplace(quoteRegEx, filepath, '%22');
   }
   if (StringPrototypeIncludes(filepath, '#')) {
-    filepath = StringPrototypeReplaceAll(filepath, '#', '%23');
-  }
-  if (StringPrototypeIncludes(filepath, '<')) {
-    filepath = StringPrototypeReplaceAll(filepath, '<', '%3C');
-  }
-  if (StringPrototypeIncludes(filepath, '>')) {
-    filepath = StringPrototypeReplaceAll(filepath, '>', '%3E');
+    filepath = RegExpPrototypeSymbolReplace(hashRegex, filepath, '%23');
   }
   if (StringPrototypeIncludes(filepath, '?')) {
-    filepath = StringPrototypeReplaceAll(filepath, '?', '%3F');
+    filepath = RegExpPrototypeSymbolReplace(questionMarkRegex, filepath, '%3F');
   }
   if (StringPrototypeIncludes(filepath, '[')) {
-    filepath = StringPrototypeReplaceAll(filepath, '[', '%5B');
+    filepath = RegExpPrototypeSymbolReplace(openSquareBracketRegEx, filepath, '%5B');
   }
   // Back-slashes must be special-cased on Windows, where they are treated as path separator.
   if (!options.windows && StringPrototypeIncludes(filepath, '\\')) {
-    filepath = StringPrototypeReplaceAll(filepath, '\\', '%5C');
+    filepath = RegExpPrototypeSymbolReplace(backslashRegEx, filepath, '%5C');
   }
   if (StringPrototypeIncludes(filepath, ']')) {
-    filepath = StringPrototypeReplaceAll(filepath, ']', '%5D');
+    filepath = RegExpPrototypeSymbolReplace(closeSquareBracketRegEx, filepath, '%5D');
   }
   if (StringPrototypeIncludes(filepath, '^')) {
-    filepath = StringPrototypeReplaceAll(filepath, '^', '%5E');
-  }
-  if (StringPrototypeIncludes(filepath, '`')) {
-    filepath = StringPrototypeReplaceAll(filepath, '`', '%60');
-  }
-  if (StringPrototypeIncludes(filepath, '{')) {
-    filepath = StringPrototypeReplaceAll(filepath, '{', '%7B');
+    filepath = RegExpPrototypeSymbolReplace(caretRegEx, filepath, '%5E');
   }
   if (StringPrototypeIncludes(filepath, '|')) {
-    filepath = StringPrototypeReplaceAll(filepath, '|', '%7C');
+    filepath = RegExpPrototypeSymbolReplace(verticalBarRegEx, filepath, '%7C');
   }
-  if (StringPrototypeIncludes(filepath, '}')) {
-    filepath = StringPrototypeReplaceAll(filepath, '}', '%7D');
+  if (StringPrototypeIncludes(filepath, '~')) {
+    filepath = RegExpPrototypeSymbolReplace(tildeRegEx, filepath, '%7E');
   }
 
   return filepath;

diff --git a/test/parallel/test-url-pathtofileurl.js b/test/parallel/test-url-pathtofileurl.js
@@ -157,12 +157,8 @@ const posixTestCases = [
   { path: '/€', expected: 'file:///%E2%82%AC' },
   // Rocket emoji (non-BMP code point)
   { path: '/🚀', expected: 'file:///%F0%9F%9A%80' },
-  // caret
-  { path: '/foo^bar', expected: 'file:///foo%5Ebar' },
-  // left bracket
-  { path: '/foo[bar', expected: 'file:///foo%5Bbar' },
-  // right bracket
-  { path: '/foo]bar', expected: 'file:///foo%5Dbar' },
+  // "unsafe" chars
+  { path: '/foo\r\n\t<>"#%{}|^[\\~]`?bar', expected: 'file:///foo%0D%0A%09%3C%3E%22%23%25%7B%7D%7C%5E%5B%5C%7E%5D%60%3Fbar' },
 ];
 
 for (const { path, expected } of windowsTestCases) {