Fix geolocation url character escaping issues.

Following on from this CL http://codereview.chromium.org/7718028/

Now escape '\' characters and make all wifi parameters optional.

BUG=None
TEST=--gtest_filter=GeolocationNet*


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

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

2 files changed, 42 insertions, 19 deletions

diff --git a/content/browser/geolocation/network_location_provider_unittest.cc b/content/browser/geolocation/network_location_provider_unittest.cc
index 386f8bd..97b5f75 100644
--- a/content/browser/geolocation/network_location_provider_unittest.cc
+++ b/content/browser/geolocation/network_location_provider_unittest.cc
@@ -181,7 +181,7 @@ class GeolocationNetworkProviderTest : public testing::Test {
       ap.radio_signal_strength = ap_count - i;
       ap.channel = IndexToChannel(i);
       ap.signal_to_noise = i + 42;
-      ap.ssid = ASCIIToUTF16("Some nice+network|name");
+      ap.ssid = ASCIIToUTF16("Some nice+network|name\\");
       data.access_point_data.insert(ap);
     }
     return data;
@@ -193,12 +193,12 @@ class GeolocationNetworkProviderTest : public testing::Test {
     for (int i = 0; i < ap_count; ++i) {
       std::string wifi_part;
       wifi_part += "wifi=";
-      wifi_part += "mac:" + base::StringPrintf("%02d-34-56-78-54-32", i);
-      wifi_part += "%7Css:" + base::IntToString(start_index + ap_count - i);
-      wifi_part += "%7Cage:0";
-      wifi_part += "%7Cchan:" + base::IntToString(IndexToChannel(i));
-      wifi_part += "%7Csnr:" + base::IntToString(i + 42);
-      wifi_part += "%7Cssid:Some%20nice%2Bnetwork%5C%7Cname";
+      wifi_part += "mac%3A" + base::StringPrintf("%02d-34-56-78-54-32", i);
+      wifi_part += "%7Css%3A" + base::IntToString(start_index + ap_count - i);
+      wifi_part += "%7Cage%3A0";
+      wifi_part += "%7Cchan%3A" + base::IntToString(IndexToChannel(i));
+      wifi_part += "%7Csnr%3A" + base::IntToString(i + 42);
+      wifi_part += "%7Cssid%3ASome%20nice%2Bnetwork%5C%7Cname%5C%5C";
       wifi_data.push_back(wifi_part);
     }
     return wifi_data;
@@ -351,7 +351,7 @@ TEST_F(GeolocationNetworkProviderTest, StartProviderLongRequest) {
   // in length by not including access points with the lowest signal strength
   // in the request.
   EXPECT_LT(fetcher->original_url().spec().size(), size_t(2048));
-  CheckRequestIsValid(fetcher->original_url().spec(), 0, 19, 1, "");
+  CheckRequestIsValid(fetcher->original_url().spec(), 0, 16, 4, "");
 }
 
 TEST_F(GeolocationNetworkProviderTest, MultipleStartProvider) {
diff --git a/content/browser/geolocation/network_location_request.cc b/content/browser/geolocation/network_location_request.cc
index 0199f12..8eb8e6f 100644
--- a/content/browser/geolocation/network_location_request.cc
+++ b/content/browser/geolocation/network_location_request.cc
@@ -163,6 +163,28 @@ GURL FormRequestURL(const std::string& url,
   return GURL(request_string);
 }
 
+void AddString(const std::string& property_name, const std::string& value,
+               std::string* wifi_params) {
+  DCHECK(wifi_params);
+  if (!value.empty()) {
+    if (!wifi_params->empty())
+      *wifi_params += '|';
+    *wifi_params += property_name;
+    *wifi_params += value;
+  }
+}
+
+void AddInteger(const std::string& property_name, int value,
+                std::string* wifi_params) {
+  DCHECK(wifi_params);
+  if (value != kint32min) {
+    if (!wifi_params->empty())
+        *wifi_params += '|';
+      *wifi_params += property_name;
+      *wifi_params += base::IntToString(value);
+  }
+}
+
 void AddWifiData(const WifiData& wifi_data,
                  int age_milliseconds,
                  std::vector<std::string>* params) {
@@ -185,21 +207,22 @@ void AddWifiData(const WifiData& wifi_data,
       access_points_by_signal_strength.begin();
       iter != access_points_by_signal_strength.end();
       ++iter) {
-    std::string wifi_params = "wifi=";
-    wifi_params += "mac:" +
-        EscapeQueryParamValue(UTF16ToUTF8((*iter)->mac_address), false);
-    wifi_params += "%7Css:" +
-        base::IntToString((*iter)->radio_signal_strength);
-    if (kint32min != age_milliseconds)
-      wifi_params += "%7Cage:" + base::IntToString(age_milliseconds);
-    wifi_params += "%7Cchan:" + base::IntToString((*iter)->channel);
-    wifi_params += "%7Csnr:" + base::IntToString((*iter)->signal_to_noise);
+    std::string wifi_params;
+    AddString("mac:", UTF16ToUTF8((*iter)->mac_address), &wifi_params);
+    AddInteger("ss:", (*iter)->radio_signal_strength, &wifi_params);
+    AddInteger("age:", age_milliseconds, &wifi_params);
+    AddInteger("chan:", (*iter)->channel, &wifi_params);
+    AddInteger("snr:", (*iter)->signal_to_noise, &wifi_params);
     std::string ssid = UTF16ToUTF8((*iter)->ssid);
+    // Backslash characters in the ssid need backslash-escaping to avoid
+    // escaping a following wifi parameter separator.
+    ReplaceSubstringsAfterOffset(&ssid, 0, "\\", "\\\\");
     // Pipe characters in the ssid need backslash-escaping to avoid being
     // interpreted as the wifi parameter separator.
     ReplaceSubstringsAfterOffset(&ssid, 0, "|", "\\|");
-    wifi_params += "%7Cssid:" + EscapeQueryParamValue(ssid, false);
-    params->push_back(wifi_params);
+    AddString("ssid:", ssid, &wifi_params);
+    params->push_back(
+        "wifi=" + EscapeQueryParamValue(wifi_params, false));
   }
 }