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

Implement page.host #207

Closed
Rich-Harris opened this issue Dec 1, 2020 · 6 comments
Closed

Implement page.host #207

Rich-Harris opened this issue Dec 1, 2020 · 6 comments

Comments

@Rich-Harris
Copy link
Member

As per sveltejs/sapper#735. This is a way to provide the host to preload functions in a uniform way between server and client.

Wrinkles:

  • Do we pass host to endpoints as well as pages, even though they could get the same information from headers?
  • How do we pre-configure a host for static export? (This is relevant to all adapters that prerender, not just adapter-static. This is also an argument for passing it to endpoints rather than expecting them to derive it themselves)
  • Do we use headers['x-forwarded-for'] where available? Express will only do this if the trust proxy setting is enabled — relevant tests here.
@Rich-Harris Rich-Harris added this to the public beta milestone Dec 4, 2020
@benmccann
Copy link
Member

I've had to deal with this in the past and I remember deciding that x-forwarded-for was not trust-worthy enough for my needs and just created a config value in my server as there did not seem to be any reliable way to do it on the server-side.

My thought for this would be to have the adapter pass the underlying request object. Then the user could handle this and any other cases we haven't covered yet in a manner of their choosing.

@Rich-Harris
Copy link
Member Author

have the adapter pass the underlying request object

Really don't want to go down that road.

What was the problem with x-forwarded-for? Presumably a provided value is preferable to something you have to configure, since preview URLs (like the ones you get in pull requests for projects hosted on Vercel, etc) are unpredictable.

@benmccann
Copy link
Member

I didn't envision the underlying request object being a frequently used thing, but only as an escape hatch

Regarding x-forwarded-for, the page you linked says:

NOTE: X-Forwarded-* headers are easily spoofed and the detected IP addresses are unreliable.

Express does not provide these headers by default as a result. I think we open ourselves up to people using these headers in unsafe ways without realizing that there can be issues with doing so. It's also not available in all environments because it requires load balancer configuration, which may lead to confusion. I would be okay with the idea if it were off by default and enabled with a config setting like in Express, so that people don't accidentally shoot themselves in the foot.

@Rich-Harris
Copy link
Member Author

Yeah, I'm not imagining it would be on by default — am thinking maybe something like this:

// svelte.config.js
module.exports = {
  adapter: '@adapter-node',
  hostHeader: 'x-forwarded-for' // defaults to 'host', but you can change it if you trust the proxy
};

We might also need to make it overridable for the sake of static exports:

// svelte.config.js
module.exports = {
  adapter: '@adapter-node',
  host: process.env.HOST // falls back to hostHeader if unspecified
};

@benmccann
Copy link
Member

That makes sense to me. I like the idea of making it overidable. That would also support non-exported sites where the user doesn't have control to set that flag on the load balancer

@Rich-Harris
Copy link
Member Author

closed via #259

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants