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

Why remove Content-Security-Policy header? #126

Closed
felipeochoa opened this issue Jan 10, 2017 · 6 comments
Closed

Why remove Content-Security-Policy header? #126

felipeochoa opened this issue Jan 10, 2017 · 6 comments
Assignees
Labels

Comments

@felipeochoa
Copy link

Is there a reason for removing the CSP header? I'm using this module to serve the script for a WebWorker and need to keep the CSP header on there to allow the worker to make XHR calls.

This header is removed in removeContentHeaderFields. It looks like a farily generic check, so I'm not even sure if you're removing this header on purpose.

@dougwilson
Copy link
Contributor

The module is trying to follow what is specified for generating a 304 https://tools.ietf.org/html/rfc7232#section-4.1

@dougwilson
Copy link
Contributor

We should dig around through the RFCs to see what should happen w.r.t that header and a 304.

@felipeochoa
Copy link
Author

felipeochoa commented Jan 10, 2017 via email

@dougwilson
Copy link
Contributor

Thanks, that helped a lot! From reading that, it sounds like there is nothing to change here. Even the fetch client specifies what this module is doing is correct. You can see when reading through the specs linked to from the lost recent comment that the steps in stage 21 is (1) load the cached response (2) if cannot load, network error (3) take any header from the network response and update the cached header.

This means that, just like it says in the http rfc, not including the header is not incorrect and simply means the browser will see the header value as it was I the 200 response it loaded up from it's cache response.

Both the http spec and that fetch spec are in agreement with as implemented here with the added statement that the http spec calls out that the server SHOULD not send any extra headers if possible, which is what we're doing.

@dougwilson
Copy link
Contributor

Sorry, didn't mean to close, this is still in discussion.

@dougwilson dougwilson reopened this Jan 10, 2017
@dougwilson dougwilson self-assigned this Feb 12, 2017
@dougwilson
Copy link
Contributor

So additional reading it looks like this module is behaving perfectly fine according to the specs and the only real issue in web browsers today with CSP + 304 is that if you're using a nonce in the CSP, if you send the CSP header with the 304 in some browsers, everything will break, so because of that bug, you actually do not want to send the CSP header with the 304, this is because you'll send a new nonce, yet because the content was loaded from the browser cache, the nonce will no longer match.

@pavelkornev pavelkornev mentioned this issue Aug 7, 2018
@pillarjs pillarjs locked and limited conversation to collaborators Aug 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants