Adds a new error for cases where a firewall may be the issue. (issue3976004)
mme...@chromium.org
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.
Please review this at http://codereview.chromium.org/3976004/show
SVN Base: svn://svn.chromium.org/chrome/trunk/src/
Affected files:
M net/base/net_error_list.h
M net/ftp/ftp_network_transaction.cc
M net/socket/ssl_client_socket_nss.cc
M net/socket/tcp_client_socket_libevent.cc
M net/socket/tcp_client_socket_win.cc
Index: net/base/net_error_list.h
===================================================================
--- net/base/net_error_list.h (revision 63512)
+++ net/base/net_error_list.h (working copy)
@@ -50,7 +50,7 @@
// invalid assumption.
NET_ERROR(UNEXPECTED, -9)
-// Permission to access a resource was denied.
+// Permission to access a (non-networking) resource was denied.
NET_ERROR(ACCESS_DENIED, -10)
// The operation failed because of unimplemented functionality.
@@ -69,6 +69,10 @@
// The socket is not connected
NET_ERROR(SOCKET_NOT_CONNECTED, -15)
+// Permission to access a to a network resource was denied. This is used
to
+// distinguish errors that were most likely caused by a firewall.
+NET_ERROR(NETWORK_ACCESS_DENIED, -16)
+
// A connection was closed (corresponding to a TCP FIN).
NET_ERROR(CONNECTION_CLOSED, -100)
Index: net/ftp/ftp_network_transaction.cc
===================================================================
--- net/ftp/ftp_network_transaction.cc (revision 63512)
+++ net/ftp/ftp_network_transaction.cc (working copy)
@@ -1298,6 +1298,7 @@
type = NET_ERROR_OK;
break;
case ERR_ACCESS_DENIED:
+ case ERR_NETWORK_ACCESS_DENIED:
type = NET_ERROR_ACCESS_DENIED;
break;
case ERR_TIMED_OUT:
Index: net/socket/ssl_client_socket_nss.cc
===================================================================
--- net/socket/ssl_client_socket_nss.cc (revision 63512)
+++ net/socket/ssl_client_socket_nss.cc (working copy)
@@ -188,7 +188,7 @@
return ERR_IO_PENDING;
case PR_ADDRESS_NOT_SUPPORTED_ERROR: // For connect.
case PR_NO_ACCESS_RIGHTS_ERROR:
- return ERR_ACCESS_DENIED;
+ return ERR_NETWORK_ACCESS_DENIED;
case PR_IO_TIMEOUT_ERROR:
return ERR_TIMED_OUT;
case PR_CONNECT_RESET_ERROR:
@@ -1378,6 +1378,7 @@
case ERR_IO_PENDING:
return PR_WOULD_BLOCK_ERROR;
case ERR_ACCESS_DENIED:
+ case ERR_NETWORK_ACCESS_DENIED:
// For connect, this could be mapped to
PR_ADDRESS_NOT_SUPPORTED_ERROR.
return PR_NO_ACCESS_RIGHTS_ERROR;
case ERR_INTERNET_DISCONNECTED: // Equivalent to ENETDOWN.
Index: net/socket/tcp_client_socket_libevent.cc
===================================================================
--- net/socket/tcp_client_socket_libevent.cc (revision 63512)
+++ net/socket/tcp_client_socket_libevent.cc (working copy)
@@ -56,7 +56,7 @@
#endif
return ERR_IO_PENDING;
case EACCES:
- return ERR_ACCESS_DENIED;
+ return ERR_NETWORK_ACCESS_DENIED;
case ENETDOWN:
return ERR_INTERNET_DISCONNECTED;
case ETIMEDOUT:
Index: net/socket/tcp_client_socket_win.cc
===================================================================
--- net/socket/tcp_client_socket_win.cc (revision 63512)
+++ net/socket/tcp_client_socket_win.cc (working copy)
@@ -67,7 +67,7 @@
// connect fails with WSAEACCES when Windows Firewall blocks the
// connection.
case WSAEACCES:
- return ERR_ACCESS_DENIED;
+ return ERR_NETWORK_ACCESS_DENIED;
case WSAENETDOWN:
return ERR_INTERNET_DISCONNECTED;
case WSAETIMEDOUT:
w...@chromium.org
I searched for ERR_ACCESS_DENIED in the source tree.
We can change http_auth_sspi_win.cc to map SEC_E_LOGON_DENIED to a
better error code, and consider adding
ERR_FILE_ACCESS_DENIED, ERR_COOKIE_ACCESS_DENIED, etc.,
if useful.
Please review the following files and see if they should
be updated:
src\webkit\blob\blob_url_request_job.cc(473): case
net::ERR_ACCESS_DENIED:
src\chrome_frame\urlmon_url_request.cc(925): ret =
net::ERR_ACCESS_DENIED;
http://codereview.chromium.org/3976004/diff/14001/15001
File net/base/net_error_list.h (right):
http://codereview.chromium.org/3976004/diff/14001/15001#newcode53
net/base/net_error_list.h:53: // Permission to access a (non-networking)
resource was denied.
Nit: how about:
Permission to access a resource (other than the network) was denied.
or just change "non-networking" to "non-network".
http://codereview.chromium.org/3976004/diff/14001/15001#newcode72
net/base/net_error_list.h:72: // Permission to access a to a network
resource was denied. This is used to
Nit: a to a network resource => the network
This is a connection-related error, so please move it to the
100-199 range. See the comments at lines 11-19 above.
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;
I believe this change is not necessary.
If I understand this CL correctly, we only want to use
ERR_NETWORK_ACCESS_DENIED in TCPClientSocket.
http://codereview.chromium.org/3976004/diff/14001/15004
File net/socket/tcp_client_socket_libevent.cc (right):
http://codereview.chromium.org/3976004/diff/14001/15004#newcode58
net/socket/tcp_client_socket_libevent.cc:58: case EACCES:
You may want to consider mapping EACCES to
ERR_NETWORK_ACCESS_DENIED only for the Connect method.
You can do this by adding
case EACCES:
return ERR_NETWORK_ACCESS_DENIED;
to the MapConnectError function at line 87 below.
If you accept this suggestion, please make the same
change to tcp_client_socket_win.cc.
mme...@google.com
appreciate
you double checking.
The switch statement in src\webkit\blob\blob_url_request_job.cc only handles
errors it generates itself explicitly, and it never even generates an
ERR_ACCESS_DENIED. As Blob objects are used for local file or data access,
the
case is most likely intended for access being denied to a local file.
As for src\chrome_frame\urlmon_url_request.cc... ChromeFrame is only used
when
the HTTP headers or HTML indicates it should be used, so regardless of what
function gives us an E_ACCESSDENIED, presumably there's no firewall. Not
clear
if we could separate out firewall errors anyways, as we use higher level COM
classes instead of managing sockets ourselves. Probably best to just let IE
take care of diagnosing any firewall issues.
http://codereview.chromium.org/3976004/diff/14001/15001
File net/base/net_error_list.h (right):
http://codereview.chromium.org/3976004/diff/14001/15001#newcode53
net/base/net_error_list.h:53: // Permission to access a (non-networking)
resource was denied.
On 2010/10/28 22:39:46, wtc wrote:
> Nit: how about:
> Permission to access a resource (other than the network) was denied.
> or just change "non-networking" to "non-network".
Text changed.
http://codereview.chromium.org/3976004/diff/14001/15001#newcode72
net/base/net_error_list.h:72: // Permission to access a to a network
resource was denied. This is used to
On 2010/10/28 22:39:46, wtc wrote:
> Nit: a to a network resource => the network
> This is a connection-related error, so please move it to the
> 100-199 range. See the comments at lines 11-19 above.
Error code changed (And comment updated). Had seen the text, but I
thought a firewall issue would be a system error.
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/28 22:39:46, wtc wrote:
> I believe this change is not necessary.
> If I understand this CL correctly, we only want to use
> ERR_NETWORK_ACCESS_DENIED in TCPClientSocket.
Done.
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.
http://codereview.chromium.org/3976004/diff/14001/15004
File net/socket/tcp_client_socket_libevent.cc (right):
http://codereview.chromium.org/3976004/diff/14001/15004#newcode58
net/socket/tcp_client_socket_libevent.cc:58: case EACCES:
On 2010/10/28 22:39:46, wtc wrote:
> You may want to consider mapping EACCES to
> ERR_NETWORK_ACCESS_DENIED only for the Connect method.
> You can do this by adding
> case EACCES:
> return ERR_NETWORK_ACCESS_DENIED;
> to the MapConnectError function at line 87 below.
> If you accept this suggestion, please make the same
> change to tcp_client_socket_win.cc.
Odds of a firewall issue springing up in the middle of a connection do
seem pretty small, though I'm not quite sure what an EACCESS or
equivalent later on would mean. Change made (to both files).
w...@chromium.org
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.