Add fake headers to URLRequestRedirectJobs.

Add fake headers to URLRequestRedirectJobs.

Plugin logic expects redirects to have headers, and gets very confused
when they're missing.

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

[davidben, 2014-08-05 19:44:02]

lgtm. Though it's apparently still not the case that every redirect has headers.
Some random other URLRequestJobs like URLRequestFileJob, for instance, will
trigger redirects. I wonder if we should just make plugins reject those since
they're not normal jobs, or perhaps wait until the RedirectInfo CL and pull it
out of there.

+battre from chrome/browser/extensions/api/web_request/OWNERS as FYI that this
slightly changes web_request API behavior when there's, say, an HSTS redirect.

[mmenke, 2014-08-05 20:24:54]

On 2014/08/05 19:44:02, David Benjamin wrote:
> lgtm. Though it's apparently still not the case that every redirect has
headers.
> Some random other URLRequestJobs like URLRequestFileJob, for instance, will
> trigger redirects. I wonder if we should just make plugins reject those since
> they're not normal jobs, or perhaps wait until the RedirectInfo CL and pull it
> out of there.

You're right - in fact, a pretty significant number of internal request types
can do that.  Looks to me like file:// URLs are probably the only requests that
plugins can make that can be redirects, but I could be wrong about that.  We
should consider either making plugins headerless redirect safe, or updating the
URLRequest API so that if you want to force a redirect, you can only do so by
providing bogus headers.  I lean a bit towards the latter.  It'll be a bit of a
pain, but it's more robust against regressions.

> +battre from chrome/browser/extensions/api/web_request/OWNERS as FYI that this
> slightly changes web_request API behavior when there's, say, an HSTS redirect.

[mmenke, 2014-08-05 20:27:22]

[+battre] Please review the webrequest changes (JS test files only).

[battre, 2014-08-06 07:54:25]

WebRequest changes LGTM, but I have one comment that you can resolve with the
network stack team.

Best regards,
Dominic

https://codereview.chromium.org/422233006/diff/120001/net/url_request/url_req...
File net/url_request/url_request_redirect_job.cc (right):

https://codereview.chromium.org/422233006/diff/120001/net/url_request/url_req...
net/url_request/url_request_redirect_job.cc:88: "Non-Authoritative-Reason: %s",
According to http://en.wikipedia.org/wiki/List_of_HTTP_status_codes, response
code 307 was only added in HTTP/1.1.

[mmenke, 2014-08-06 14:57:01]

Thanks!

https://codereview.chromium.org/422233006/diff/120001/net/url_request/url_req...
File net/url_request/url_request_redirect_job.cc (right):

https://codereview.chromium.org/422233006/diff/120001/net/url_request/url_req...
net/url_request/url_request_redirect_job.cc:88: "Non-Authoritative-Reason: %s",
On 2014/08/06 07:54:25, battre wrote:
> According to http://en.wikipedia.org/wiki/List_of_HTTP_status_codes, response
> code 307 was only added in HTTP/1.1.

Done.  For some reason, was thinking I'd have to have a Content-Length if it was
HTTP/1.1, but skimming over RFC 2616, doesn't look like that's the case.

[mmenke, 2014-08-06 14:57:05]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2014-08-06 14:58:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mmenke@chromium.org/422233006/160001

[commit-bot: I haz the power, 2014-08-06 19:11:29]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[mmenke, 2014-08-06 19:36:31]

The CQ bit was unchecked by mmenke@chromium.org

[mmenke, 2014-08-06 19:37:02]

On 2014/08/06 19:11:29, I haz the power (commit-bot) wrote:
> FYI, CQ is re-trying this CL (attempt #1).
> The failing builders are:
>   linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
>
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
>   linux_chromium_rel_swarming on tryserver.chromium.linux
>
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
>   mac_chromium_rel on tryserver.chromium.mac
>
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)
>   win_chromium_rel on tryserver.chromium.win
>
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
>   win_chromium_x64_rel on tryserver.chromium.win
>
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

Oops...forgot to save one of those JS files, after updating the response line it
expects.

[mmenke, 2014-08-06 19:41:16]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2014-08-06 19:43:02]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mmenke@chromium.org/422233006/180001

[commit-bot: I haz the power, 2014-08-06 23:45:21]

Change committed as 287901