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

4 files changed, 28 insertions, 21 deletions

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,
     },
   };