Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port added to Domain #658

Closed
FNGR2911 opened this issue Nov 23, 2023 · 16 comments · Fixed by #662 or #979
Closed

Port added to Domain #658

FNGR2911 opened this issue Nov 23, 2023 · 16 comments · Fixed by #662 or #979
Labels
bug Something isn't working reproduction-missing unconfirmed Needs triage.

Comments

@FNGR2911
Copy link

Description

I use the middleware with domain-based routing and have the problem that the port is appended to the domain on my server when the domain is entered without a prefix.

For example:
frontend.domain.com => frontend.domain.com:3000/en
frontend.domain.ca => frontend.domain.ca:3000/fr

I use Caddy as a reverse proxy. If I enter the entire address including the language prefix (e.g. frontend.domain.com/en) it works without any problems and the port is not appended.

Mandatory reproduction URL (CodeSandbox or GitHub repository)

NA

Reproduction description

Next.js 14.0.3, next-intl 3.0

Domain Config:

const domains = [
  {
    id: 1,
    domain: 'frontend.domain.de',
    marketCode: 'de',
    defaultLocale: 'de',
    countryName: 'Germany',
    locales: ['de'],
    showInSelector: false
  },
  {
    id: 2,
    domain: 'frontend.domain.com',
    marketCode: 'en',
    defaultLocale: 'en',
    countryName: 'US',
    locales: ['en'],
    showInSelector: true
  },
  {
    id: 3,
    domain: 'frontend.domain.ca',
    marketCode: 'ca',
    defaultLocale: 'ca',
    countryName: 'Canada',
    locales: ['en', 'fr'],
    showInSelector: true
  }
]

Middleware:

import createIntlMiddleware from 'next-intl/middleware'
import { locales } from '@/translations/locales'
import { domains } from '@/translations/domains'

export default async function middleware(request) {
  const handleI18nRouting = createIntlMiddleware({
    locales: locales,
    defaultLocale: 'de',
    domains: domains,
    localePrefix: 'always'
  })
  const response = handleI18nRouting(request)

  return response
}

export const config = {
  matcher: ['/', '/(de|en|ru|nl|fr|pl)/:path*']
} 

Caddyfile:

{
    email mail@domain.com
}
frontend.domain.de {
    encode zstd gzip
    reverse_proxy frontend:3000

    header {
        Strict-Transport-Security max-age=31536000;
        X-Content-Type-Options nosniff
        X-Frame-Options DENY
        Referrer-Policy no-referrer-when-downgrade
    }
}
frontend.domain.com {
    encode zstd gzip
    reverse_proxy frontend:3000

    header {
        Strict-Transport-Security max-age=31536000;
        X-Content-Type-Options nosniff
        X-Frame-Options DENY
        Referrer-Policy no-referrer-when-downgrade
    }
}
frontend.domain.ca {
    encode zstd gzip
    reverse_proxy frontend:3000

    header {
        Strict-Transport-Security max-age=31536000;
        X-Content-Type-Options nosniff
        X-Frame-Options DENY
        Referrer-Policy no-referrer-when-downgrade
    }
}

Expected behaviour

The port will not be appended to the domain if I enter the domain without language prefix.

@FNGR2911 FNGR2911 added bug Something isn't working unconfirmed Needs triage. labels Nov 23, 2023
@amannn
Copy link
Owner

amannn commented Nov 23, 2023

I unfortunately don't have any experience with Caddy. To look into this, it would be required that a reproduction is available that demonstrates incorrect behavior on the next-intl side.

We have quite an extensive test suite for the middleware. If you have a special use case, you can try to add a test there to see if the bug is caused by next-intl.

Copy link

Thank you for your report.

Unfortunately, the reproduction is missing or incomplete, and as such we cannot investigate this issue. Please add a reproduction to the issue, otherwise it will be closed automatically.

Templates:

Creating a good bug report takes time.

To help us resolve the issue quickly, please simplify the reproduction as much as possible by removing any unnecessary code, files, and dependencies that are not directly related to the problem. The easier it is for us to see the issue, the faster we can help you.

Apart from the reproduction, make sure to include the precise steps on how to reproduce the issue, e.g.:

  1. Open reproduction
  2. Click on …
  3. See error: …

Thank you for your understanding!

FAQ

"I've included a reproduction, what is missing?"

This comment might have been added because your reproduction doesn't point to either a CodeSandbox or a public GitHub repository where the issue can be reproduced.

Please make sure:

  1. If you've linked to a CodeSandbox, make sure to save the latest changes.
  2. If you've linked to a GitHub repository, make sure that you've committed and pushed the latest changes.
  3. Linking to other resources, e.g. to the docs, unfortunately doesn't help to reproduce your issue.

@7thkey
Copy link

7thkey commented Nov 24, 2023

This is happening to us using Docker Compose and using domains like es.midomain.com > mx.mydomain.com
To fix it temporarly we had to change the headers location in middleware

@DanWebb
Copy link

DanWebb commented Nov 24, 2023

I'm also getting this issue, v3.1.3, not using anything fancy like Docker Compose or Caddy.

The port is being appended at some point in the domain-based redirect

@FNGR2911
Copy link
Author

This is happening to us using Docker Compose and using domains like es.midomain.com > mx.mydomain.com To fix it temporarly we had to change the headers location in middleware

Do you have any example code how you did that @7thkey? Would be great, thank you!

@amannn
Copy link
Owner

amannn commented Nov 24, 2023

I think I found the issue. If tests pass in #662, this will be fixed in the next patch version.

@DanWebb
Copy link

DanWebb commented Nov 24, 2023

Nice!

In case it's still helpful, I was setting up a reproduction:

It's a bit awkward to reproduce as it needs two domains, but you can see the port there in the console-logged middleware response when going cy -> en.

@FNGR2911
Copy link
Author

I think I found the issue. If tests pass in #662, this will be fixed in the next patch version.

Thank you @amannn! Sounds great.

amannn added a commit that referenced this issue Nov 24, 2023
…en called from an internal address (e.g. from a proxy) (#662)

Fixes #658
@Quackzoer
Copy link

Was it fixed? I keep getting port added after upgrading next-intl package

@amannn
Copy link
Owner

amannn commented Jan 17, 2024

@Quackzoer Can you open a new issue with a reproduction and more details about your setup?

@awkaiser-tr
Copy link
Contributor

@amannn unfortunately the fix ...

if (redirectDomain) {
urlObj.protocol =
request.headers.get('x-forwarded-proto') ?? request.nextUrl.protocol;
urlObj.port = '';
urlObj.host = redirectDomain;
}
return NextResponse.redirect(urlObj.toString());

... breaks domain-based routing when Next.js isn't running on standard HTTP/HTTPS port, and isn't behind a proxy.

For instance, when running a dev server (port 3000 by default, but could be anything), http://us-yourcompany-dev.localhost:3000/en-US/foobar will be redirected to 💥 http://us-yourcompany-dev.localhost/foobar 💥 when localePrefix: 'as-needed'. Whenever redirectDomain has a truthy value, the port will always be excluded. That works fine for the reported proxy issues and most website deployments, but not for deliberate use of alternative ports.

This one test:

it('uses the correct port and protocol when being called from an internal address', () => {
middleware(
createMockRequest('/', 'fr', 'http://192.168.0.1:3000', undefined, {
'x-forwarded-host': 'ca.example.com',
'x-forwarded-proto': 'https'
})
);
expect(MockedNextResponse.next).not.toHaveBeenCalled();
expect(MockedNextResponse.rewrite).not.toHaveBeenCalled();
expect(MockedNextResponse.redirect.mock.calls[0][0].toString()).toBe(
'https://ca.example.com/fr'
);
});

... would benefit from another to ensure that, in the absence of x-forwarded-host,1 Next.js' port is preserved:

it('uses the correct port and protocol', () => { 
  middleware( 
    createMockRequest('/', 'fr', 'http://192.168.0.1:3000') 
  ); 
  expect(MockedNextResponse.next).not.toHaveBeenCalled(); 
  expect(MockedNextResponse.rewrite).not.toHaveBeenCalled(); 
  expect(MockedNextResponse.redirect.mock.calls[0][0].toString()).toBe( 
    'http://192.168.0.1:3000/fr' 
  ); 
});

Footnotes

  1. Regarding x-forwarded-* headers, there's no standard for communicating the forwarding proxy's port: some append it to x-forwarded-host, others set a separate x-forwarded-port header. The forwarded header offers a standard (RFC-7239) way to include additional details like the port.

@amannn
Copy link
Owner

amannn commented Apr 4, 2024

@awkaiser-tr Thanks for digging into this and proposing a solution! I agree that it might be reasonable to assume that if there are no x-forwarded-* headers, then Next.js isn't running behind a reverse proxy. Would you be interested in adding a PR with your test and a proposed fix?

@awkaiser-tr
Copy link
Contributor

@amannn sure! 🍻

amannn added a commit that referenced this issue Apr 5, 2024
…p doesn't run behind a proxy (#979 by @awkaiser-tr)

Fixes #658

----

# Contribution feedback

@amannn to preserve your desired formatting, this project would benefit
from an [EditorConfig](https://editorconfig.org/) or tool such as
[Prettier](https://prettier.io/)

---------

Co-authored-by: Jan Amann <jan@amann.me>
@pepijn-vanvlaanderen
Copy link

pepijn-vanvlaanderen commented Apr 8, 2024

@amannn interestingly enough, this 'fix' actually gives issues for us on Railway. Maybe a better solution would be to add it as a config somewhere.

@awkaiser-tr
Copy link
Contributor

@pepijn-vanvlaanderen there are multiple patches related to this issue, can you be specific:

  1. What version of next-intl are you using?
  2. What issues are you experiencing? What would the simplest reproduction steps be?

@pepijn-vanvlaanderen
Copy link

Hi @awkaiser-tr, thanks for your reply. I have opened an issue here: #987

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment