Escape non-ASCII response headers.

Escape non-ASCII response headers.

Response headers that are not valid UTF-8 can't be stored in a
base::StringValue, which is always a UTF-8 string, so escape any non-ASCII
values to be safe. This prevents a DCHECK when dev tools' network request
inspector is open and we receive these headers.

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

[Elly Fong-Jones, 2014-08-07 17:09:30]

PTAL?

[davidben, 2014-08-07 17:36:36]

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

https://codereview.chromium.org/453483002/diff/1/net/http/http_response_heade...
net/http/http_response_headers.cc:1362: base::StringPrintf("%s: %s",
escaped_name.c_str(),
Header names and values are "supposed to be" treated as latin1, so doing a
latin1 to utf8 might be better. Looks like we have a net/base/net_string_util.h
which defines a net::ConvertToUtf8 and kCharsetLatin1. There's other code in
net/ which uses ConvertToUtf8AndNormalize, so that seems fine.

[Elly Fong-Jones, 2014-08-07 19:41:37]

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

https://codereview.chromium.org/453483002/diff/1/net/http/http_response_heade...
net/http/http_response_headers.cc:1362: base::StringPrintf("%s: %s",
escaped_name.c_str(),
On 2014/08/07 17:36:36, David Benjamin wrote:
> Header names and values are "supposed to be" treated as latin1, so doing a
> latin1 to utf8 might be better. Looks like we have a
net/base/net_string_util.h
> which defines a net::ConvertToUtf8 and kCharsetLatin1. There's other code in
> net/ which uses ConvertToUtf8AndNormalize, so that seems fine.

Acknowledged.

[davidben, 2014-08-07 19:42:42]

(Per in-person conversation, escaping lgtm too. I have a minor preference for
latin-1 for this showing up in DevTools, but whatever. Sending non-latin-1
characters in headers is pretty wonky whatever you do.)

[Elly Fong-Jones, 2014-08-07 19:42:56]

The CQ bit was checked by ellyjones@chromium.org

[commit-bot: I haz the power, 2014-08-07 19:46:52]

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

[commit-bot: I haz the power, 2014-08-08 01:09:00]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tri...)

[commit-bot: I haz the power, 2014-08-08 01:31:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-08 01:31:29]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tri...)

[Elly Fong-Jones, 2014-08-08 14:29:14]

The CQ bit was checked by ellyjones@chromium.org

[commit-bot: I haz the power, 2014-08-08 14:30:21]

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

[commit-bot: I haz the power, 2014-08-08 14:37:21]

Change committed as 288357