Change the default SOCKS version used by gnome settings (and environment variables) to be 5 instead of 4.

BUG=56833
TEST=Open Chrome in gnome. Go to change the proxy settings (which will pop open the gnome network settings). Enter as socks proxy server, "localhost:8080". Now load chrome://net-internals/#proxy and verify that it says the proxy server is "socks5://localhost:8080" (and NOT socks4://localhost:8080).
Review URL: http://codereview.chromium.org/3413037

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

2 files changed, 20 insertions, 16 deletions

diff --git a/net/proxy/proxy_config_service_linux.cc b/net/proxy/proxy_config_service_linux.cc
index 1cf9e3d..b61167f 100644
--- a/net/proxy/proxy_config_service_linux.cc
+++ b/net/proxy/proxy_config_service_linux.cc
@@ -43,11 +43,11 @@ namespace {
 // TODO(arindam): Remove URI string manipulation by using MapUrlSchemeToProxy.
 std::string FixupProxyHostScheme(ProxyServer::Scheme scheme,
                                  std::string host) {
-  if (scheme == ProxyServer::SCHEME_SOCKS4 &&
-      StartsWithASCII(host, "socks5://", false)) {
-    // We default to socks 4, but if the user specifically set it to
-    // socks5://, then use that.
-    scheme = ProxyServer::SCHEME_SOCKS5;
+  if (scheme == ProxyServer::SCHEME_SOCKS5 &&
+      StartsWithASCII(host, "socks4://", false)) {
+    // We default to socks 5, but if the user specifically set it to
+    // socks4://, then use that.
+    scheme = ProxyServer::SCHEME_SOCKS4;
   }
   // Strip the scheme if any.
   std::string::size_type colon = host.find("://");
@@ -152,11 +152,13 @@ bool ProxyConfigServiceLinux::Delegate::GetConfigFromEnv(ProxyConfig* config) {
   }
   if (config->proxy_rules().empty()) {
     // If the above were not defined, try for socks.
-    ProxyServer::Scheme scheme = ProxyServer::SCHEME_SOCKS4;
+    // For environment variables, we default to version 5, per the gnome
+    // documentation: http://library.gnome.org/devel/gnet/stable/gnet-socks.html
+    ProxyServer::Scheme scheme = ProxyServer::SCHEME_SOCKS5;
     std::string env_version;
     if (env_var_getter_->GetVar("SOCKS_VERSION", &env_version)
-        && env_version == "5")
-      scheme = ProxyServer::SCHEME_SOCKS5;
+        && env_version == "4")
+      scheme = ProxyServer::SCHEME_SOCKS4;
     if (GetProxyFromEnvVarForScheme("SOCKS_SERVER", scheme, &proxy_server)) {
       config->proxy_rules().type = ProxyConfig::ProxyRules::TYPE_SINGLE_PROXY;
       config->proxy_rules().single_proxy = proxy_server;
@@ -915,7 +917,7 @@ bool ProxyConfigServiceLinux::Delegate::GetProxyFromGConf(
     host += ":" + base::IntToString(port);
   }
   host = FixupProxyHostScheme(
-      is_socks ? ProxyServer::SCHEME_SOCKS4 : ProxyServer::SCHEME_HTTP,
+      is_socks ? ProxyServer::SCHEME_SOCKS5 : ProxyServer::SCHEME_HTTP,
       host);
   ProxyServer proxy_server = ProxyServer::FromURI(host,
                                                   ProxyServer::SCHEME_HTTP);
@@ -982,7 +984,9 @@ bool ProxyConfigServiceLinux::Delegate::GetConfigFromGConf(
     // Try socks.
     if (GetProxyFromGConf("/system/proxy/socks_", true, &proxy_server)) {
       // gconf settings do not appear to distinguish between socks
-      // version. We default to version 4.
+      // version. We default to version 5. For more information on this policy
+      // decisions, see:
+      // http://code.google.com/p/chromium/issues/detail?id=55912#c2
       config->proxy_rules().type = ProxyConfig::ProxyRules::TYPE_SINGLE_PROXY;
       config->proxy_rules().single_proxy = proxy_server;
     }
diff --git a/net/proxy/proxy_config_service_linux_unittest.cc b/net/proxy/proxy_config_service_linux_unittest.cc
index 41fe7d2..2cb1259 100644
--- a/net/proxy/proxy_config_service_linux_unittest.cc
+++ b/net/proxy/proxy_config_service_linux_unittest.cc
@@ -582,7 +582,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicGConfTest) {
       false,                                          // auto_detect
       GURL(),                                         // pac_url
       ProxyRulesExpectation::Single(
-          "socks4://socks.com:99",  // single proxy
+          "socks5://socks.com:99",  // single proxy
           "")                       // bypass rules
     },
 
@@ -825,12 +825,12 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
       false,                                   // auto_detect
       GURL(),                                  // pac_url
       ProxyRulesExpectation::Single(
-          "socks4://socks.com:888",  // single proxy
+          "socks5://socks.com:888",  // single proxy
           ""),                       // bypass rules
     },
 
     {
-      TEST_DESC("socks5"),
+      TEST_DESC("socks4"),
       { // Input.
         NULL,  // DESKTOP_SESSION
         NULL,  // HOME
@@ -839,7 +839,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
         NULL,  // auto_proxy
         "",  // all_proxy
         NULL, NULL, NULL,  // per-proto proxies
-        "socks.com:888", "5",  // SOCKS
+        "socks.com:888", "4",  // SOCKS
         NULL,  // no_proxy
       },
 
@@ -847,7 +847,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
       false,                                   // auto_detect
       GURL(),                                  // pac_url
       ProxyRulesExpectation::Single(
-          "socks5://socks.com:888",  // single proxy
+          "socks4://socks.com:888",  // single proxy
           ""),                       // bypass rules
     },
 
@@ -869,7 +869,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
       false,                                   // auto_detect
       GURL(),                                  // pac_url
       ProxyRulesExpectation::Single(
-          "socks4://socks.com:1080",  // single proxy
+          "socks5://socks.com:1080",  // single proxy
           ""),                        // bypass rules
     },