From 9e1e55ba4215cab829def3b6793dc2c4ded1dffe Mon Sep 17 00:00:00 2001
From: Kunal Kundu <kunal99kundu@gmail.com>
Date: Mon, 15 Aug 2022 15:57:52 +0530
Subject: [PATCH] fix: stop pass through request params for function redirects
 (#4897)

* fix: stop pass through request params for function redirects

* chore: remove only modifier from test

* test: check if splat is being passed as query param in function redirects

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
---
 src/utils/proxy.js                        |  7 +++--
 tests/integration/200.command.dev.test.js | 31 ++++++++++++++---------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/src/utils/proxy.js b/src/utils/proxy.js
index 2d2442b6489..0dd2e8640d1 100644
--- a/src/utils/proxy.js
+++ b/src/utils/proxy.js
@@ -201,10 +201,9 @@ const serveRedirect = async function ({ match, options, proxy, req, res }) {
     // construct destination URL from redirect rule match
     const dest = new URL(match.to, `${reqUrl.protocol}//${reqUrl.host}`)
 
-    // We pass through request params in one of the following cases:
-    // 1. The redirect rule doesn't have any query params
-    // 2. This is a function redirect https://github.com/netlify/cli/issues/1605
-    if ([...dest.searchParams].length === 0 || isFunction(options.functionsPort, stripOrigin(dest))) {
+    // We pass through request params if the redirect rule
+    // doesn't have any query params
+    if ([...dest.searchParams].length === 0) {
       dest.searchParams.forEach((_, key) => {
         dest.searchParams.delete(key)
       })
diff --git a/tests/integration/200.command.dev.test.js b/tests/integration/200.command.dev.test.js
index 592e1a1171e..42d7cc91e4f 100644
--- a/tests/integration/200.command.dev.test.js
+++ b/tests/integration/200.command.dev.test.js
@@ -37,10 +37,11 @@ testMatrix.forEach(({ args }) => {
         })
         .withRedirectsFile({
           redirects: [
-            { from: `/api/*`, to: `/.netlify/functions/echo?a=1&a=2`, status: '200' },
-            { from: `/foo`, to: `/`, status: '302' },
-            { from: `/bar`, to: `/?a=1&a=2`, status: '302' },
-            { from: `/test id=:id`, to: `/?param=:id` },
+            { from: '/api/*', to: '/.netlify/functions/echo?a=1&a=2', status: '200' },
+            { from: '/foo', to: '/', status: '302' },
+            { from: '/bar', to: '/?a=1&a=2', status: '302' },
+            { from: '/test id=:id', to: '/?param=:id' },
+            { from: '/baz/*', to: '/.netlify/functions/echo?query=:splat' },
           ],
         })
         .withFunction({
@@ -53,15 +54,18 @@ testMatrix.forEach(({ args }) => {
         .buildAsync()
 
       await withDevServer({ cwd: builder.directory, args }, async (server) => {
-        const [fromFunction, queryPassthrough, queryInRedirect, withParamMatching] = await Promise.all([
-          got(`${server.url}/api/test?foo=1&foo=2&bar=1&bar=2`).json(),
-          got(`${server.url}/foo?foo=1&foo=2&bar=1&bar=2`, { followRedirect: false }),
-          got(`${server.url}/bar?foo=1&foo=2&bar=1&bar=2`, { followRedirect: false }),
-          got(`${server.url}/test?id=1`, { followRedirect: false }),
-        ])
+        const [fromFunction, queryPassthrough, queryInRedirect, withParamMatching, functionWithSplat] =
+          await Promise.all([
+            got(`${server.url}/api/test?foo=1&foo=2&bar=1&bar=2`).json(),
+            got(`${server.url}/foo?foo=1&foo=2&bar=1&bar=2`, { followRedirect: false }),
+            got(`${server.url}/bar?foo=1&foo=2&bar=1&bar=2`, { followRedirect: false }),
+            got(`${server.url}/test?id=1`, { followRedirect: false }),
+            got(`${server.url}/baz/abc`).json(),
+          ])
 
-        // query params should be taken from the request
-        t.deepEqual(fromFunction.multiValueQueryStringParameters, { foo: ['1', '2'], bar: ['1', '2'] })
+        // query params should be taken from redirect rule for functions
+        // eslint-disable-next-line id-length
+        t.deepEqual(fromFunction.multiValueQueryStringParameters, { a: ['1', '2'] })
 
         // query params should be passed through from the request
         t.is(queryPassthrough.headers.location, '/?foo=1&foo=2&bar=1&bar=2')
@@ -71,6 +75,9 @@ testMatrix.forEach(({ args }) => {
 
         // query params should be taken from the redirect rule
         t.is(withParamMatching.headers.location, '/?param=1')
+
+        // splat should be passed as query param in function redirects
+        t.deepEqual(functionWithSplat.queryStringParameters, { query: 'abc' })
       })
     })
   })