fix: Detect host and protocol for alternate links correctly when running behind a proxy

[HHongSeungWoo]

We are currently utilizing a reverse proxy in front of Next.js.
request.url is returning an internal address instead of an external one.
Consequently, the rewrite functionality is not behaving as expected, and the alternatelink header is inadvertently exposing the internal address.

[vercel]

@HHongSeungWoo is attempting to deploy a commit to the next-intl Team on Vercel.

A member of the Team first needs to authorize it.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
example-next-13-next-auth	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 22, 2023 9:56am
next-intl-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 22, 2023 9:56am
next-intl-example-next-13	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 22, 2023 9:56am

[HHongSeungWoo]

I'm concerned about trusting the host of the header, but if there are multiple external addresses on the same server, I can't think of any other solution :(

[amannn]

Thank you so much for proposing this fix @HHongSeungWoo!

It's definitely a good idea to use the external host for the alternate links, please see my inline comment for details.

Can you please also include tests that would have failed previously and will now pass after the fix?

Thank you so much! 🙌

[HHongSeungWoo]

before / after

[dzenand]

Is this somehow related to this issue?
vercel/next.js#54277

[amannn]

This looks great! The only thing missing would be a regression test that helps to ensure that we keep this functionality in the future.

Thanks also for linking to the Next.js issue, I'll keep an eye on that, but seems like it should be addressed on the Next.js side.

[HHongSeungWoo]

This looks great! The only thing missing would be a regression test that helps to ensure that we keep this functionality in the future.

Thanks also for linking to the Next.js issue, I'll keep an eye on that, but seems like it should be addressed on the Next.js side.

I'm unsure about the necessary test cases. The request.url within the middleware is a string generated by Next.js. If a hostname is configured, it will be used; otherwise, localhost will be used.

Upon reviewing the code, it appears that the option trustHostHeader, which is not currently utilized, generates the URL with the request's host when set to true. However, since this isn't documented, it seems to be an unused option.

These changes are independent of external configurations or code modifications, so I can't think of any regression cases unless there are changes to the HTTP protocol itself.

[amannn]

@HHongSeungWoo I've added a commit where the now fixed behavior is handled within a single test case. I also noticed that we should consider the protocol as well and fixed that accordingly.

Thanks for noticing that we can use the NextRequest constructor instead of constructing a mock object, I've adapted the tests to use that consistently to avoid unnecessary redirection.

Would you like to have a last look? From my opinion this is good to go! 👍

[HHongSeungWoo]

@HHongSeungWoo I've added a commit where the now fixed behavior is handled within a single test case. I also noticed that we should consider the protocol as well and fixed that accordingly.

Thanks for noticing that we can use the NextRequest constructor instead of constructing a mock object, I've adapted the tests to use that consistently to avoid unnecessary redirection.

Would you like to have a last look? From my opinion this is good to go! 👍

Thank you for making the necessary corrections. It looks better to me as well. :)

[CarbonC]

Hello @amannn ! This looks great, but please would it be possible to push this fix to the beta branch as well? Currently im stuck with the bug on the beta-10 :-/
Thanks for the great work

[amannn]

@CarbonC Yep, definitely! I've got another release on the main branch coming, then I'll rebase the RSC branch and publish a new beta!

[amannn]

@CarbonC [email protected] is out now with all recent improvements from the main branch!