Fix redirect when containing "+" and decode values for URLSearchParams (#603)

* encode "+" character in query string

* Decode query string for URLSearchParams

* add test case for + character and query string

* fix format with prettier

* fix up comment

* internal redirect expects result to be undefined

* Revert "internal redirect expects result to be undefined"

This reverts commit ea90c3c.

* internal redirect expects result to be undefined

* Revert "internal redirect expects result to be undefined"

This reverts commit a6f5ccb.

* remove internal and set locale to false

* Create big-grapes-hammer.md

---------

Co-authored-by: conico974 <[email protected]>

Author: john-trieu-nguyen

.changeset/big-grapes-hammer.md

@@ -0,0 +1,5 @@
+---
+"@opennextjs/aws": patch
+---
+
+Fix redirect when containing "+" and decode values for URLSearchParams

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

@@ -200,10 +200,18 @@ 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 toDestinationHost = compile(escapeRegex(hostname ?? "") ?? "");
-    const toDestinationQuery = compile(escapeRegex(queryString ?? "") ?? "");
+    const toDestinationQuery = compile(
+      escapeRegex(encodePlusQueryString ?? "") ?? "",
+    );
     let params = {
       // params for the source
       ...getParamsFromSource(match(escapeRegex(rewrite?.source) ?? ""))(
@@ -219,7 +227,7 @@ export function handleRewrites<T extends RewriteDefinition>(
       }, {}),
     };
     const isUsingParams = Object.keys(params).length > 0;
-    let rewrittenQuery = queryString;
+    let rewrittenQuery = encodePlusQueryString;
     let rewrittenHost = hostname;
     let rewrittenPath = pathname;
     if (isUsingParams) {

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

@@ -98,12 +98,15 @@ export function convertRes(res: OpenNextNodeResponse): InternalResult {
  * @__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();
   Object.entries(query).forEach(([key, value]) => {
     if (Array.isArray(value)) {
-      value.forEach((entry) => urlQuery.append(key, entry));
+      value.forEach((entry) => urlQuery.append(key, decodeURIComponent(entry)));
     } else {
-      urlQuery.append(key, value);
+      urlQuery.append(key, decodeURIComponent(value));
     }
   });
   const queryString = urlQuery.toString();

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

@@ -284,6 +284,27 @@ describe("handleRedirects", () => {
 
     expect(result).toBeUndefined();
   });
+
+  it("should redirect with + character and query string", () => {
+    const event = createEvent({
+      url: "/foo",
+    });
+
+    const result = handleRedirects(event, [
+      {
+        source: "/foo",
+        destination: "/search?bar=hello+world&baz=new%2C+earth",
+        locale: false,
+        statusCode: 308,
+        regex: "^(?!/_next)/foo(?:/)?$",
+      },
+    ]);
+
+    expect(result.statusCode).toEqual(308);
+    expect(result.headers.Location).toEqual(
+      "/search?bar=hello+world&baz=new%2C+earth",
+    );
+  });
 });
 
 describe("handleRewrites", () => {