Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(145)

Issue 8502024: Allow chrome to handle 407 auth challenges to CONNECT requests (Closed)

Created:
9 years, 1 month ago by Ryan Hamilton
Modified:
9 years, 1 month ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, darin-cc_chromium.org, Paweł Hajdan Jr.
Visibility:
Public.

Description

Allow chrome to handle 407 auth challenges to CONNECT requests through HTTPS Proxies. This also changes the mechanism used to restart HttpProxyClientSocket requests with auth. Previously the transport socket would be Disconnected, and then re-Connected (which was not implemented for SSLClientSockets). However, the approach was problematic in the face of, for example, ipv6. The new approach is to close the HttpProxyClientSocket, and request a new socket from the pool. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110529 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110879 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110965

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : Initial version #

Patch Set 5 : Fix SPDY case #

Patch Set 6 : Working #

Patch Set 7 : Remove inline virtuals #

Total comments: 12

Patch Set 8 : Address final comments #

Patch Set 9 : sync #

Patch Set 10 : sync again #

Total comments: 1

Patch Set 11 : re-upload #

Patch Set 12 : asan! #

Patch Set 13 : '' #

Patch Set 14 : fix typo in unittest #

Unified diffs Side-by-side diffs Delta from patch set Stats (+581 lines, -95 lines) Patch
M net/http/http_network_transaction_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 12 chunks +409 lines, -13 lines 0 comments Download
M net/http/http_proxy_client_socket.h View 1 2 3 4 5 6 7 8 9 10 11 5 chunks +2 lines, -15 lines 0 comments Download
M net/http/http_proxy_client_socket.cc View 1 2 3 4 5 6 7 8 9 10 11 7 chunks +8 lines, -37 lines 0 comments Download
M net/http/http_proxy_client_socket_pool_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +19 lines, -11 lines 0 comments Download
M net/http/http_proxy_utils.h View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +9 lines, -0 lines 0 comments Download
M net/http/http_proxy_utils.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +18 lines, -0 lines 0 comments Download
M net/http/http_stream_factory_impl_job.cc View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +9 lines, -9 lines 0 comments Download
M net/http/proxy_client_socket.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +10 lines, -0 lines 0 comments Download
M net/spdy/spdy_proxy_client_socket.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -8 lines 0 comments Download
M net/spdy/spdy_proxy_client_socket.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +23 lines, -0 lines 0 comments Download
M net/spdy/spdy_proxy_client_socket_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 6 chunks +72 lines, -2 lines 0 comments Download

Messages

Total messages: 28 (0 generated)
Ryan Hamilton
Hi Chris, Can you take a look at this CL. I think you'll probably want ...
9 years, 1 month ago (2011-11-10 22:18:37 UTC) #1
cbentzel
Will get to it tomorrow - is that OK? On Thu, Nov 10, 2011 at ...
9 years, 1 month ago (2011-11-10 22:55:31 UTC) #2
cbentzel
I thought there was a test for this - but the reason that we don't ...
9 years, 1 month ago (2011-11-12 02:21:04 UTC) #3
Ryan Hamilton
On 2011/11/12 02:21:04, cbentzel wrote: > I thought there was a test for this - ...
9 years, 1 month ago (2011-11-12 04:11:25 UTC) #4
vandebo (ex-Chrome)
So I'm a bit out of the loop on this stuff and I don't know ...
9 years, 1 month ago (2011-11-14 17:23:56 UTC) #5
rch (use chromium not google)
Thanks for the comments... a couple of comments: On Mon, Nov 14, 2011 at 9:23 ...
9 years, 1 month ago (2011-11-14 18:36:34 UTC) #6
cbentzel
On 2011/11/14 17:23:56, vandebo wrote: > So I'm a bit out of the loop on ...
9 years, 1 month ago (2011-11-14 20:04:48 UTC) #7
rch (use chromium not google)
On Mon, Nov 14, 2011 at 12:04 PM, <cbentzel@chromium.org> wrote: > > This is an ...
9 years, 1 month ago (2011-11-14 21:30:49 UTC) #8
cbentzel
OK, this looks like it will work. I don't want to block it for a ...
9 years, 1 month ago (2011-11-14 22:11:53 UTC) #9
vandebo (ex-Chrome)
On 2011/11/14 18:36:34, rch wrote: > On Mon, Nov 14, 2011 at 9:23 AM, <mailto:vandebo@chromium.org> ...
9 years, 1 month ago (2011-11-14 22:35:25 UTC) #10
Ryan Hamilton
http://codereview.chromium.org/8502024/diff/12016/net/http/proxy_client_socket.h File net/http/proxy_client_socket.h (right): http://codereview.chromium.org/8502024/diff/12016/net/http/proxy_client_socket.h#newcode9 net/http/proxy_client_socket.h:9: #include "net/http/http_auth_controller.h" On 2011/11/14 22:11:53, cbentzel wrote: > Can ...
9 years, 1 month ago (2011-11-14 23:53:40 UTC) #11
cbentzel
On 2011/11/14 22:35:25, vandebo wrote: > On 2011/11/14 18:36:34, rch wrote: > > On Mon, ...
9 years, 1 month ago (2011-11-15 19:59:46 UTC) #12
cbentzel
LGTM You haven't answered my question about returning a tunnel connection establishment error if the ...
9 years, 1 month ago (2011-11-15 20:00:49 UTC) #13
rch (use chromium not google)
On Tue, Nov 15, 2011 at 12:00 PM, <cbentzel@chromium.org> wrote: > LGTM > Thanks! > ...
9 years, 1 month ago (2011-11-15 20:50:02 UTC) #14
cbentzel
Still LGTM. Thanks for adding the test for the NTLM case. On 2011/11/15 20:50:02, rch ...
9 years, 1 month ago (2011-11-16 18:54:42 UTC) #15
cbentzel
http://codereview.chromium.org/8502024/diff/10039/net/spdy/spdy_proxy_client_socket.cc File net/spdy/spdy_proxy_client_socket.cc (right): http://codereview.chromium.org/8502024/diff/10039/net/spdy/spdy_proxy_client_socket.cc#newcode404 net/spdy/spdy_proxy_client_socket.cc:404: // SPDY only supports basic and digest auth It ...
9 years, 1 month ago (2011-11-16 18:57:38 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rch@chromium.org/8502024/10039
9 years, 1 month ago (2011-11-17 16:31:30 UTC) #17
commit-bot: I haz the power
Can't process patch for file net/http/http_proxy_utils.h. File's status is None, patchset upload is incomplete.
9 years, 1 month ago (2011-11-17 16:31:33 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rch@chromium.org/8502024/10039
9 years, 1 month ago (2011-11-17 16:35:40 UTC) #19
commit-bot: I haz the power
Can't process patch for file net/http/http_proxy_utils.h. File's status is None, patchset upload is incomplete.
9 years, 1 month ago (2011-11-17 16:35:42 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rch@chromium.org/8502024/43001
9 years, 1 month ago (2011-11-17 16:39:10 UTC) #21
commit-bot: I haz the power
Change committed as 110529
9 years, 1 month ago (2011-11-17 18:31:05 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rch@chromium.org/8502024/19019
9 years, 1 month ago (2011-11-19 03:20:03 UTC) #23
commit-bot: I haz the power
Try job failure for 8502024-19019 (retry) on linux_rel for step "ui_tests". It's a second try, ...
9 years, 1 month ago (2011-11-19 04:13:08 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rch@chromium.org/8502024/19019
9 years, 1 month ago (2011-11-21 03:02:42 UTC) #25
commit-bot: I haz the power
Change committed as 110879
9 years, 1 month ago (2011-11-21 04:21:13 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rch@chromium.org/8502024/61003
9 years, 1 month ago (2011-11-21 16:28:31 UTC) #27
commit-bot: I haz the power
9 years, 1 month ago (2011-11-21 18:32:06 UTC) #28
Change committed as 110965

Powered by Google App Engine
This is Rietveld 408576698