search

pillarjs / send

Streaming static file server with Range and conditional-GET support
MIT License
796 stars 188 forks source link

Keep CSP on 304 #204

Closed felipeochoa closed 2 years ago

felipeochoa commented 3 years ago

I'd like to reopen #126 since the standard has been clarified since the last discussion. Specifically, it seems sending a CSP on a 304 response is now allowed. See web-platform-tests/wpt#13579 for a concrete demonstration that CSP on 304 is valid, and httpwg/http-core#337 for the full discussion.

Beyond just CSP, the blacklisting of content-* seems to have been replaced with a more targeted list for caches to ignore, so it might be worth updating that filter function more comprehensively.

dougwilson commented 3 years ago

Hi @felipeochoa , and welcome back! Thank you so much for following up on that. I see the web platform test there, but the http-core link I'm not sure how exactly it is related. But even then, it seems like the actual standard has not yet been changed, has it? I took a look at RFC 7232 and I don't see any changes. Can you point me to the new/updated standard? In particular the main issue with CSP is the nonce header (as I mentioned in the previous issue) -- how did they resolve that, as I don't see it mentioned anywhere?

dougwilson commented 3 years ago

I am trying to read over the tests you linked (https://github.com/web-platform-tests/wpt/pull/13579/files) and, if I'm reading it right, it adds a test to send CSP headers in a 304 and then it validates that the new CSP policy from the 304 is not actually used, am I interpreting that right?

    var t1 = async_test("Test that the first frame uses nonce abc");
    var t2 = async_test("Test that the first frame does not use nonce def");

The first frame is a 200 with the abc nonce and the second frame is a new CSP in a 304 with the def nonce, but the test states the second (304 one) should be ignored, right?

dougwilson commented 3 years ago

Oh, I see. The first two both say first frame and the second pair says second frame. My bad. So yea, that test I believe does indeed check that the CSP in a 304 is updated by the web browser.

dougwilson commented 3 years ago

I have determined the location for the spec change, which is located in the published httpbis-cache https://datatracker.ietf.org/doc/draft-ietf-httpbis-cache/16/

felipeochoa commented 3 years ago

Sorry for the delay! Somehow disabled notifications here.

I'm still not great at navigating the spec, but basically what I gathered is that before the update, all Content-* headers were ignored on 304s, but with this update the list of ignored headers is now more tightly scoped. httpwg/http-core#165 might be the more informative issue, though a bit of a longer read.

I think the key section in the spec you noted is 3.2, especially where they say:

Note that the Content-* prefix is not a signal that a header field is omitted from update; it is a convention for MIME header fields, not HTTP.

felipeochoa commented 2 years ago

Thank you!