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

Add x-forward headers to external rewrites #17557

Merged
merged 4 commits into from
Jul 16, 2021
Merged

Add x-forward headers to external rewrites #17557

merged 4 commits into from
Jul 16, 2021

Conversation

thomasmarshall
Copy link
Contributor

I'd like to be able to use external rewrites to proxy requests to a Rails app. However, the changeOrigin: true proxy setting switches out the requested host for the target origin, which means the Rails app doesn't know it's being proxied to and will generate URLs and perform redirects to the wrong host.

For example: if I rewrite next.test/foo to rails.test/foo, any links or redirects on the proxied site will use the host rails.test instead of the requested next.test and break out of the proxy.

Rather than changing or removing the changeOrigin setting, this commit configures the proxy to include x-forward headers 1. Rails can then use the X-Forwarded-Host header to determine which host to use for URL generation and redirects 2.

@ijjk

This comment has been minimized.

@thomasmarshall
Copy link
Contributor Author

We're currently using an API route with a custom proxy to handle this:

// next.config.js

module.exports = {
  rewrites: async () => [
    {
      source: '/:path*',
      destination: '/:path*',
    },
    {
      source: '/:path*',
      destination: '/api/proxy/:path*',
    },
  ],
};
// pages/api/proxy/[[...path]].js

import { createProxyMiddleware } from 'http-proxy-middleware';

export default createProxyMiddleware({
  target: process.env.PROXY_HOST,
  changeOrigin: true,
  pathRewrite: { '^/api': '/' },
  xfwd: true,
});

@ijjk

This comment has been minimized.

@focux
Copy link

focux commented Dec 11, 2020

Any thoughts on this @timneutkens? My use case is similar to the OP's but in PHP.

@minusfive
Copy link

Yeah, would love to see this feature make it in soon — we're hacking around a similar use case. Any feedback @Timer , @lfades , @timneutkens ?

This commit configures the proxy used for external rewrites to include
x-forward headers [1]. This is particularly useful for incremental
adoption, where some routes will be handled by Next.js and others by a
different website. For example, a Rails app will use the
X-Forwarded-Host header to determine which host to use for URL
generation and redirects [2].

[1]: https://github.com/http-party/node-http-proxy/blob/91fee3e943dc4497e8dd4ef27116388dce091988/lib/http-proxy.js#L31
[2]: https://github.com/rails/rails/blob/41139f6ba27556e710ecad93f3cd241ea6764f9d/actionpack/lib/action_dispatch/http/url.rb#L221-L227
@ijjk

This comment has been minimized.

@vwatel
Copy link

vwatel commented Apr 24, 2021

Any update on this one?
After upgrading to version 10, we are missing lot of headers when being behind a Proxy such as Application Gateway on Azure.
For example, here are few HTTP headers that are not part of the request anymore:

  • "x-original-host": custom HTTP header set by Azure Application Gateway
  • "x-forwarded-host"
  • "cache-control"
  • etc

Thank you for your help 👍

@thomasmarshall
Copy link
Contributor Author

Hey @timneutkens @ijjk – is there any possibility of getting this reviewed? If so, I'll rebase and resolve the conflict – otherwise I can just close the PR.

@adrienharnay
Copy link
Contributor

Hello, I just noticed that the x-forwarded-for header does not contain the correct IP address while using the rewrites feature in next.config.js (e.g. for incremental adoption). Could someone please review this small PR?

@ijjk

This comment has been minimized.

ijjk
ijjk previously approved these changes Jul 16, 2021
Copy link
Member

@ijjk ijjk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

@ijjk

This comment has been minimized.

@ijjk
Copy link
Member

ijjk commented Jul 16, 2021

Stats from current PR

Default Build (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary thomasmarshall/next.js rewrite-proxy-xfwd Change
buildDuration 17.2s 16.8s -385ms
buildDurationCached 3.8s 4s ⚠️ +167ms
nodeModulesSize 49.4 MB 49.4 MB ⚠️ +369 B
Page Load Tests Overall decrease ⚠️
vercel/next.js canary thomasmarshall/next.js rewrite-proxy-xfwd Change
/ failed reqs 0 0
/ total time (seconds) 2.882 2.838 -0.04
/ avg req/sec 867.33 880.98 +13.65
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.715 1.763 ⚠️ +0.05
/error-in-render avg req/sec 1457.41 1418.44 ⚠️ -38.97
Client Bundles (main, webpack, commons)
vercel/next.js canary thomasmarshall/next.js rewrite-proxy-xfwd Change
359.HASH.js gzip 2.96 kB 2.96 kB
framework-HASH.js gzip 42 kB 42 kB
main-HASH.js gzip 20.9 kB 20.9 kB
webpack-HASH.js gzip 1.49 kB 1.49 kB
Overall change 67.4 kB 67.4 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary thomasmarshall/next.js rewrite-proxy-xfwd Change
polyfills-HASH.js gzip 31.1 kB 31.1 kB
Overall change 31.1 kB 31.1 kB
Client Pages
vercel/next.js canary thomasmarshall/next.js rewrite-proxy-xfwd Change
_app-HASH.js gzip 803 B 803 B
_error-HASH.js gzip 3.06 kB 3.06 kB
amp-HASH.js gzip 554 B 554 B
css-HASH.js gzip 329 B 329 B
hooks-HASH.js gzip 903 B 903 B
image-HASH.js gzip 5.6 kB 5.6 kB
index-HASH.js gzip 261 B 261 B
link-HASH.js gzip 1.66 kB 1.66 kB
routerDirect..HASH.js gzip 319 B 319 B
withRouter-HASH.js gzip 320 B 320 B
bb14e60e810b..30f.css gzip 125 B 125 B
Overall change 13.9 kB 13.9 kB
Client Build Manifests
vercel/next.js canary thomasmarshall/next.js rewrite-proxy-xfwd Change
_buildManifest.js gzip 417 B 417 B
Overall change 417 B 417 B
Rendered Page Sizes
vercel/next.js canary thomasmarshall/next.js rewrite-proxy-xfwd Change
index.html gzip 529 B 529 B
link.html gzip 543 B 543 B
withRouter.html gzip 523 B 523 B
Overall change 1.59 kB 1.59 kB

Webpack 4 Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary thomasmarshall/next.js rewrite-proxy-xfwd Change
buildDuration 14s 13.6s -422ms
buildDurationCached 5.7s 5.3s -376ms
nodeModulesSize 49.4 MB 49.4 MB ⚠️ +369 B
Page Load Tests Overall increase ✓
vercel/next.js canary thomasmarshall/next.js rewrite-proxy-xfwd Change
/ failed reqs 0 0
/ total time (seconds) 3.044 3.056 ⚠️ +0.01
/ avg req/sec 821.2 817.95 ⚠️ -3.25
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.852 1.756 -0.1
/error-in-render avg req/sec 1349.69 1423.58 +73.89
Client Bundles (main, webpack, commons)
vercel/next.js canary thomasmarshall/next.js rewrite-proxy-xfwd Change
14.HASH.js gzip 2.98 kB 2.98 kB
677f882d2ed8..HASH.js gzip 13.7 kB 13.7 kB
framework.HASH.js gzip 41.8 kB 41.8 kB
main-HASH.js gzip 8.39 kB 8.39 kB
webpack-HASH.js gzip 1.19 kB 1.19 kB
Overall change 68 kB 68 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary thomasmarshall/next.js rewrite-proxy-xfwd Change
polyfills-HASH.js gzip 31.3 kB 31.3 kB
Overall change 31.3 kB 31.3 kB
Client Pages
vercel/next.js canary thomasmarshall/next.js rewrite-proxy-xfwd Change
_app-HASH.js gzip 791 B 791 B
_error-HASH.js gzip 3.76 kB 3.76 kB
amp-HASH.js gzip 552 B 552 B
css-HASH.js gzip 333 B 333 B
hooks-HASH.js gzip 910 B 910 B
index-HASH.js gzip 230 B 230 B
link-HASH.js gzip 1.64 kB 1.64 kB
routerDirect..HASH.js gzip 297 B 297 B
withRouter-HASH.js gzip 293 B 293 B
e025d2764813..52f.css gzip 125 B 125 B
Overall change 8.93 kB 8.93 kB
Client Build Manifests
vercel/next.js canary thomasmarshall/next.js rewrite-proxy-xfwd Change
_buildManifest.js gzip 420 B 420 B
Overall change 420 B 420 B
Rendered Page Sizes
vercel/next.js canary thomasmarshall/next.js rewrite-proxy-xfwd Change
index.html gzip 576 B 576 B
link.html gzip 590 B 590 B
withRouter.html gzip 569 B 569 B
Overall change 1.74 kB 1.74 kB
Commit: 8810ba7

@ijjk ijjk merged commit ccb62f8 into vercel:canary Jul 16, 2021
flybayer pushed a commit to blitz-js/next.js that referenced this pull request Aug 19, 2021
* Add x-forward headers to external rewrites

This commit configures the proxy used for external rewrites to include
x-forward headers [1]. This is particularly useful for incremental
adoption, where some routes will be handled by Next.js and others by a
different website. For example, a Rails app will use the
X-Forwarded-Host header to determine which host to use for URL
generation and redirects [2].

[1]: https://github.com/http-party/node-http-proxy/blob/91fee3e943dc4497e8dd4ef27116388dce091988/lib/http-proxy.js#L31
[2]: https://github.com/rails/rails/blob/41139f6ba27556e710ecad93f3cd241ea6764f9d/actionpack/lib/action_dispatch/http/url.rb#L221-L227

* Handle image-optimizer case

Co-authored-by: JJ Kasper <[email protected]>
@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants