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

Blacklist fetch response headers when serialised for initial fetch #1971

Closed
tanhauhau opened this issue Jul 20, 2021 · 2 comments · Fixed by #6569
Closed

Blacklist fetch response headers when serialised for initial fetch #1971

tanhauhau opened this issue Jul 20, 2021 · 2 comments · Fixed by #6569
Labels
breaking change bug Something isn't working fetch-headers issues relating to server-side `fetch` headers p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.
Milestone

Comments

@tanhauhau
Copy link
Member

Describe the problem

when serialise the fetch response into <script type="application/json" data-type="svelte-data">, it includes the fetch response header, which includes headers that is always changed:

  • Date

which makes the html content hash different, even though the api response body and svelte rendered content is the same

Describe the proposed solution

blacklist some of the response header when serialised

Alternatives considered

use serverFetch hook to overwrite fetch behavior on server, and remove the Date response header

Importance

would make my life easier

Additional Information

No response

@benmccann benmccann added this to the 1.0 milestone Jul 20, 2021
@benmccann benmccann added the bug Something isn't working label Jul 20, 2021
@benmccann benmccann added the p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. label Aug 4, 2021
@benmccann
Copy link
Member

benmccann commented Aug 5, 2021

Blacklisting might be a bit brittle. E.g. Google adds X-Cloud-Trace-Context which is also a unique header

Unfortunately, the alternative of using serverFetch doesn't work because it only is for external fetches (there's a PR out to rename it to externalFetch to make that clearer #2110)

@benmccann
Copy link
Member

I found an old message in Discord where you mentioned this with more detail:

i noticed for sveltekit, that the calculation of etag is based on the response body,
which includes the serialised response from fetch, which one of the headers is Date, which will never be the same.
so i never got the same etag if i have fetch, tho the response body and rendered html is exactly the same

@benmccann benmccann added p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc. and removed pending clarification p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. labels Aug 5, 2021
@benmccann benmccann removed this from the 1.0 milestone Aug 5, 2021
@Rich-Harris Rich-Harris added this to the 1.0 milestone Mar 5, 2022
@Rich-Harris Rich-Harris added breaking change fetch-headers issues relating to server-side `fetch` headers labels Aug 27, 2022
Rich-Harris added a commit that referenced this issue Sep 5, 2022
* refactor

* add filterSerializedResponseHeaders option - closes #1971

* Update .changeset/silent-cycles-reply.md

Co-authored-by: Conduitry <git@chor.date>

* Update documentation/docs/06-hooks.md

Co-authored-by: Conduitry <git@chor.date>

* throw error if excluded header is read during SSR

* argh

* fix

Co-authored-by: Conduitry <git@chor.date>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change bug Something isn't working fetch-headers issues relating to server-side `fetch` headers p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.
Projects
None yet
3 participants