Bug 1334443 - Rewrite nsProtocolProxyService::LoadHostFilters to use Tokenizer. r=bagder, a=gchang
authorValentin Gosu <valentin.gosu@gmail.com>
Mon, 20 Mar 2017 20:21:15 +0100
changeset 395488 cbacf5b8b2f1cfe92d6f1ffb4d9953b0400e83a8
parent 395487 1067ca60736c13db4282e9697f8e96f396ee8073
child 395489 f15990bc60cc6dbb11bb87c733e901ac4cb7bc69
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbagder, gchang
bugs1334443
milestone54.0a2
Bug 1334443 - Rewrite nsProtocolProxyService::LoadHostFilters to use Tokenizer. r=bagder, a=gchang This eliminates some of the pointer math and makes the method a bit safer. The function's behaviour remains the same as before. MozReview-Commit-ID: 94wBk6xvkd6
netwerk/base/nsProtocolProxyService.cpp
netwerk/base/nsProtocolProxyService.h
netwerk/test/gtest/TestProtocolProxyService.cpp
--- a/netwerk/base/nsProtocolProxyService.cpp
+++ b/netwerk/base/nsProtocolProxyService.cpp
@@ -31,16 +31,17 @@
 #include "nsPACMan.h"
 #include "nsProxyRelease.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/CondVar.h"
 #include "nsISystemProxySettings.h"
 #include "nsINetworkLinkService.h"
 #include "nsIHttpChannelInternal.h"
 #include "mozilla/Logging.h"
+#include "mozilla/Tokenizer.h"
 
 //----------------------------------------------------------------------------
 
 namespace mozilla {
 namespace net {
 
   extern const char kProxyType_HTTP[];
   extern const char kProxyType_HTTPS[];
@@ -641,17 +642,17 @@ nsProtocolProxyService::PrefsChanged(nsI
     if (!pref || !strcmp(pref, PROXY_PREF("failover_timeout")))
         proxy_GetIntPref(prefBranch, PROXY_PREF("failover_timeout"),
                          mFailedProxyTimeout);
 
     if (!pref || !strcmp(pref, PROXY_PREF("no_proxies_on"))) {
         rv = prefBranch->GetCharPref(PROXY_PREF("no_proxies_on"),
                                      getter_Copies(tempString));
         if (NS_SUCCEEDED(rv))
-            LoadHostFilters(tempString.get());
+            LoadHostFilters(tempString);
     }
 
     // We're done if not using something that could give us a PAC URL
     // (PAC, WPAD or System)
     if (mProxyConfig != PROXYCONFIG_PAC && mProxyConfig != PROXYCONFIG_WPAD &&
         mProxyConfig != PROXYCONFIG_SYSTEM)
         return;
 
@@ -1460,86 +1461,139 @@ nsProtocolProxyService::UnregisterChanne
 NS_IMETHODIMP
 nsProtocolProxyService::GetProxyConfigType(uint32_t* aProxyConfigType)
 {
   *aProxyConfigType = mProxyConfig;
   return NS_OK;
 }
 
 void
-nsProtocolProxyService::LoadHostFilters(const char *filters)
+nsProtocolProxyService::LoadHostFilters(const nsACString& aFilters)
 {
     // check to see the owners flag? /!?/ TODO
     if (mHostFiltersArray.Length() > 0) {
         mHostFiltersArray.Clear();
     }
 
-    if (!filters)
-        return; // fail silently...
+    if (aFilters.IsEmpty()) {
+        return;
+    }
 
     //
     // filter  = ( host | domain | ipaddr ["/" mask] ) [":" port]
     // filters = filter *( "," LWS filter)
     //
     // Reset mFilterLocalHosts - will be set to true if "<local>" is in pref string
     mFilterLocalHosts = false;
-    while (*filters) {
+
+    mozilla::Tokenizer t(aFilters);
+    mozilla::Tokenizer::Token token;
+    bool eof = false;
+    // while (*filters) {
+    while (!eof) {
         // skip over spaces and ,
-        while (*filters && (*filters == ',' || IS_ASCII_SPACE(*filters)))
-            filters++;
-
-        const char *starthost = filters;
-        const char *endhost = filters + 1; // at least that...
-        const char *portLocation = 0;
-        const char *maskLocation = 0;
-
-        while (*endhost && (*endhost != ',' && !IS_ASCII_SPACE(*endhost))) {
-            if (*endhost == ':')
-                portLocation = endhost;
-            else if (*endhost == '/')
-                maskLocation = endhost;
-            else if (*endhost == ']') // IPv6 address literals
-                portLocation = 0;
-            endhost++;
+        t.SkipWhites();
+        while (t.CheckChar(',')) {
+            t.SkipWhites();
         }
 
-        filters = endhost; // advance iterator up front
+        nsAutoCString portStr;
+        nsAutoCString hostStr;
+        nsAutoCString maskStr;
+        t.Record();
+
+        bool parsingIPv6 = false;
+        bool parsingPort = false;
+        bool parsingMask = false;
+        while (t.Next(token)) {
+            if (token.Equals(mozilla::Tokenizer::Token::EndOfFile()))  {
+                eof = true;
+                break;
+            }
+            if (token.Equals(mozilla::Tokenizer::Token::Char(',')) ||
+                token.Type() == mozilla::Tokenizer::TOKEN_WS) {
+                break;
+            }
+
+            if (token.Equals(mozilla::Tokenizer::Token::Char('['))) {
+                parsingIPv6 = true;
+                continue;
+            }
 
-        // locate end of host
-        const char *end = maskLocation ? maskLocation :
-                          portLocation ? portLocation :
-                          endhost;
+            if (!parsingIPv6 && token.Equals(mozilla::Tokenizer::Token::Char(':'))) {
+                // Port is starting. Claim the previous as host.
+                if (parsingMask) {
+                    t.Claim(maskStr);
+                } else {
+                    t.Claim(hostStr);
+                }
+                t.Record();
+                parsingPort = true;
+                continue;
+            } else if (token.Equals(mozilla::Tokenizer::Token::Char('/'))) {
+                t.Claim(hostStr);
+                t.Record();
+                parsingMask = true;
+                continue;
+            } else if (token.Equals(mozilla::Tokenizer::Token::Char(']'))) {
+                parsingIPv6 = false;
+                continue;
+            }
+        }
+        if (!parsingPort && !parsingMask) {
+            t.Claim(hostStr);
+        } else if (parsingPort) {
+            t.Claim(portStr);
+        } else if (parsingMask) {
+            t.Claim(maskStr);
+        } else {
+            NS_WARNING("Could not parse this rule");
+            continue;
+        }
 
-        nsAutoCString str(starthost, end - starthost);
+        if (hostStr.IsEmpty()) {
+            continue;
+        }
 
         // If the current host filter is "<local>", then all local (i.e.
         // no dots in the hostname) hosts should bypass the proxy
-        if (str.EqualsIgnoreCase("<local>")) {
+        if (hostStr.EqualsIgnoreCase("<local>")) {
             mFilterLocalHosts = true;
             LOG(("loaded filter for local hosts "
                  "(plain host names, no dots)\n"));
             // Continue to next host filter;
             continue;
         }
 
         // For all other host filters, create HostInfo object and add to list
         HostInfo *hinfo = new HostInfo();
-        hinfo->port = portLocation ? atoi(portLocation + 1) : 0;
+        nsresult rv = NS_OK;
+
+        int32_t port = portStr.ToInteger(&rv);
+        if (NS_FAILED(rv)) {
+            port = 0;
+        }
+        hinfo->port = port;
+
+        int32_t maskLen = maskStr.ToInteger(&rv);
+        if (NS_FAILED(rv)) {
+            maskLen = 128;
+        }
 
         // PR_StringToNetAddr can't parse brackets enclosed IPv6
-        nsAutoCString addrString = str;
-        if (str.Length() > 0 && str.First() == '[' && str.Last() == ']') {
-            addrString = Substring(str, 1, str.Length() - 2);
+        nsAutoCString addrString = hostStr;
+        if (hostStr.First() == '[' && hostStr.Last() == ']') {
+            addrString = Substring(hostStr, 1, hostStr.Length() - 2);
         }
 
         PRNetAddr addr;
         if (PR_StringToNetAddr(addrString.get(), &addr) == PR_SUCCESS) {
             hinfo->is_ipaddr   = true;
             hinfo->ip.family   = PR_AF_INET6; // we always store address as IPv6
-            hinfo->ip.mask_len = maskLocation ? atoi(maskLocation + 1) : 128;
+            hinfo->ip.mask_len = maskLen;
 
             if (hinfo->ip.mask_len == 0) {
                 NS_WARNING("invalid mask");
                 goto loser;
             }
 
             if (addr.raw.family == PR_AF_INET) {
                 // convert to IPv4-mapped address
@@ -1556,37 +1610,43 @@ nsProtocolProxyService::LoadHostFilters(
                 NS_WARNING("unknown address family");
                 goto loser;
             }
 
             // apply mask to IPv6 address
             proxy_MaskIPv6Addr(hinfo->ip.addr, hinfo->ip.mask_len);
         }
         else {
-            uint32_t startIndex, endIndex;
-            if (str.First() == '*')
-                startIndex = 1; // *.domain -> .domain
-            else
-                startIndex = 0;
-            endIndex = (portLocation ? portLocation : endhost) - starthost;
+            nsAutoCString host;
+            if (hostStr.First() == '*') {
+                host = Substring(hostStr, 1);
+            } else {
+                host = hostStr;
+            }
+
+            if (host.IsEmpty()) {
+                hinfo->name.host = nullptr;
+                goto loser;
+            }
+
+            hinfo->name.host_len = host.Length();
 
             hinfo->is_ipaddr = false;
-            hinfo->name.host = ToNewCString(Substring(str, startIndex, endIndex));
+            hinfo->name.host = ToNewCString(host);
 
             if (!hinfo->name.host)
                 goto loser;
-
-            hinfo->name.host_len = endIndex - startIndex;
         }
 
 //#define DEBUG_DUMP_FILTERS
 #ifdef DEBUG_DUMP_FILTERS
         printf("loaded filter[%zu]:\n", mHostFiltersArray.Length());
         printf("  is_ipaddr = %u\n", hinfo->is_ipaddr);
         printf("  port = %u\n", hinfo->port);
+        printf("  host = %s\n", hostStr.get());
         if (hinfo->is_ipaddr) {
             printf("  ip.family = %x\n", hinfo->ip.family);
             printf("  ip.mask_len = %u\n", hinfo->ip.mask_len);
 
             PRNetAddr netAddr;
             PR_SetNetAddr(PR_IpAddrNull, PR_AF_INET6, 0, &netAddr);
             memcpy(&netAddr.ipv6.ip, &hinfo->ip.addr, sizeof(hinfo->ip.addr));
 
--- a/netwerk/base/nsProtocolProxyService.h
+++ b/netwerk/base/nsProtocolProxyService.h
@@ -269,17 +269,17 @@ protected:
                                     nsIProxyInfo **proxyInfo);
 
     /**
      * This method populates mHostFiltersArray from the given string.
      *
      * @param hostFilters
      *        A "no-proxy-for" exclusion list.
      */
-    void LoadHostFilters(const char *hostFilters);
+    void LoadHostFilters(const nsACString& hostFilters);
 
     /**
      * This method checks the given URI against mHostFiltersArray.
      *
      * @param uri
      *        The URI to test.
      * @param defaultPort
      *        The default port for the given URI.
--- a/netwerk/test/gtest/TestProtocolProxyService.cpp
+++ b/netwerk/test/gtest/TestProtocolProxyService.cpp
@@ -83,41 +83,46 @@ TEST(TestProtocolProxyService, LoadHostF
   };
 
   // --------------------------------------------------------------------------
 
   nsAutoCString filter;
 
   // Anything is allowed when there are no filters set
   printf("Testing empty filter: %s\n", filter.get());
-  pps->LoadHostFilters(filter.get());
+  pps->LoadHostFilters(filter);
 
   CheckLoopbackURLs(true); // only time when loopbacks can be proxied. bug?
   CheckLocalDomain(true);
   CheckURLs(true);
   CheckPortDomain(true);
 
   // --------------------------------------------------------------------------
 
   filter = "example.com, 1.2.3.4/16, [2001::1], 10.0.0.0/8, 2.3.0.0/16:7777, [abcd::1]/64:123, *.test.com";
   printf("Testing filter: %s\n", filter.get());
-  pps->LoadHostFilters(filter.get());
+  pps->LoadHostFilters(filter);
   // Check URLs can no longer use filtered proxy
   CheckURLs(false);
   CheckLoopbackURLs(false);
   CheckLocalDomain(true);
   CheckPortDomain(true);
 
   // --------------------------------------------------------------------------
 
   // This is space separated. See bug 1346711 comment 4. We check this to keep
   // backwards compatibility.
   filter = "<local> blabla.com:10";
   printf("Testing filter: %s\n", filter.get());
-  pps->LoadHostFilters(filter.get());
+  pps->LoadHostFilters(filter);
   CheckURLs(true);
   CheckLoopbackURLs(false);
   CheckLocalDomain(false);
   CheckPortDomain(false);
+
+  // Check that we don't crash on weird input
+  filter = "a b c abc:1x2, ,, * ** *.* *:10 :20 :40/12 */12:90";
+  printf("Testing filter: %s\n", filter.get());
+  pps->LoadHostFilters(filter);
 }
 
 } // namespace net
 } // namespace mozila