Add to the list of HTTP headers that don't overwrite in 304 responses.

Add to the list of HTTP headers that don't overwrite in 304 responses.

Entity headers, such as those prefixed by 'Content-', 'X-WebKit-', and
'X-Content-', should only be accepted on the original response, not on 304
responses for the same resource.

This patch adds some specific headers ('X-XSS-Protection', 'X-Frame-Options'),
and adds support for prefixes we should ignore on these response.

BUG=174301
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=180724

[Mike West, 2013-02-05 11:21:12]

Hi Jochen! Would you mind taking a look at this patch? Once you're happy with
it, I'll grab someone like agl@ for OWNERS review.

FYI: abarth@, tsepez@, this patch is intended to address the Chromium side of
https://bugs.webkit.org/show_bug.cgi?id=71851.

[jochen (gone - plz use gerrit), 2013-02-05 12:46:03]

pure happiness, please go ahead!

[Mike West, 2013-02-05 12:52:31]

On 2013/02/05 12:46:03, jochen wrote:
> pure happiness, please go ahead!

You are evidently quite easily pleased.

agl@: Would you mind taking a look at this patch? The goal is simply to add a
few classes of header to the blacklist in ShouldUpdateHeader.

Thanks!

[Mike West, 2013-02-05 13:39:34]

On 2013/02/05 12:52:31, Mike West (chromium) wrote:
> On 2013/02/05 12:46:03, jochen wrote:
> > pure happiness, please go ahead!
> 
> You are evidently quite easily pleased.
> 
> agl@: Would you mind taking a look at this patch? The goal is simply to add a
> few classes of header to the blacklist in ShouldUpdateHeader.
> 
> Thanks!

Sorry, wrong link in the first comment: should have pointed to
https://bugs.webkit.org/show_bug.cgi?id=72414 for discussion of why I ended up
with this list of additions.

[agl, 2013-02-05 14:41:55]

lgtm

https://codereview.chromium.org/12224008/diff/1/net/http/http_response_header...
File net/http/http_response_headers.cc (right):

https://codereview.chromium.org/12224008/diff/1/net/http/http_response_header...
net/http/http_response_headers.cc:89: const char* const
kNonUpdatedHeaderPrefixes[] = {
Minor sadness at the extra indirection and relocation records here.

const char kNonUpdatedHeaderPrefixes[][12] = {
   ...
}

Would save all that, but it goes against the format of all the other lists in
this file. So I probably wouldn't change it, but it's something to consider.

https://codereview.chromium.org/12224008/diff/1/net/http/http_response_header...
net/http/http_response_headers.cc:102: if
(StartsWithASCII(std::string(name_begin, name_end),
Yet more sadness for there not being a StartsWithASCII that takes a StringPiece
or iterator pair.

[Mike West, 2013-02-05 15:59:39]

Thank, Adam. I'll throw this in the queue as-is, and I've filed bugs to follow
up on the bits you've noted as being sub-awesome.

https://codereview.chromium.org/12224008/diff/1/net/http/http_response_header...
File net/http/http_response_headers.cc (right):

https://codereview.chromium.org/12224008/diff/1/net/http/http_response_header...
net/http/http_response_headers.cc:89: const char* const
kNonUpdatedHeaderPrefixes[] = {
On 2013/02/05 14:41:55, agl wrote:
> Minor sadness at the extra indirection and relocation records here.
> 
> const char kNonUpdatedHeaderPrefixes[][12] = {
>    ...
> }
> 
> Would save all that, but it goes against the format of all the other lists in
> this file. So I probably wouldn't change it, but it's something to consider.

Filed https://code.google.com/p/chromium/issues/detail?id=174346. If, upon
consideration, you want me to update this and other lists in this file, I'm
happy to. Just assign the bug to me, or close it if you change your mind. :)

https://codereview.chromium.org/12224008/diff/1/net/http/http_response_header...
net/http/http_response_headers.cc:102: if
(StartsWithASCII(std::string(name_begin, name_end),
On 2013/02/05 14:41:55, agl wrote:
> Yet more sadness for there not being a StartsWithASCII that takes a
StringPiece
> or iterator pair.

Indeed. I looked briefly at adding such a beast, but it's nontrivial. Filed
https://code.google.com/p/chromium/issues/detail?id=174347. Can you recommend
some folks to CC on that bug?

[commit-bot: I haz the power, 2013-02-05 16:04:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkwst@chromium.org/12224008/1

[commit-bot: I haz the power, 2013-02-05 18:10:30]

Change committed as 180724