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..2297f9bddc7e3ebc6d8b9d7266f0d9416770f9bb 100644
--- a/net/proxy/proxy_service.h
+++ b/net/proxy/proxy_service.h
@@ -44,6 +44,31 @@ class ProxyResolverScriptData;
 class ProxyScriptDecider;
 class ProxyScriptFetcher;
 
+// Enumerates the policy to use when sanitizing URLs for proxy resolution
+// (before passing them off to PAC scripts).
+enum class SanitizeUrlForPacScriptPolicy {
+  // 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,
+};
+
+// 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 GURL SanitizeUrlForPacScript(const GURL& url,

[mmenke, 2016/05/19 22:33:23]

Do you need to include gurl.h when returning a GURL?

[eroman, 2016/05/19 23:26:05]

Done.

+                                        SanitizeUrlForPacScriptPolicy policy);

[mmenke, 2016/05/19 22:33:24]

Not a big fan of bonus enums and methods hanging out in a class's header file. 
Can we just move these into ProxyService?

[eroman, 2016/05/19 23:26:05]

Done.

+
 // This class can be used to resolve the proxy server to use when loading a
 // HTTP(S) URL.  It uses the given ProxyResolver to handle the actual proxy
 // resolution.  See ProxyResolverV8 for example.
@@ -296,6 +321,11 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver,
     quick_check_enabled_ = value;
   }
 
+  void set_sanitize_url_for_pac_script_policy(
+      SanitizeUrlForPacScriptPolicy policy) {
+    sanitize_url_for_pac_script_policy_ = policy;
+  }
+
  private:
   FRIEND_TEST_ALL_PREFIXES(ProxyServiceTest, UpdateConfigAfterFailedAutodetect);
   FRIEND_TEST_ALL_PREFIXES(ProxyServiceTest, UpdateConfigFromPACToDirect);
@@ -460,6 +490,9 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver,
   // Whether child ProxyScriptDeciders should use QuickCheck
   bool quick_check_enabled_;
 
+  SanitizeUrlForPacScriptPolicy sanitize_url_for_pac_script_policy_ =
+      SanitizeUrlForPacScriptPolicy::SAFE;

[mmenke, 2016/05/19 22:33:23]

nit:  Seems weird to me to mix inline simple initialization with initialization
in the constructor.  I'm fine with either pattern, just don't like
mixing-and-matching.

+
   DISALLOW_COPY_AND_ASSIGN(ProxyService);
 };