Sanitize https:// URLs before sending them to PAC scripts.

net/proxy/proxy_service.h

Index: net/proxy/proxy_service.h
diff --git a/net/proxy/proxy_service.h b/net/proxy/proxy_service.h
index 635d26dbe2a71f7bd7777392f4e2256f43b101e6..3149ab3aecf51eaf04baef8c1134aed57f8cc445 100644
--- a/net/proxy/proxy_service.h
+++ b/net/proxy/proxy_service.h
@@ -25,6 +25,7 @@
 #include "net/proxy/proxy_config_service.h"
 #include "net/proxy/proxy_info.h"
 #include "net/proxy/proxy_server.h"
+#include "url/gurl.h"
 
 class GURL;
 
@@ -52,6 +53,25 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver,
                                 public ProxyConfigService::Observer,
                                 NON_EXPORTED_BASE(public base::NonThreadSafe) {
  public:
+  // Enumerates the policy to use when sanitizing URLs for proxy resolution
+  // (before passing them off to PAC scripts).
+  enum class SanitizeUrlPolicy {
+    // Do a basic level of sanitization for URLs:
+    //   - strip embedded identities (ex: "username:password@")
+    //   - strip the fragment (ex: "#blah")
+    //
+    // This is considered "unsafe" because it does not do any additional
+    // stripping for https:// URLs.
+    UNSAFE,
+
+    // SAFE does the same sanitization as UNSAFE, but additionally strips
+    // everything but the (scheme,host,port) from cryptographic URL schemes
+    // (https:// and wss://).
+    //
+    // In other words, it strips the path and query portion of https:// URLs.
+    SAFE,
+  };
+
   static const size_t kDefaultNumPacThreads = 4;
 
   // This interface defines the set of policies for when to poll the PAC
@@ -296,6 +316,15 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver,
     quick_check_enabled_ = value;
   }
 
+  void set_sanitize_url_policy(SanitizeUrlPolicy policy) {
+    sanitize_url_policy_ = policy;
+  }
+
+  // Returns a sanitized copy of |url| which is safe to pass on to a PAC script.
+  // The method for sanitizing is determined by |policy|. See the comments for
+  // that enum for details.
+  NET_EXPORT static GURL SanitizeUrl(const GURL& url, SanitizeUrlPolicy policy);

[mmenke, 2016/05/20 17:40:23]

Should this be NET_EXPORT_PRIVATE?  Not really concerned about external use,
just not sure we really expect it.

[eroman, 2016/05/20 21:23:53]

No longer applicable, as I moved the function to be internal-only.

That said, I am not a fan of NET_EXPORT_PRIVATE for the reasons given in
https://bugs.chromium.org/p/chromium/issues/detail?id=552248

(My pessimistic view is that its value erodes with time as consumers invariably
depend on it, making it little more than a confusing out-of-date comment).

+
  private:
   FRIEND_TEST_ALL_PREFIXES(ProxyServiceTest, UpdateConfigAfterFailedAutodetect);
   FRIEND_TEST_ALL_PREFIXES(ProxyServiceTest, UpdateConfigFromPACToDirect);
@@ -460,6 +489,9 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver,
   // Whether child ProxyScriptDeciders should use QuickCheck
   bool quick_check_enabled_;
 
+  // The method to use for sanitizing URLs seen by the proxy resolver.
+  SanitizeUrlPolicy sanitize_url_policy_;
+
   DISALLOW_COPY_AND_ASSIGN(ProxyService);
 };