-
Notifications
You must be signed in to change notification settings - Fork 437
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
v8.0.0-beta1: Preloader: Sec-Purpose
header is ignored by fetch(..)
#1107
Comments
As discussed in #1101 the In the Fetch Standard under the "Forbidden Header Name" section, the following paragraph explicitly mentions that any header starting with
The linked spec also mentions why these are reserved, even in the future:
The current implementation in Turbo 8 is a no-op. |
It would be great to have this addressed. Could you guys please have a look? Ping @rik @hey-leon @seanpdoyle @davidalejandroaguilar @afcapel |
Hey @psyipm I am not part of any team on the Turbo repository but I did read the PR body and elaborating comment and it seems pretty clear cut to me. If its explicitly called out that |
Fixed in #1108 We're now using |
Previously the header for indicating that a resource was preloaded was named
VND.PREFETCH
. The dot in the header name caused it to be stripped by reverse proxies, including CloudFlare, as discussed in #924It was discussed that it should be renamed, originally the PR I made in #925 suggested
x-purpose
. However, it was suggested that the header should instead be namedsec-purpose
to match a newer spec. This was done in #925 and merged and released as part of v8 beta.However, it seems that
fetch(..)
- at least in Chrome - strips this header from the request being made.To reproduce, fire a manual
fetch(..)
from the console:Inspecting the request headers, the
sec-purpose
is not sent, while thex-something
is:I guess the
sec-...
are reserved headers and that fetch strips these out before sending.I propose that we rename the header to
x-purpose
which seems to be the header used by Safari and Chrome (for the preview on the launch pages), as well as Facebook in-app browser. I'll wrap up a PR.The text was updated successfully, but these errors were encountered: