When redirecting from middleware double `x-forwarded-proto: https` set by the proxies/gateways breaks the redirect (uses localhost:3000 by default)

phev8 commented 1 year ago

Verify canary release

[X] I verified that the issue exists in the latest Next.js canary release

Provide environment information

Not the actual environment but can be reproduced here:
Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 22.5.0: Thu Jun  8 22:22:20 PDT 2023; root:xnu-8796.121.3~7/RELEASE_ARM64_T6000
    Binaries:
      Node: 18.16.0
      npm: 9.5.1
      Yarn: 1.22.19
      pnpm: 8.6.12
    Relevant Packages:
      next: 13.4.19
      eslint-config-next: 13.4.19
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.1.6
    Next.js Config:
      output: N/A

Which area(s) of Next.js are affected? (leave empty if unsure)

Middleware / Edge (API routes, runtime)

Link to the code that reproduces this issue or a replay of the bug

https://github.com/coneno/nextjs-middleware-redirect-issue

To Reproduce

Start dev or test server.

Using: curl --location 'http://localhost:3000/' --header 'x-forwarded-proto: https' --header 'x-forwarded-proto: https' -i Results in:

HTTP/1.1 307 Temporary Redirect
date: Wed, 23 Aug 2023 14:56:19 GMT
location: https://localhost:3000/test
Connection: keep-alive
Keep-Alive: timeout=5
Transfer-Encoding: chunked

Note Location is an absolute path compared to expected redirect (see below).

Describe the Bug

Having a simple redirect middleware:

import { NextResponse, NextRequest } from 'next/server'

export const config = {
    matcher: ['/']
}

export function middleware(req: NextRequest) { 
    const newURL = req.nextUrl.clone();
    newURL.pathname = `/test`;
    return NextResponse.redirect(newURL);    
}

due to the infrastructure (gateways / ingress) there are two x-forwarded-proto: https present in the request header (simulated in curl command to reproduce)
when there is a second x-forwarded-proto: https in the header, it breaks the application, because it uses an absolute path in the redirect location header
this absolute path is always localhost:3000/<pathname>, not the URL from the request (when using on the production server in OpenShift - not sure how to simulate this with dev server without proxy)

Expected Behavior

Using: curl --location 'http://localhost:3000/' --header 'x-forwarded-proto: https' -i Redirects as expected: (note location being relative)

HTTP/1.1 307 Temporary Redirect
date: Wed, 23 Aug 2023 14:59:43 GMT
location: /test
Connection: keep-alive
Keep-Alive: timeout=5
Transfer-Encoding: chunked

HTTP/1.1 200 OK
cache-control: no-store, must-revalidate
content-type: text/html; charset=utf-8
date: Wed, 23 Aug 2023 14:59:43 GMT
Vary: RSC, Next-Router-State-Tree, Next-Router-Prefetch, Next-Url, Accept-Encoding
x-powered-by: Next.js
Connection: keep-alive
Keep-Alive: timeout=5
Transfer-Encoding: chunked

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

OpenShift behind an Azure Gateway

phev8 commented 1 year ago

Printing the new URL in both test cases results in the same value, so I assume the difference in behaviour comes form NextResponse.redirect(...)

thollander commented 1 year ago

This might be linked to this comment from last year : https://github.com/vercel/next.js/pull/37925/files#r906559517

phev8 commented 1 year ago

After some testing, this issue is not present in v13.4.12, but the behaviour started from v.13.4.13 So likely one of the changed from here: https://github.com/vercel/next.js/releases/tag/v13.4.13

phev8 commented 1 year ago

Might this be related to this PR https://github.com/vercel/next.js/pull/53157 (and how requests are cloned when processing the redirect)? @balazsorban44 I don't see too much other related changes from 13.4.12 ->13.4.13