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

Issue 9600022: net: Disconnect proxy sockets that have a certificate error. (Closed)

Created:
8 years, 9 months ago by agl
Modified:
8 years, 9 months ago
Reviewers:
wtc, Ryan Hamilton
Visibility:
Public.

Description

net: Disconnect proxy sockets that have a certificate error. BUG=116398 TEST=Set a SPDY proxy with a certificate error. Ensure that requests always get ERR_PROXY_CERTIFICATE_INVALID, even with several reloads. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=124970

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -0 lines) Patch
M net/http/http_proxy_client_socket_pool.cc View 1 chunk +1 line, -0 lines 3 comments Download

Messages

Total messages: 6 (0 generated)
agl
8 years, 9 months ago (2012-03-05 15:47:41 UTC) #1
Ryan Hamilton
LGTM Nice catch! I can't believe I missed this...
8 years, 9 months ago (2012-03-05 16:43:21 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/agl@chromium.org/9600022/1
8 years, 9 months ago (2012-03-05 16:56:54 UTC) #3
commit-bot: I haz the power
Change committed as 124970
8 years, 9 months ago (2012-03-05 18:26:54 UTC) #4
wtc
rch: I have two questions for you. http://codereview.chromium.org/9600022/diff/1/net/http/http_proxy_client_socket_pool.cc File net/http/http_proxy_client_socket_pool.cc (right): http://codereview.chromium.org/9600022/diff/1/net/http/http_proxy_client_socket_pool.cc#newcode220 net/http/http_proxy_client_socket_pool.cc:220: return result; ...
8 years, 9 months ago (2012-03-05 21:08:14 UTC) #5
agl
8 years, 9 months ago (2012-03-05 21:16:14 UTC) #6
http://codereview.chromium.org/9600022/diff/1/net/http/http_proxy_client_sock...
File net/http/http_proxy_client_socket_pool.cc (right):

http://codereview.chromium.org/9600022/diff/1/net/http/http_proxy_client_sock...
net/http/http_proxy_client_socket_pool.cc:220: return result;
On 2012/03/05 21:08:14, wtc wrote:
> rch: do we also need to disconnect the socket here?

I thought about this when writing the patch but decided that the socket, in this
case, was unusable. So even if we do need to call Disconnect here, it's not a
security issue because writing to the socket wouldn't work (or so I expect).
Therefore, since this patch was scheduled for merging, it was best to keep it
minimal.

Powered by Google App Engine
This is Rietveld 408576698