nextjs redirects strip query parameters

[tazapply]

It seems query parameters are being stripped off the URL when using nextjs redirects.

If you add something like this to the next.config.js

module.exports = {
  async redirects() {
    return [
      {
        source: '/one',
        destination: '/newplace',
        permanent: true,
      },
      {
        source: '/two/:path*',
        destination: '/newplacetwo/:path*',
        permanent: true,
      },
    ]
  },
};

Then test running nextjs in development mode it passes the query parameters along as per the documentation:

curl --head http://localhost:3000/one\?x\=1
HTTP/1.1 308 Permanent Redirect
Location: /newplace?x=1

Then deploy via tf-next and test:

curl --head https://d3il86aqaEXAMPLE.cloudfront.net/one\?x\=1
HTTP/2 308 
content-length: 0
server: CloudFront
date: Thu, 07 Apr 2022 22:40:07 GMT
cache-control: public,max-age=31536000,immutable
location: /newplace

Same if we use the :path* redirect. Locally in nextjs:

curl --head http://localhost:3000/two/tree\?x\=2           
HTTP/1.1 308 Permanent Redirect
Location: /newplacetwo/tree?x=2

and no query string when deployed via tf-next:

curl --head https://d3il86aqaEXAMPLE.cloudfront.net/two/tree\?x\=1
HTTP/2 308 
content-length: 0
server: CloudFront
date: Thu, 07 Apr 2022 22:38:23 GMT
cache-control: public,max-age=31536000,immutable
location: /newplacetwo/tree

[ofhouse]

Hi, thanks for the detailed report!
Yes seems like the query string is not appended, will take a look soon.

[kchh90]

Hi, I had a look at this issue and from what I've been able to gather so far:

By default nextjs redirects URLs with trailing slash to non-trailing slash counterpart

This means the first entry in the routes list generated by tf-next build will be

    {
      "src": "^(?:\\/((?:[^\\/]+?)(?:\\/(?:[^\\/]+?))*))\\/$",
      "headers": {
        "Location": "/$1"
      },
      "status": 308,
      "continue": true
    },

Which causes the Location header for the redirect to be set sans query params at

terraform-aws-next-js/packages/proxy/src/proxy.ts

Line 191 in a5a8fb5

if (headers) {

        if (headers) {
          for (const originalKey in headers) {
            const originalValue = headers[originalKey];
            const value = resolveRouteParameters(originalValue, match, keys);
            combinedHeaders[originalKey] = value;
          }
        }

Similarly any redirect added to next.config.js produces a similar route entry in the config.json file generated by tf-next build, the above redirects in the original post would result in the following entries:

    {
      "src": "^\\/en-US\\/one$",
      "headers": {
        "Location": "/newplace"
      },
      "status": 308
    },
    {
      "src": "^(?:\\/(en\\-US))\\/one$",
      "headers": {
        "Location": "/$1/newplace"
      },
      "status": 308
    },

Both of which cause a similar query param loss due to the current proxy.ts code.

Currently, I've been adding the query param back to the location header as a workaround for this issue but not sure if this is the best way to go about it since it likely defeats some of the performance enhancement logic provided by this package?

        if (headers) {
          for (const originalKey in headers) {
            const originalValue = headers[originalKey];
            const value = resolveRouteParameters(originalValue, match, keys);
            combinedHeaders[originalKey] = value;
            // Pass query params through to Location header.
            if (originalKey == 'Location') {
              combinedHeaders[originalKey] = value + '?' + parsedUrl.searchParams.toString();
            }
          }
        }`

[ofhouse]

Took a look at how Vercel handles it and it seems like they always append the original search params to the Location header:

curl --head https://some.vercel.app/one\?x\=1
HTTP/2 308 
location: /newplace?x=1
cache-control: public, max-age=0, must-revalidate
x-vercel-cache: MISS

What's interesting here ist that Vercel does not cache the response although 308 is a permanent redirect.
Even repeated requests result in a cache MISS.
We currently issue an cache-control: public,max-age=31536000,immutable on these redirects.

So I would vote to use the same solution for our implementation:

• Original querystring is always appended to Location header
• Cache control becomes public, max-age=0, must-revalidate for redirect responses

[ofhouse]

This has now been fixed in v0.12.2.