Bug 1042521 - Drop some cases when backslashes from urlbar input were converted to slashes on windows. r=bz, a=sledru
authorAlex Bardas <abardas@mozilla.com>
Mon, 25 Aug 2014 14:54:00 +0100
changeset 216800 73202bfb3f03
parent 216799 fa3e1469d0f1
child 216801 4bfa8b78669c
push id3919
push userryanvm@gmail.com
push date2014-09-19 19:44 +0000
treeherdermozilla-beta@0cc0faf4524b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, sledru
bugs1042521
milestone33.0
Bug 1042521 - Drop some cases when backslashes from urlbar input were converted to slashes on windows. r=bz, a=sledru * * * Bug 1042521 - Use original input when calling KeywordToURI and do not trim the input before calling urifixup in tests. r=Gijs
docshell/base/nsDefaultURIFixup.cpp
docshell/test/unit/test_nsDefaultURIFixup_info.js
--- a/docshell/base/nsDefaultURIFixup.cpp
+++ b/docshell/base/nsDefaultURIFixup.cpp
@@ -135,27 +135,29 @@ nsDefaultURIFixup::CreateFixupURI(const 
 
 NS_IMETHODIMP
 nsDefaultURIFixup::GetFixupURIInfo(const nsACString& aStringURI, uint32_t aFixupFlags,
                                    nsIInputStream **aPostData, nsIURIFixupInfo **aInfo)
 {
     NS_ENSURE_ARG(!aStringURI.IsEmpty());
 
     nsresult rv;
-    nsRefPtr<nsDefaultURIFixupInfo> info = new nsDefaultURIFixupInfo(aStringURI);
-    NS_ADDREF(*aInfo = info);
 
     nsAutoCString uriString(aStringURI);
-    uriString.Trim(" ");  // Cleanup the empty spaces that might be on each end.
 
     // Eliminate embedded newlines, which single-line text fields now allow:
     uriString.StripChars("\r\n");
+    // Cleanup the empty spaces that might be on each end:
+    uriString.Trim(" ");
 
     NS_ENSURE_TRUE(!uriString.IsEmpty(), NS_ERROR_FAILURE);
 
+    nsRefPtr<nsDefaultURIFixupInfo> info = new nsDefaultURIFixupInfo(uriString);
+    NS_ADDREF(*aInfo = info);
+
     nsCOMPtr<nsIIOService> ioService = do_GetService(NS_IOSERVICE_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
     nsAutoCString scheme;
     ioService->ExtractScheme(aStringURI, scheme);
     
     // View-source is a pseudo scheme. We're interested in fixing up the stuff
     // after it. The easiest way to do that is to call this method again with the
     // "view-source:" lopped off and then prepend it again afterwards.
@@ -944,17 +946,17 @@ void nsDefaultURIFixup::KeywordURIFixup(
     nsresult rv;
     // We do keyword lookups if a space or quote preceded the dot, colon
     // or question mark (or if the latter were not found):
     if (((spaceLoc < dotLoc || quoteLoc < dotLoc) &&
          (spaceLoc < colonLoc || quoteLoc < colonLoc) &&
          (spaceLoc < qMarkLoc || quoteLoc < qMarkLoc)) ||
         qMarkLoc == 0)
     {
-        rv = KeywordToURI(aURIString, aPostData,
+        rv = KeywordToURI(aFixupInfo->mOriginalInput, aPostData,
                           getter_AddRefs(aFixupInfo->mPreferredURI));
         if (NS_SUCCEEDED(rv) && aFixupInfo->mPreferredURI)
         {
             aFixupInfo->mFixupUsedKeyword = true;
         }
     }
     // ... or if there is no question mark or colon, and there is either no
     // dot, or exactly 1 and it is the first or last character of the input:
@@ -979,17 +981,17 @@ void nsDefaultURIFixup::KeywordURIFixup(
             }
             if (Preferences::GetBool(pref.get(), false))
             {
                 return;
             }
         }
         // If we get here, we don't have a valid URI, or we did but the
         // host is not whitelisted, so we do a keyword search *anyway*:
-        rv = KeywordToURI(aURIString, aPostData,
+        rv = KeywordToURI(aFixupInfo->mOriginalInput, aPostData,
                           getter_AddRefs(aFixupInfo->mPreferredURI));
         if (NS_SUCCEEDED(rv) && aFixupInfo->mPreferredURI)
         {
             aFixupInfo->mFixupUsedKeyword = true;
         }
     }
 }
 
--- a/docshell/test/unit/test_nsDefaultURIFixup_info.js
+++ b/docshell/test/unit/test_nsDefaultURIFixup_info.js
@@ -54,26 +54,41 @@ let testcases = [
   ["http://test./", "http://test./", "http://www.test./", false, false],
   ["127.0.0.1", "http://127.0.0.1/", null, false, true],
   ["1234", "http://1234/", "http://www.1234.com/", true, true],
   ["host/foo.txt", "http://host/foo.txt", "http://www.host.com/foo.txt", false, true],
   ["mozilla", "http://mozilla/", "http://www.mozilla.com/", true, true],
   ["test.", "http://test./", "http://www.test./", true, true],
   [".test", "http://.test/", "http://www..test/", true, true],
   ["mozilla is amazing", null, null, true, true],
+  ["mozilla ", "http://mozilla/", "http://www.mozilla.com/", true, true],
+  ["   mozilla  ", "http://mozilla/", "http://www.mozilla.com/", true, true],
+  ["mozilla \\", null, null, true, true],
+  ["mozilla \\ foo.txt", null, null, true, true],
+  ["mozilla \\\r foo.txt", null, null, true, true],
+  ["mozilla\n", "http://mozilla/", "http://www.mozilla.com/", true, true],
+  ["mozilla \r\n", "http://mozilla/", "http://www.mozilla.com/", true, true],
+  ["moz\r\nfirefox\nos\r", "http://mozfirefoxos/", "http://www.mozfirefoxos.com/", true, true],
+  ["moz\r\n firefox\n", null, null, true, true],
   ["", null, null, true, true],
   ["[]", null, null, true, true]
 ];
 
 if (Services.appinfo.OS.toLowerCase().startsWith("win")) {
   testcases.push(["C:\\some\\file.txt", "file:///C:/some/file.txt", null, false, true]);
   testcases.push(["//mozilla", "http://mozilla/", "http://www.mozilla.com/", false, true]);
+  testcases.push(["mozilla\\", "http://mozilla/", "http://www.mozilla.com/", true, true]);
 } else {
   testcases.push(["/some/file.txt", "file:///some/file.txt", null, false, true]);
   testcases.push(["//mozilla", "file:////mozilla", null, false, true]);
+  testcases.push(["mozilla\\", "http://mozilla\\/", "http://www.mozilla/", true, true]);
+}
+
+function sanitize(input) {
+  return input.replace(/\r|\n/g, "").trim();
 }
 
 function run_test() {
   for (let [testInput, expectedFixedURI, alternativeURI,
             expectKeywordLookup, expectProtocolChange] of testcases) {
     for (let flags of flagInputs) {
       let info;
       let fixupURIOnly = null;
@@ -112,20 +127,20 @@ function run_test() {
       // Check booleans on input:
       let couldDoKeywordLookup = flags & urifixup.FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP;
       do_check_eq(info.fixupUsedKeyword, couldDoKeywordLookup && expectKeywordLookup);
       do_check_eq(info.fixupChangedProtocol, expectProtocolChange);
       do_check_eq(info.fixupCreatedAlternateURI, makeAlternativeURI && alternativeURI != null);
 
       // Check the preferred URI
       if (couldDoKeywordLookup && expectKeywordLookup) {
-        let urlparamInput = encodeURIComponent(testInput).replace("%20", "+", "g");
+        let urlparamInput = encodeURIComponent(sanitize(testInput)).replace("%20", "+", "g");
         let searchURL = kSearchEngineURL.replace("{searchTerms}", urlparamInput);
         do_check_eq(info.preferredURI.spec, searchURL);
       } else {
         // In these cases, we should never be doing a keyword lookup and
         // the fixed URI should be preferred:
         do_check_eq(info.preferredURI.spec, info.fixedURI.spec);
       }
-      do_check_eq(testInput, info.originalInput);
+      do_check_eq(sanitize(testInput), info.originalInput);
     }
   }
 }