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

Issue 3976004: Adds a new error for cases where a firewall may be the issue. (Closed)

Created:
10 years, 2 months ago by mmenke
Modified:
9 years, 7 months ago
Reviewers:
wtc
CC:
chromium-reviews, cbentzel+watch_chromium.org, darin-cc_chromium.org, Paweł Hajdan Jr.
Visibility:
Public.

Description

Adds a new error (ERR_NETWORK_ACCESS_DENIED) for when network operations return an access is denied error. This is in preparation for adding new error text suggesting a firewall may be causing the problem, which would be a little embarrassing to do when an SSPI logon fails, or when trying to open a local file one doesn't have permissions to. BUG=57108 TEST=Install firewall, block Chrome, verify that you get ERR_NETWORK_ACCESS_DENIED. Unblock it, fail an SSPI logon (Or any other event that should generate an ERR_ACCESS_DENIED) and verify that you still get an ERR_ACCESS_DENIED. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=64744

Patch Set 1 : '' #

Total comments: 9

Patch Set 2 : Response to comments #

Total comments: 1

Patch Set 3 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -4 lines) Patch
M net/base/net_error_list.h View 1 2 3 chunks +7 lines, -2 lines 0 comments Download
M net/ftp/ftp_network_transaction.cc View 1 chunk +1 line, -0 lines 0 comments Download
M net/socket/ssl_client_socket_nss.cc View 1 1 chunk +1 line, -0 lines 0 comments Download
M net/socket/tcp_client_socket_libevent.cc View 1 1 chunk +2 lines, -0 lines 0 comments Download
M net/socket/tcp_client_socket_win.cc View 1 2 chunks +4 lines, -2 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
mmenke
10 years, 1 month ago (2010-10-28 19:38:51 UTC) #1
wtc
Thanks for writing the CL. I searched for ERR_ACCESS_DENIED in the source tree. We can ...
10 years, 1 month ago (2010-10-28 22:39:46 UTC) #2
mmenke1
I'd already checked the two ERR_ACCESS_DENIEDs you mention, though I appreciate you double checking. The ...
10 years, 1 month ago (2010-10-29 00:54:42 UTC) #3
wtc
10 years, 1 month ago (2010-10-29 20:29:23 UTC) #4
LGTM.  Thanks!

http://codereview.chromium.org/3976004/diff/14001/15003
File net/socket/ssl_client_socket_nss.cc (right):

http://codereview.chromium.org/3976004/diff/14001/15003#newcode191
net/socket/ssl_client_socket_nss.cc:191: return ERR_NETWORK_ACCESS_DENIED;
On 2010/10/29 00:54:42, mmenke wrote:
> 
> Sent the CL to you mainly because I was unsure of that change.  I wasn't sure
if
> an ERR_NETWORK_ACCESS_DENIED from a TcpClientSocket could end up being sent
> through NSS's callbacks and then end up back here, if we couldn't connect to
> verify a certificate or something.

I see.  If we couldn't connect to verify a certificate,
we would get ERR_CERT_UNABLE_TO_CHECK_REVOCATION.

We cannot possible get TCPClientSocket connect error here
because when a SSLClientSocket is constructed, it is given
a "transport socket" that's already connected.

Note: I believe I added the PR_NO_ACCESS_RIGHTS_ERROR case
to this function by going through all NSPR error codes and
map the ones that have an equivalent error code in Chrome.
It doesn't necessarily mean I have actually got those error
codes in SSLClientSocket.

http://codereview.chromium.org/3976004/diff/34001/35001
File net/base/net_error_list.h (right):

http://codereview.chromium.org/3976004/diff/34001/35001#newcode213
net/base/net_error_list.h:213: // by a firewall.
Nit: add "See also ERR_ACCESS_DENIED." to provide the context
for the second sentence.

Powered by Google App Engine
This is Rietveld 408576698