fix use searchParams original encoding

Author: conico974

packages/open-next/src/core/routing/matcher.ts

@@ -196,19 +196,16 @@ export function handleRewrites<T extends RewriteDefinition>(
     );
     // We need to use a localized path if the rewrite is not locale specific
     const pathToUse = rewrite.locale === false ? rawPath : localizedRawPath;
-    // We need to encode the "+" character with its UTF-8 equivalent "%20" to avoid 2 issues:
-    // 1. The compile function from path-to-regexp will throw an error if it finds a "+" character.
-    // https://github.com/pillarjs/path-to-regexp?tab=readme-ov-file#unexpected--or-
-    // 2. The convertToQueryString function will replace the "+" character with %2B instead of %20.
-    // %2B does not get interpreted as a space in the URL thus breaking the query string.
-    const encodePlusQueryString = queryString.replaceAll("+", "%20");
+
     debug("urlParts", { pathname, protocol, hostname, queryString });
-    const toDestinationPath = compile(escapeRegex(pathname));
+    const toDestinationPath = compile(escapeRegex(pathname, { isPath: true }));
     const toDestinationHost = compile(escapeRegex(hostname));
-    const toDestinationQuery = compile(escapeRegex(encodePlusQueryString));
+    const toDestinationQuery = compile(escapeRegex(queryString));
     const params = {
       // params for the source
-      ...getParamsFromSource(match(escapeRegex(rewrite.source)))(pathToUse),
+      ...getParamsFromSource(
+        match(escapeRegex(rewrite.source, { isPath: true })),
+      )(pathToUse),
       // params for the has
       ...rewrite.has?.reduce((acc, cur) => {
         return Object.assign(acc, computeHas(cur));
@@ -219,7 +216,7 @@ export function handleRewrites<T extends RewriteDefinition>(
       }, {}),
     };
     const isUsingParams = Object.keys(params).length > 0;
-    let rewrittenQuery = encodePlusQueryString;
+    let rewrittenQuery = queryString;
     let rewrittenHost = hostname;
     let rewrittenPath = pathname;
     if (isUsingParams) {

packages/open-next/src/core/routing/util.ts

@@ -122,23 +122,20 @@ export function convertRes(res: OpenNextNodeResponse): InternalResult {
  * Make sure that multi-value query parameters are transformed to
  * ?key=value1&key=value2&... so that Next converts those parameters
  * to an array when reading the query parameters
+ * query should be properly encoded before using this function
  * @__PURE__
  */
 export function convertToQueryString(query: Record<string, string | string[]>) {
-  // URLSearchParams is a representation of the PARSED query.
-  // So we must decode the value before appending it to the URLSearchParams.
-  // https://stackoverflow.com/a/45516812
-  const urlQuery = new URLSearchParams();
+  const queryStrings: string[] = [];
   Object.entries(query).forEach(([key, value]) => {
     if (Array.isArray(value)) {
-      value.forEach((entry) => urlQuery.append(key, decodeURIComponent(entry)));
+      value.forEach((entry) => queryStrings.push(`${key}=${entry}`));
     } else {
-      urlQuery.append(key, decodeURIComponent(value));
+      queryStrings.push(`${key}=${value}`);
     }
   });
-  const queryString = urlQuery.toString();
 
-  return queryString ? `?${queryString}` : "";
+  return queryStrings.length > 0 ? `?${queryStrings.join("&")}` : "";
 }
 
 /**
@@ -182,11 +179,15 @@ export function getMiddlewareMatch(
  *
  * @__PURE__
  */
-export function escapeRegex(str: string) {
-  return str
+export function escapeRegex(
+  str: string,
+  { isPath }: { isPath?: boolean } = {},
+) {
+  const result = str
     .replaceAll("(.)", "_µ1_")
     .replaceAll("(..)", "_µ2_")
     .replaceAll("(...)", "_µ3_");
+  return isPath ? result : result.replaceAll("+", "_µ4_");
 }
 
 /**
@@ -197,7 +198,8 @@ export function unescapeRegex(str: string) {
   return str
     .replaceAll("_µ1_", "(.)")
     .replaceAll("_µ2_", "(..)")
-    .replaceAll("_µ3_", "(...)");
+    .replaceAll("_µ3_", "(...)")
+    .replaceAll("_µ4_", "+");
 }
 
 /**

packages/tests-unit/tests/core/routing/util.test.ts

@@ -308,6 +308,16 @@ describe("convertToQueryString", () => {
       "?key=value1&key=value2&another=value3",
     );
   });
+
+  it("should respect existing query encoding", () => {
+    const query = {
+      key: ["value%201", "value2+something+else"],
+      another: "value3",
+    };
+    expect(convertToQueryString(query)).toBe(
+      "?key=value%201&key=value2+something+else&another=value3",
+    );
+  });
 });
 
 describe("convertToQuery", () => {
@@ -383,13 +393,18 @@ describe("regex", () => {
       ["/a/b/(..)(..)c", "/a/b/_µ2__µ2_c"],
       ["/a/(...)b", "/a/_µ3_b"],
       ["/feed/(..)photo/[id]", "/feed/_µ2_photo/[id]"],
+      ["?something=a+b", "?something=a_µ4_b"],
     ])(
-      "should escape (.), (..), (...) with _µ1_, _µ2_, _µ3_ - %s",
+      "should escape (.), (..), (...), + with _µ1_, _µ2_, _µ3_, _µ4_ - %s",
       (input, expected) => {
         const result = escapeRegex(input);
         expect(result).toBe(expected);
       },
     );
+
+    it("should not escape plus for a path", () => {
+      expect(escapeRegex("/:path+", { isPath: true })).toBe("/:path+");
+    });
   });
 
   describe("unescapeRegex", () => {
@@ -399,6 +414,7 @@ describe("regex", () => {
       ["/a/b/_µ2__µ2_c", "/a/b/(..)(..)c"],
       ["/a/_µ3_b", "/a/(...)b"],
       ["/feed/_µ2_photo/[id]", "/feed/(..)photo/[id]"],
+      ["?something=a_µ4_b", "?something=a+b"],
     ])(
       "should unescape _µ1_, _µ2_, _µ3_ with (.), (..), (...) - %s",
       (input, expected) => {