Route request headers are forwarding to backend fetch calls.

[nhunzaker]

Describe the bug

Request headers for the initial HTML page load are getting passed through to external fetch requests on the backend. For example, if http://localhost:3000 is requested with the Accept: text/html header, that gets passed along to my API requests.

This occurs when using @sveltejs/node-adapter@1.0.0-next.78. I've included a reproduction, but here are some more details:

I'm seeing this in my server logs within my API, however it is also observable in the externalFetch svelte-kit hook:

// hooks.js
export function externalFetch(request) {
  console.log(request.headers) // <-- this has request headers from my svelte-kit route!
  return fetch(request)
}

This is unexpected and causes some wonky interactions with our API. We've hit this in two places:

1. A client passes through Accept: text/html to our API endpoint, which results in our JSON API responding with a 406 Not Acceptable error. This is fine 🤷, but pretty weird and an annoying class of errors to filter through in error tracking software.
2. More frustrating: a client passes through a If-Modified-Since header, our API returns a 304 Not Modified response, which results in a user facing error when fetch on the backend considers the request a failure:

let request = await fetch("https://myapi.com/thing.json")

console.log(request.status) // 304

if (request.ok) { // this is false server-side
  // ...
} 

We'd expect these headers to not get automatically passed through. Those headers are for the SvelteKit request, not our API.

As a temporary work around, we're removing these headers using the externalFetch hook, but I think that this should not be necessary.

Reproduction

I have provided a reproducing project here:
https://github.com/nhunzaker/sveltekit-reproduction

You can reproduce this issue by running the project with:

npm install
npm run dev

Then cURL the request with the Accept header:

curl --header "Accept: text/html" http://localhost:3000

When running the example, you'll see the request come through with the Accept header sent along with the request for http://localhost:3000:

External Fetch:
https://www.gov.uk/bank-holidays.json
Accept Header: text/html

(UK Bank holidays are not related to this issue, they simply have a public JSON API)

Logs

No response

System Info

System:
    OS: Linux 5.10 Ubuntu 20.04.4 LTS (Focal Fossa)
    CPU: (16) x64 AMD Ryzen 7 5800X 8-Core Processor
    Memory: 3.99 GB / 7.72 GB
    Container: Yes
    Shell: 3.1.0 - /usr/bin/fish
  Binaries:
    Node: 16.13.0 - ~/.asdf/installs/nodejs/16.13.0/bin/node
    Yarn: 1.22.17 - ~/.asdf/installs/nodejs/16.13.0/.npm/bin/yarn
    npm: 8.1.0 - ~/.asdf/plugins/nodejs/shims/npm
  npmPackages:
    @sveltejs/adapter-node: 1.0.0-next.78 => 1.0.0-next.78
    @sveltejs/kit: 1.0.0-next.350 => 1.0.0-next.350
    svelte: ^3.44.1 => 3.48.0

Severity

serious, but I can work around it

Additional Information

No response

[nhunzaker]

Well shoot. Now that I've submitted this, I just saw #5195. It sounds like there's already been quite a bit of conversation about this.

Hopefully this issue, and explanation of why this is surprising and annoying, is helpful.

[Conduitry]

#5195 is almost certainly not going to happen in its current form, but yes we do either need to be smarter about which headers get carried over, and probably also give more control over that.

[nhunzaker]

We do either need to be smarter about which headers get carried over, and probably also give more control over that.

I would want an option to completely disable this behavior, and I think it should be opt-in. Edit: Maybe this could be a list of allowed headers?

This makes the behavior of fetch inconsistent across server and client executing code. The same fetch calls executed in the browser do not pass through Accept, User-Agent, or other headers from the initial request.

Also - I don't want to be a pain and bog you all down rehashing this decision. I disagree with it, but I'd also just be okay with a way to disable this behavior for my app. It would have also been helpful to have some documentation (and justification) about this for folks like me totally surprised when unexpected headers cause problems.

[itssumitrai]

#5195 is almost certainly not going to happen in its current form, but yes we do either need to be smarter about which headers get carried over, and probably also give more control over that.

I believe sveltekit's fetch should have a consistent behavior with the global fetch atleast in this aspect. Application should be able to pass headers if they choose to do it, sometimes its completely valid & security aspect is handled on the receiving host. Its good to provide defaults but not good to completely disallow a valid and consistent behavior from public api.