Bypass data reduction proxy if no handler is found for a 407 error.

Bypass data reduction proxy if no handler is found for a 407 error.

Bypass data reduction proxy if we get a 407 without a Proxy-Authenticate
header. This should occur only when the headers have been stripped
by middle servers.

BUG=371523

[bengr, 2014-06-05 17:25:46]

On 2014/06/05 17:09:04, kundaji wrote:

The description isn't quite right. How about:

Bypass data reduction proxy if 407 is missing a challenge.

Bypass the data reduction proxy if a 407 is received without
a Proxy-Authenticate header. This should occur only when the headers
have been stripped by an intermediary.

[bengr, 2014-06-05 17:26:24]

https://codereview.chromium.org/314173002/diff/1/components/data_reduction_pr...
File components/data_reduction_proxy/common/data_reduction_proxy_headers.cc
(right):

https://codereview.chromium.org/314173002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/common/data_reduction_proxy_headers.cc:126: &&
!headers->EnumerateHeader(NULL, "Proxy-Authenticate", &dummy)) {
Use headers->HasHeader("Proxy-Authenticate").

https://codereview.chromium.org/314173002/diff/1/net/proxy/proxy_service.h
File net/proxy/proxy_service.h (right):

https://codereview.chromium.org/314173002/diff/1/net/proxy/proxy_service.h#ne...
net/proxy/proxy_service.h:295: // Bypass the proxy because we got a 407 from the
proxy.
More specifically, "Bypass the proxy because we got a 407 from the proxy that
did not include a challenge."

Leave out the name of the header because we are likely moving from
Proxy-Authenticate to Chrome-Proxy.

Also, you could call this a MALFORMED_407_BYPASS. Up to you.

https://codereview.chromium.org/314173002/diff/1/tools/metrics/histograms/his...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/314173002/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:33639: +  <int value="6" label="Bypass
due to 407 response from proxy"/>
"Bypass due to 407 response from proxy lacking a challenge"

[Not at Google. Contact bengr, 2014-06-05 17:51:53]

https://codereview.chromium.org/314173002/diff/1/components/data_reduction_pr...
File components/data_reduction_proxy/common/data_reduction_proxy_headers.cc
(right):

https://codereview.chromium.org/314173002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/common/data_reduction_proxy_headers.cc:126: &&
!headers->EnumerateHeader(NULL, "Proxy-Authenticate", &dummy)) {
On 2014/06/05 17:26:24, bengr1 wrote:
> Use headers->HasHeader("Proxy-Authenticate").

Done.

https://codereview.chromium.org/314173002/diff/1/net/proxy/proxy_service.h
File net/proxy/proxy_service.h (right):

https://codereview.chromium.org/314173002/diff/1/net/proxy/proxy_service.h#ne...
net/proxy/proxy_service.h:295: // Bypass the proxy because we got a 407 from the
proxy.
On 2014/06/05 17:26:24, bengr1 wrote:
> More specifically, "Bypass the proxy because we got a 407 from the proxy that
> did not include a challenge."
> 
> Leave out the name of the header because we are likely moving from
> Proxy-Authenticate to Chrome-Proxy.
> 
> Also, you could call this a MALFORMED_407_BYPASS. Up to you.

Done.

https://codereview.chromium.org/314173002/diff/1/tools/metrics/histograms/his...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/314173002/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:33639: +  <int value="6" label="Bypass
due to 407 response from proxy"/>
On 2014/06/05 17:26:24, bengr1 wrote:
> "Bypass due to 407 response from proxy lacking a challenge"

Done.

[bengr, 2014-06-05 18:02:21]

Please add tests. Add asvitkine@ as a reviewer for the histogram and mef@ as a
reviewer for net/.

https://codereview.chromium.org/314173002/diff/20001/components/data_reductio...
File components/data_reduction_proxy/common/data_reduction_proxy_headers.cc
(right):

https://codereview.chromium.org/314173002/diff/20001/components/data_reductio...
components/data_reduction_proxy/common/data_reduction_proxy_headers.cc:125: &&
!headers->HasHeader("Proxy-Authenticate")) {
nit: I tend to put the && on the previous line.

[Alexei Svitkine (slow), 2014-06-05 19:51:54]

histograms lgtm

[Not at Google. Contact bengr, 2014-06-05 20:20:34]

bengr: components/data_reduction_proxy/common/*
asvitkine: tools/metrics/histograms*
mef: net/*

[Not at Google. Contact bengr, 2014-06-05 20:21:23]

https://codereview.chromium.org/314173002/diff/20001/components/data_reductio...
File components/data_reduction_proxy/common/data_reduction_proxy_headers.cc
(right):

https://codereview.chromium.org/314173002/diff/20001/components/data_reductio...
components/data_reduction_proxy/common/data_reduction_proxy_headers.cc:125: &&
!headers->HasHeader("Proxy-Authenticate")) {
On 2014/06/05 18:02:21, bengr1 wrote:
> nit: I tend to put the && on the previous line.

Done.

[mef, 2014-06-05 20:23:27]

net/* lgtm

[bengr, 2014-06-05 20:37:22]

lgtm, with nit.

https://codereview.chromium.org/314173002/diff/40001/components/data_reductio...
File components/data_reduction_proxy/common/data_reduction_proxy_headers.cc
(right):

https://codereview.chromium.org/314173002/diff/40001/components/data_reductio...
components/data_reduction_proxy/common/data_reduction_proxy_headers.cc:124: if
(headers->response_code() == net::HTTP_PROXY_AUTHENTICATION_REQUIRED &&
Add a TODO that we should also report that the response is malformed if the
response contains a Proxy-Authenticate header value that can't be interpreted by
the data reduction proxy.

[Not at Google. Contact bengr, 2014-06-05 20:43:43]

https://codereview.chromium.org/314173002/diff/40001/components/data_reductio...
File components/data_reduction_proxy/common/data_reduction_proxy_headers.cc
(right):

https://codereview.chromium.org/314173002/diff/40001/components/data_reductio...
components/data_reduction_proxy/common/data_reduction_proxy_headers.cc:124: if
(headers->response_code() == net::HTTP_PROXY_AUTHENTICATION_REQUIRED &&
On 2014/06/05 20:37:22, bengr1 wrote:
> Add a TODO that we should also report that the response is malformed if the
> response contains a Proxy-Authenticate header value that can't be interpreted
by
> the data reduction proxy.

Done.

[bengr, 2014-06-19 15:11:09]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-06-19 15:13:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kundaji@chromium.org/314173002/60001

[commit-bot: I haz the power, 2014-06-19 16:14:38]

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

[commit-bot: I haz the power, 2014-06-19 16:14:39]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/18978)

[bengr, 2014-06-19 21:58:52]

On 2014/06/19 16:14:39, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   mac_gpu on tryserver.chromium.gpu
>
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/18978)

Please close this issue if it is out of date.

[Not at Google. Contact bengr, 2014-06-19 22:10:51]

On 2014/06/19 21:58:52, bengr1 wrote:
> On 2014/06/19 16:14:39, I haz the power (commit-bot) wrote:
> > Try jobs failed on following builders:
> >   mac_gpu on tryserver.chromium.gpu
> >
>
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/18978)
> 
> Please close this issue if it is out of date.

Done.