Add specific error codes for when SOCKS connect fails.

This also fixes HttpNetworkTransaction to fallback to the next proxy after a failure is encountered using a SOCKS proxy server.

Note that I decided against using finer granularity error codes, since it just adds a higher maintenance cost to ReconsiderProxyAfterError().
Power users can get better information on the cause of the failure by looking at the LoadLog anyway.

BUG=34386

Review URL: http://codereview.chromium.org/567030

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@38170 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 46 insertions, 21 deletions

diff --git a/net/base/net_error_list.h b/net/base/net_error_list.h
index bff55d5..7c0c3de 100644
--- a/net/base/net_error_list.h
+++ b/net/base/net_error_list.h
@@ -125,6 +125,13 @@ NET_ERROR(CONNECTION_TIMED_OUT, -118)
 // aborted.
 NET_ERROR(HOST_RESOLVER_QUEUE_TOO_LARGE, -119)
 
+// Failed establishing a connection to the SOCKS proxy server for a target host.
+NET_ERROR(SOCKS_CONNECTION_FAILED, -120)
+
+// The SOCKS proxy server failed establishing connection to the target host
+// because that host is unreachable.
+NET_ERROR(SOCKS_CONNECTION_HOST_UNREACHABLE, -121)
+
 // Certificate error codes
 //
 // The values of certificate error codes must be consecutive.
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 4be555f..f970384 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -1585,7 +1585,18 @@ int HttpNetworkTransaction::ReconsiderProxyAfterError(int error) {
     case ERR_CONNECTION_ABORTED:
     case ERR_TIMED_OUT:
     case ERR_TUNNEL_CONNECTION_FAILED:
+    case ERR_SOCKS_CONNECTION_FAILED:
       break;
+    case ERR_SOCKS_CONNECTION_HOST_UNREACHABLE:
+      // Remap the SOCKS-specific "host unreachable" error to a more
+      // generic error code (this way consumers like the link doctor
+      // know to substitute their error page).
+      //
+      // Note that if the host resolving was done by the SOCSK5 proxy, we can't
+      // differentiate between a proxy-side "host not found" versus a proxy-side
+      // "address unreachable" error, and will report both of these failures as
+      // ERR_ADDRESS_UNREACHABLE.
+      return ERR_ADDRESS_UNREACHABLE;
     default:
       return error;
   }
diff --git a/net/socket/socks5_client_socket.cc b/net/socket/socks5_client_socket.cc
index a4ba814..cbb2dc7 100644
--- a/net/socket/socks5_client_socket.cc
+++ b/net/socket/socks5_client_socket.cc
@@ -223,7 +223,7 @@ int SOCKS5ClientSocket::DoGreetWrite() {
     LoadLog::AddStringLiteral(load_log_,
         "Failed sending request because hostname is "
         "longer than 255 characters");
-    return ERR_INVALID_URL;
+    return ERR_SOCKS_CONNECTION_FAILED;
   }
 
   if (buffer_.empty()) {
@@ -266,8 +266,11 @@ int SOCKS5ClientSocket::DoGreetReadComplete(int result) {
   if (result < 0)
     return result;
 
-  if (result == 0)
-    return ERR_CONNECTION_CLOSED;  // Unexpected socket close
+  if (result == 0) {
+    LoadLog::AddStringLiteral(
+        load_log_, "Connection unexpected closed while reading greeting.");
+    return ERR_SOCKS_CONNECTION_FAILED;
+  }
 
   bytes_received_ += result;
   buffer_.append(handshake_buf_->data(), result);
@@ -281,13 +284,13 @@ int SOCKS5ClientSocket::DoGreetReadComplete(int result) {
     LoadLog::AddStringLiteral(load_log_, "Unexpected SOCKS version");
     LoadLog::AddString(load_log_, StringPrintf(
         "buffer_[0] = 0x%x", static_cast<int>(buffer_[0])));
-    return ERR_INVALID_RESPONSE;
+    return ERR_SOCKS_CONNECTION_FAILED;
   }
   if (buffer_[1] != 0x00) {
     LoadLog::AddStringLiteral(load_log_, "Unexpected authentication method");
     LoadLog::AddString(load_log_, StringPrintf(
         "buffer_[1] = 0x%x", static_cast<int>(buffer_[1])));
-    return ERR_INVALID_RESPONSE;  // Unknown error
+    return ERR_SOCKS_CONNECTION_FAILED;
   }
 
   buffer_.clear();
@@ -374,8 +377,11 @@ int SOCKS5ClientSocket::DoHandshakeReadComplete(int result) {
     return result;
 
   // The underlying socket closed unexpectedly.
-  if (result == 0)
-    return ERR_CONNECTION_CLOSED;
+  if (result == 0) {
+    LoadLog::AddStringLiteral(
+        load_log_, "Connection unexpected closed while reading handshake.");
+    return ERR_SOCKS_CONNECTION_FAILED;
+  }
 
   buffer_.append(handshake_buf_->data(), result);
   bytes_received_ += result;
@@ -389,7 +395,7 @@ int SOCKS5ClientSocket::DoHandshakeReadComplete(int result) {
           "buffer_[0] = 0x%x; buffer_[2] = 0x%x",
           static_cast<int>(buffer_[0]),
           static_cast<int>(buffer_[2])));
-      return ERR_INVALID_RESPONSE;
+      return ERR_SOCKS_CONNECTION_FAILED;
     }
     if (buffer_[1] != 0x00) {
       LoadLog::AddStringLiteral(load_log_,
@@ -401,7 +407,7 @@ int SOCKS5ClientSocket::DoHandshakeReadComplete(int result) {
         LoadLog::AddString(load_log_, StringPrintf(
             "buffer_[1] = 0x%x", static_cast<int>(buffer_[1])));
       }
-      return ERR_FAILED;
+      return ERR_SOCKS_CONNECTION_FAILED;
     }
 
     // We check the type of IP/Domain the server returns and accordingly
@@ -421,7 +427,7 @@ int SOCKS5ClientSocket::DoHandshakeReadComplete(int result) {
       LoadLog::AddStringLiteral(load_log_, "Unknown address type in response");
       LoadLog::AddString(load_log_, StringPrintf(
           "buffer_[3] = 0x%x", static_cast<int>(buffer_[3])));
-      return ERR_INVALID_RESPONSE;
+      return ERR_SOCKS_CONNECTION_FAILED;
     }
 
     read_header_size += 2;  // for the port.
diff --git a/net/socket/socks5_client_socket_unittest.cc b/net/socket/socks5_client_socket_unittest.cc
index b4eb2cf..4b87a14 100644
--- a/net/socket/socks5_client_socket_unittest.cc
+++ b/net/socket/socks5_client_socket_unittest.cc
@@ -204,7 +204,7 @@ TEST_F(SOCKS5ClientSocketTest, LargeHostNameFails) {
   // the transport socket first) because the hostname is too long.
   TestCompletionCallback callback;
   int rv = user_sock_->Connect(&callback, NULL);
-  EXPECT_EQ(ERR_INVALID_URL, rv);
+  EXPECT_EQ(ERR_SOCKS_CONNECTION_FAILED, rv);
 }
 
 TEST_F(SOCKS5ClientSocketTest, PartialReadWrites) {
diff --git a/net/socket/socks_client_socket.cc b/net/socket/socks_client_socket.cc
index 5850da3..7d8aaf9 100644
--- a/net/socket/socks_client_socket.cc
+++ b/net/socket/socks_client_socket.cc
@@ -355,8 +355,10 @@ int SOCKSClientSocket::DoHandshakeReadComplete(int result) {
   if (result == 0)
     return ERR_CONNECTION_CLOSED;
 
-  if (bytes_received_ + result > kReadHeaderSize)
-    return ERR_INVALID_RESPONSE;
+  if (bytes_received_ + result > kReadHeaderSize) {
+    // TODO(eroman): Describe failure in LoadLog.
+    return ERR_SOCKS_CONNECTION_FAILED;
+  }
 
   buffer_.append(handshake_buf_->data(), result);
   bytes_received_ += result;
@@ -370,28 +372,27 @@ int SOCKSClientSocket::DoHandshakeReadComplete(int result) {
 
   if (response->reserved_null != 0x00) {
     LOG(ERROR) << "Unknown response from SOCKS server.";
-    return ERR_INVALID_RESPONSE;
+    return ERR_SOCKS_CONNECTION_FAILED;
   }
 
-  // TODO(arindam): Add SOCKS specific failure codes in net_error_list.h
   switch (response->code) {
     case kServerResponseOk:
       completed_handshake_ = true;
       return OK;
     case kServerResponseRejected:
       LOG(ERROR) << "SOCKS request rejected or failed";
-      return ERR_FAILED;
+      return ERR_SOCKS_CONNECTION_FAILED;
     case kServerResponseNotReachable:
       LOG(ERROR) << "SOCKS request failed because client is not running "
                  << "identd (or not reachable from the server)";
-      return ERR_NAME_NOT_RESOLVED;
+      return ERR_SOCKS_CONNECTION_HOST_UNREACHABLE;
     case kServerResponseMismatchedUserId:
       LOG(ERROR) << "SOCKS request failed because client's identd could "
                  << "not confirm the user ID string in the request";
-      return ERR_FAILED;
+      return ERR_SOCKS_CONNECTION_FAILED;
     default:
       LOG(ERROR) << "SOCKS server sent unknown response";
-      return ERR_INVALID_RESPONSE;
+      return ERR_SOCKS_CONNECTION_FAILED;
   }
 
   // Note: we ignore the last 6 bytes as specified by the SOCKS protocol
diff --git a/net/socket/socks_client_socket_unittest.cc b/net/socket/socks_client_socket_unittest.cc
index dfa078f..bbd6f0f 100644
--- a/net/socket/socks_client_socket_unittest.cc
+++ b/net/socket/socks_client_socket_unittest.cc
@@ -176,12 +176,12 @@ TEST_F(SOCKSClientSocketTest, HandshakeFailures) {
     // Failure of the server response code
     {
       { 0x01, 0x5A, 0x00, 0x00, 0, 0, 0, 0 },
-      ERR_INVALID_RESPONSE,
+      ERR_SOCKS_CONNECTION_FAILED,
     },
     // Failure of the null byte
     {
       { 0x00, 0x5B, 0x00, 0x00, 0, 0, 0, 0 },
-      ERR_FAILED,
+      ERR_SOCKS_CONNECTION_FAILED,
     },
   };