Check in patch for pierre.lafayette, http://codereview.chromium.org/178059/show.

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

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

8 files changed, 75 insertions, 79 deletions

diff --git a/net/base/net_util.cc b/net/base/net_util.cc
index 3b6349f..13e99a5 100644
--- a/net/base/net_util.cc
+++ b/net/base/net_util.cc
@@ -749,6 +749,8 @@ std::wstring FormatViewSourceUrl(const GURL& url,
 
 namespace net {
 
+std::set<int> explicitly_allowed_ports;
+
 // Appends the substring |in_component| inside of the URL |spec| to |output|,
 // and the resulting range will be filled into |out_component|. |unescape_rules|
 // defines how to clean the URL for human readability.
@@ -1041,6 +1043,18 @@ bool IsPortAllowedByFtp(int port) {
   return IsPortAllowedByDefault(port);
 }
 
+bool IsPortAllowedByOverride(int port) {
+  if (explicitly_allowed_ports.empty())
+    return false;
+
+  std::set<int>::const_iterator it =
+      std::find(explicitly_allowed_ports.begin(),
+                explicitly_allowed_ports.end(),
+                port);
+
+  return it != explicitly_allowed_ports.end();
+}
+
 int SetNonBlocking(int fd) {
 #if defined(OS_WIN)
   unsigned long no_block = 1;
@@ -1316,4 +1330,33 @@ GURL SimplifyUrlForRequest(const GURL& url) {
   return url.ReplaceComponents(replacements);
 }
 
+// Specifies a comma separated list of port numbers that should be accepted
+// despite bans. If the string is invalid no allowed ports are stored.
+void SetExplicitlyAllowedPorts(const std::wstring& allowed_ports) {
+  if (allowed_ports.empty())
+    return;
+
+  std::set<int> ports;
+  size_t last = 0;
+  size_t size = allowed_ports.size();
+  // The comma delimiter.
+  const std::wstring::value_type kComma = L',';
+
+  // Overflow is still possible for evil user inputs.
+  for (size_t i = 0; i <= size; ++i) {
+    // The string should be composed of only digits and commas.
+    if (i != size && !IsAsciiDigit(allowed_ports[i]) &&
+        (allowed_ports[i] != kComma))
+      return;
+    if (i == size || allowed_ports[i] == kComma) {
+      size_t length = i - last;
+      if (length > 0)
+        ports.insert(StringToInt(WideToASCII(
+            allowed_ports.substr(last, length))));
+      last = i + 1;
+    }
+  }
+  explicitly_allowed_ports = ports;
+}
+
 }  // namespace net
diff --git a/net/base/net_util.h b/net/base/net_util.h
index 0e247b8..60e5636 100644
--- a/net/base/net_util.h
+++ b/net/base/net_util.h
@@ -12,6 +12,7 @@
 #endif
 
 #include <string>
+#include <set>
 
 #include "base/basictypes.h"
 #include "base/string16.h"
@@ -35,6 +36,9 @@ struct Parsed;
 
 namespace net {
 
+// Holds a list of ports that should be accepted despite bans.
+extern std::set<int> explicitly_allowed_ports;
+
 // Given the full path to a file name, creates a file: URL. The returned URL
 // may not be valid if the input is malformed.
 GURL FilePathToFileURL(const FilePath& path);
@@ -191,6 +195,10 @@ bool IsPortAllowedByDefault(int port);
 // restricted.
 bool IsPortAllowedByFtp(int port);
 
+// Check if banned |port| has been overriden by an entry in
+// |explicitly_allowed_ports_|.
+bool IsPortAllowedByOverride(int port);
+
 // Set socket to non-blocking mode
 int SetNonBlocking(int fd);
 
@@ -230,6 +238,8 @@ inline std::wstring FormatUrl(const GURL& url, const std::wstring& languages) {
 //   - reference section
 GURL SimplifyUrlForRequest(const GURL& url);
 
+void SetExplicitlyAllowedPorts(const std::wstring& allowed_ports);
+
 }  // namespace net
 
 #endif  // NET_BASE_NET_UTIL_H__
diff --git a/net/base/net_util_unittest.cc b/net/base/net_util_unittest.cc
index a36c07f..fed9c0d6 100644
--- a/net/base/net_util_unittest.cc
+++ b/net/base/net_util_unittest.cc
@@ -1313,3 +1313,19 @@ TEST(NetUtilTest, SimplifyUrlForRequest) {
     EXPECT_EQ(expected_url, net::SimplifyUrlForRequest(input_url));
   }
 }
+
+TEST(NetUtilTest, SetExplicitlyAllowedPortsTest) {
+  std::wstring invalid[] = { L"1,2,a", L"'1','2'", L"1, 2, 3", L"1 0,11,12" };
+  std::wstring valid[] = { L"", L"1", L"1,2", L"1,2,3", L"10,11,12,13" };
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(invalid); ++i) {
+    net::SetExplicitlyAllowedPorts(invalid[i]);
+    EXPECT_EQ(0, static_cast<int>(net::explicitly_allowed_ports.size()));
+  }
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(valid); ++i) {
+    net::SetExplicitlyAllowedPorts(valid[i]);
+    EXPECT_EQ(i, net::explicitly_allowed_ports.size());
+  }
+}
+
diff --git a/net/url_request/url_request_ftp_job.cc b/net/url_request/url_request_ftp_job.cc
index c7cb333..eca845a 100644
--- a/net/url_request/url_request_ftp_job.cc
+++ b/net/url_request/url_request_ftp_job.cc
@@ -62,8 +62,10 @@ URLRequestJob* URLRequestFtpJob::Factory(URLRequest* request,
 
   DCHECK(scheme == "ftp");
 
+  int port = request->url().IntPort();
+
   if (request->url().has_port() &&
-      !net::IsPortAllowedByFtp(request->url().IntPort()))
+      !net::IsPortAllowedByFtp(port) && !net::IsPortAllowedByOverride(port))
     return new URLRequestErrorJob(request, net::ERR_UNSAFE_PORT);
 
   return new URLRequestFtpJob(request);
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 71eee28..ada4ad1 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -30,18 +30,14 @@
 #include "net/url_request/url_request_error_job.h"
 #include "net/url_request/url_request_redirect_job.h"
 
-// static
-std::set<int> URLRequestHttpJob::explicitly_allowed_ports_;
-
 // TODO(darin): make sure the port blocking code is not lost
-
 // static
 URLRequestJob* URLRequestHttpJob::Factory(URLRequest* request,
                                           const std::string& scheme) {
   DCHECK(scheme == "http" || scheme == "https");
 
   int port = request->url().IntPort();
-  if (!net::IsPortAllowedByDefault(port) && !IsPortAllowedByOverride(port))
+  if (!net::IsPortAllowedByDefault(port) && !net::IsPortAllowedByOverride(port))
     return new URLRequestErrorJob(request, net::ERR_UNSAFE_PORT);
 
   if (!request->context() ||
@@ -70,35 +66,6 @@ URLRequestJob* URLRequestHttpJob::Factory(URLRequest* request,
   return new URLRequestHttpJob(request);
 }
 
-// static
-void URLRequestHttpJob::SetExplicitlyAllowedPorts(
-    const std::wstring& allowed_ports) {
-  if (allowed_ports.empty())
-    return;
-
-  std::set<int> ports;
-  size_t last = 0;
-  size_t size = allowed_ports.size();
-  // The comma delimiter.
-  const std::wstring::value_type kComma = L',';
-
-  // Overflow is still possible for evil user inputs.
-  for (size_t i = 0; i <= size; ++i) {
-    // The string should be composed of only digits and commas.
-    if (i != size && !IsAsciiDigit(allowed_ports[i]) &&
-        (allowed_ports[i] != kComma))
-      return;
-    if (i == size || allowed_ports[i] == kComma) {
-      size_t length = i - last;
-      if (length > 0)
-        ports.insert(StringToInt(WideToASCII(
-            allowed_ports.substr(last, length))));
-      last = i + 1;
-    }
-  }
-  explicitly_allowed_ports_ = ports;
-}
-
 URLRequestHttpJob::URLRequestHttpJob(URLRequest* request)
     : URLRequestJob(request),
       context_(request->context()),
@@ -378,19 +345,6 @@ void URLRequestHttpJob::RestartTransactionWithAuth(
       this, &URLRequestHttpJob::OnStartCompleted, rv));
 }
 
-// static
-bool URLRequestHttpJob::IsPortAllowedByOverride(int port) {
-  if (explicitly_allowed_ports().empty())
-    return false;
-
-  std::set<int>::const_iterator it =
-      std::find(explicitly_allowed_ports().begin(),
-                explicitly_allowed_ports().end(),
-                port);
-
-  return it != explicitly_allowed_ports().end();
-}
-
 void URLRequestHttpJob::CancelAuth() {
   // Proxy gets set first, then WWW.
   if (proxy_auth_state_ == net::AUTH_STATE_NEED_AUTH) {
diff --git a/net/url_request/url_request_http_job.h b/net/url_request/url_request_http_job.h
index a74d398..f8cc55e 100644
--- a/net/url_request/url_request_http_job.h
+++ b/net/url_request/url_request_http_job.h
@@ -26,12 +26,6 @@ class URLRequestContext;
 class URLRequestHttpJob : public URLRequestJob {
  public:
   static URLRequestJob* Factory(URLRequest* request, const std::string& scheme);
-  // Specifies a comma separated list of port numbers that should be accepted
-  // despite bans. If the string is invalid no allowed ports are stored.
-  static void SetExplicitlyAllowedPorts(const std::wstring& allowed_ports);
-  static const std::set<int>& explicitly_allowed_ports() {
-    return explicitly_allowed_ports_;
-  }
 
   virtual ~URLRequestHttpJob();
 
@@ -85,10 +79,6 @@ class URLRequestHttpJob : public URLRequestJob {
   void RestartTransactionWithAuth(const std::wstring& username,
                                   const std::wstring& password);
 
-  // Check if banned |port| has been overriden by an entry in
-  // |explicitly_allowed_ports_|.
-  static bool IsPortAllowedByOverride(int port);
-
   // Keep a reference to the url request context to be sure it's not deleted
   // before us.
   scoped_refptr<URLRequestContext> context_;
@@ -126,10 +116,6 @@ class URLRequestHttpJob : public URLRequestJob {
   // For recording of stats, we need to remember if this is cached content.
   bool is_cached_content_;
 
- private:
-  // Holds a list of ports that should be accepted despite bans.
-  static std::set<int> explicitly_allowed_ports_;
-
   DISALLOW_COPY_AND_ASSIGN(URLRequestHttpJob);
 };
 
diff --git a/net/url_request/url_request_new_ftp_job.cc b/net/url_request/url_request_new_ftp_job.cc
index f3d8aed..231e654 100644
--- a/net/url_request/url_request_new_ftp_job.cc
+++ b/net/url_request/url_request_new_ftp_job.cc
@@ -80,8 +80,9 @@ URLRequestJob* URLRequestNewFtpJob::Factory(URLRequest* request,
                                             const std::string& scheme) {
   DCHECK_EQ(scheme, "ftp");
 
+  int port = request->url().IntPort();
   if (request->url().has_port() &&
-      !net::IsPortAllowedByFtp(request->url().IntPort()))
+    !net::IsPortAllowedByFtp(port) && !net::IsPortAllowedByOverride(port))
     return new URLRequestErrorJob(request, net::ERR_UNSAFE_PORT);
 
   DCHECK(request->context());
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index bd5c8fa..6ceebc0 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -312,22 +312,6 @@ TEST_F(URLRequestTest, TrackingGraveyardBounded) {
                 .original_url.spec().size());
 }
 
-TEST_F(URLRequestTestHTTP, SetExplicitlyAllowedPortsTest) {
-  std::wstring invalid[] = { L"1,2,a", L"'1','2'", L"1, 2, 3", L"1 0,11,12" };
-  std::wstring valid[] = { L"", L"1", L"1,2", L"1,2,3", L"10,11,12,13" };
-
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(invalid); ++i) {
-    URLRequestHttpJob::SetExplicitlyAllowedPorts(invalid[i]);
-    EXPECT_EQ(0, static_cast<int>(
-        URLRequestHttpJob::explicitly_allowed_ports().size()));
-  }
-
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(valid); ++i) {
-    URLRequestHttpJob::SetExplicitlyAllowedPorts(valid[i]);
-    EXPECT_EQ(i, URLRequestHttpJob::explicitly_allowed_ports().size());
-  }
-}
-
 TEST_F(URLRequestTest, QuitTest) {
   // Don't use shared server here because we order it to quit.
   // It would impact other tests.