Bug 1334443 - Rewrite nsProtocolProxyService::LoadHostFilters to use Tokenizer r=bagder
authorValentin Gosu <valentin.gosu@gmail.com>
Mon, 20 Mar 2017 20:21:15 +0100
changeset 348598 c64a319909a88eeca02d680f03b74343b238c18e
parent 348597 17ca3c39d1d988548f5c1c89ecdad4856e67c19e
child 348599 dcecf5dc62f64030f31b9f37907a94db3b7483b4
push id31532
push userkwierso@gmail.com
push dateTue, 21 Mar 2017 22:32:51 +0000
treeherdermozilla-central@18bb0299dd9b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbagder
bugs1334443
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1334443 - Rewrite nsProtocolProxyService::LoadHostFilters to use Tokenizer r=bagder 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;
 
@@ -1465,86 +1466,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
@@ -1561,37 +1615,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