Bug 1172080 - Part 2: Use ^ instead of ! to delimit originAttributes from the URI in nsIPrincipal.origin, r=bholley
☠☠ backed out by de3fb216f066 ☠ ☠
authorMichael Layzell <michael@thelayzells.com>
Thu, 02 Jul 2015 16:50:26 -0400
changeset 286192 84fe04b2e7d15f1dd7779996001e6b2e5d7df342
parent 286191 0ff004760a1ff67fdce8b410910001b6fdbdfc06
child 286193 5f190b448f83719f5a56f6e6c0a1a13746c9a91f
push id934
push userraliiev@mozilla.com
push dateMon, 26 Oct 2015 12:58:05 +0000
treeherdermozilla-release@05704e35c1d0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1172080
milestone42.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 1172080 - Part 2: Use ^ instead of ! to delimit originAttributes from the URI in nsIPrincipal.origin, r=bholley
caps/BasePrincipal.cpp
caps/tests/unit/test_origin.js
dom/apps/tests/unit/test_moziapplication.js
extensions/cookie/test/unit/test_permmanager_defaults.js
extensions/cookie/test/unit/test_permmanager_migrate_4-5.js
toolkit/modules/BrowserUtils.jsm
--- a/caps/BasePrincipal.cpp
+++ b/caps/BasePrincipal.cpp
@@ -46,17 +46,17 @@ OriginAttributes::CreateSuffix(nsACStrin
   if (!mAddonId.IsEmpty()) {
     params->Set(NS_LITERAL_STRING("addonId"), mAddonId);
   }
 
   aStr.Truncate();
 
   params->Serialize(value);
   if (!value.IsEmpty()) {
-    aStr.AppendLiteral("!");
+    aStr.AppendLiteral("^");
     aStr.Append(NS_ConvertUTF16toUTF8(value));
   }
 }
 
 namespace {
 
 class MOZ_STACK_CLASS PopulateFromSuffixIterator final
   : public URLParams::ForEachIterator
@@ -112,34 +112,34 @@ private:
 
 bool
 OriginAttributes::PopulateFromSuffix(const nsACString& aStr)
 {
   if (aStr.IsEmpty()) {
     return true;
   }
 
-  if (aStr[0] != '!') {
+  if (aStr[0] != '^') {
     return false;
   }
 
   UniquePtr<URLParams> params(new URLParams());
   params->ParseInput(Substring(aStr, 1, aStr.Length() - 1));
 
   PopulateFromSuffixIterator iterator(this);
   return params->ForEach(iterator);
 }
 
 bool
 OriginAttributes::PopulateFromOrigin(const nsACString& aOrigin,
                                      nsACString& aOriginNoSuffix)
 {
   // RFindChar is only available on nsCString.
   nsCString origin(aOrigin);
-  int32_t pos = origin.RFindChar('!');
+  int32_t pos = origin.RFindChar('^');
 
   if (pos == kNotFound) {
     aOriginNoSuffix = origin;
     return true;
   }
 
   aOriginNoSuffix = Substring(origin, 0, pos);
   return PopulateFromSuffix(Substring(origin, pos));
--- a/caps/tests/unit/test_origin.js
+++ b/caps/tests/unit/test_origin.js
@@ -63,43 +63,43 @@ function run_test() {
 
   //
   // Test origin attributes.
   //
 
   // Just app.
   var exampleOrg_app = ssm.createCodebasePrincipal(makeURI('http://example.org'), {appId: 42});
   var nullPrin_app = ssm.createNullPrincipal({appId: 42});
-  checkOriginAttributes(exampleOrg_app, {appId: 42}, '!appId=42');
-  checkOriginAttributes(nullPrin_app, {appId: 42}, '!appId=42');
-  do_check_eq(exampleOrg_app.origin, 'http://example.org!appId=42');
+  checkOriginAttributes(exampleOrg_app, {appId: 42}, '^appId=42');
+  checkOriginAttributes(nullPrin_app, {appId: 42}, '^appId=42');
+  do_check_eq(exampleOrg_app.origin, 'http://example.org^appId=42');
 
   // Just browser.
   var exampleOrg_browser = ssm.createCodebasePrincipal(makeURI('http://example.org'), {inBrowser: true});
   var nullPrin_browser = ssm.createNullPrincipal({inBrowser: true});
-  checkOriginAttributes(exampleOrg_browser, {inBrowser: true}, '!inBrowser=1');
-  checkOriginAttributes(nullPrin_browser, {inBrowser: true}, '!inBrowser=1');
-  do_check_eq(exampleOrg_browser.origin, 'http://example.org!inBrowser=1');
+  checkOriginAttributes(exampleOrg_browser, {inBrowser: true}, '^inBrowser=1');
+  checkOriginAttributes(nullPrin_browser, {inBrowser: true}, '^inBrowser=1');
+  do_check_eq(exampleOrg_browser.origin, 'http://example.org^inBrowser=1');
 
   // App and browser.
   var exampleOrg_appBrowser = ssm.createCodebasePrincipal(makeURI('http://example.org'), {inBrowser: true, appId: 42});
   var nullPrin_appBrowser = ssm.createNullPrincipal({inBrowser: true, appId: 42});
-  checkOriginAttributes(exampleOrg_appBrowser, {appId: 42, inBrowser: true}, '!appId=42&inBrowser=1');
-  checkOriginAttributes(nullPrin_appBrowser, {appId: 42, inBrowser: true}, '!appId=42&inBrowser=1');
-  do_check_eq(exampleOrg_appBrowser.origin, 'http://example.org!appId=42&inBrowser=1');
+  checkOriginAttributes(exampleOrg_appBrowser, {appId: 42, inBrowser: true}, '^appId=42&inBrowser=1');
+  checkOriginAttributes(nullPrin_appBrowser, {appId: 42, inBrowser: true}, '^appId=42&inBrowser=1');
+  do_check_eq(exampleOrg_appBrowser.origin, 'http://example.org^appId=42&inBrowser=1');
 
   // App and browser, different domain.
   var exampleCom_appBrowser = ssm.createCodebasePrincipal(makeURI('https://www.example.com:123'), {appId: 42, inBrowser: true});
-  checkOriginAttributes(exampleCom_appBrowser, {appId: 42, inBrowser: true}, '!appId=42&inBrowser=1');
-  do_check_eq(exampleCom_appBrowser.origin, 'https://www.example.com:123!appId=42&inBrowser=1');
+  checkOriginAttributes(exampleCom_appBrowser, {appId: 42, inBrowser: true}, '^appId=42&inBrowser=1');
+  do_check_eq(exampleCom_appBrowser.origin, 'https://www.example.com:123^appId=42&inBrowser=1');
 
   // Addon.
   var exampleOrg_addon = ssm.createCodebasePrincipal(makeURI('http://example.org'), {addonId: 'dummy'});
-  checkOriginAttributes(exampleOrg_addon, { addonId: "dummy" }, '!addonId=dummy');
-  do_check_eq(exampleOrg_addon.origin, 'http://example.org!addonId=dummy');
+  checkOriginAttributes(exampleOrg_addon, { addonId: "dummy" }, '^addonId=dummy');
+  do_check_eq(exampleOrg_addon.origin, 'http://example.org^addonId=dummy');
 
   // Check that all of the above are cross-origin.
   checkCrossOrigin(exampleOrg_app, exampleOrg);
   checkCrossOrigin(exampleOrg_app, nullPrin_app);
   checkCrossOrigin(exampleOrg_browser, exampleOrg_app);
   checkCrossOrigin(exampleOrg_browser, nullPrin_browser);
   checkCrossOrigin(exampleOrg_appBrowser, exampleOrg_app);
   checkCrossOrigin(exampleOrg_appBrowser, nullPrin_appBrowser);
--- a/dom/apps/tests/unit/test_moziapplication.js
+++ b/dom/apps/tests/unit/test_moziapplication.js
@@ -46,17 +46,17 @@ add_test(() => {
     if (key == "principal") {
       return;
     }
     Assert.equal(app[key], mozapp[key],
                  "app[" + key + "] should be equal to mozapp[" + key + "]");
   });
 
   Assert.ok(mozapp.principal, "app principal should exist");
-  let expectedPrincipalOrigin = app.origin + "!appId=" + app.localId;
+  let expectedPrincipalOrigin = app.origin + "^appId=" + app.localId;
   Assert.equal(mozapp.principal.origin, expectedPrincipalOrigin,
                "app principal origin ok");
   Assert.equal(mozapp.principal.appId, app.localId, "app principal appId ok");
   Assert.equal(mozapp.principal.isInBrowserElement, false,
                "app principal isInBrowserElement ok");
   run_next_test();
 });
 
--- a/extensions/cookie/test/unit/test_permmanager_defaults.js
+++ b/extensions/cookie/test/unit/test_permmanager_defaults.js
@@ -35,17 +35,17 @@ add_task(function* do_test() {
              createInstance(Ci.nsIConverterOutputStream);
   conv.init(ostream, "UTF-8", 0, 0);
 
   conv.writeString("# this is a comment\n");
   conv.writeString("\n"); // a blank line!
   conv.writeString("host\t" + TEST_PERMISSION + "\t1\t" + TEST_ORIGIN.host + "\n");
   conv.writeString("host\t" + TEST_PERMISSION + "\t1\t" + TEST_ORIGIN_2.host + "\n");
   conv.writeString("origin\t" + TEST_PERMISSION + "\t1\t" + TEST_ORIGIN_3.spec + "\n");
-  conv.writeString("origin\t" + TEST_PERMISSION + "\t1\t" + TEST_ORIGIN.spec + "!appId=1000&inBrowser=1\n");
+  conv.writeString("origin\t" + TEST_PERMISSION + "\t1\t" + TEST_ORIGIN.spec + "^appId=1000&inBrowser=1\n");
   ostream.close();
 
   // Set the preference used by the permission manager so the file is read.
   Services.prefs.setCharPref("permissions.manager.defaultsUrl", "file://" + file.path);
 
   // initialize the permission manager service - it will read that default.
   let pm = Cc["@mozilla.org/permissionmanager;1"].
            getService(Ci.nsIPermissionManager);
--- a/extensions/cookie/test/unit/test_permmanager_migrate_4-5.js
+++ b/extensions/cookie/test/unit/test_permmanager_migrate_4-5.js
@@ -89,43 +89,43 @@ add_task(function test() {
   db = null;
 
   let expected = [
     // The http:// entries under foo.com won't be inserted, as there are history entries for foo.com,
     // and http://foo.com or a subdomain are never visited.
     // However, permissions for subdomains of foo.com will be present for both http:// and https://,
     // as they do not apply to any entry in the history
     // ["http://foo.com", "A", 1, 0, 0],
-    // ["http://foo.com!appId=1000", "A", 1, 0, 0],
-    // ["http://foo.com!appId=2000&inBrowser=1", "A", 1, 0, 0],
+    // ["http://foo.com^appId=1000", "A", 1, 0, 0],
+    // ["http://foo.com^appId=2000&inBrowser=1", "A", 1, 0, 0],
 
     ["http://sub.foo.com", "B", 1, 0, 0],
     ["http://subber.sub.foo.com", "B", 1, 0, 0],
 
     ["https://foo.com", "A", 1, 0, 0],
-    ["https://foo.com!appId=1000", "A", 1, 0, 0],
-    ["https://foo.com!appId=2000&inBrowser=1", "A", 1, 0, 0],
+    ["https://foo.com^appId=1000", "A", 1, 0, 0],
+    ["https://foo.com^appId=2000&inBrowser=1", "A", 1, 0, 0],
     ["https://sub.foo.com", "B", 1, 0, 0],
     ["https://subber.sub.foo.com", "B", 1, 0, 0],
 
     // bar.ca will have both http:// and https:// for all entries, because the foo did the bar a favour
     ["http://bar.ca", "B", 1, 0, 0],
     ["https://bar.ca", "B", 1, 0, 0],
-    ["http://bar.ca!appId=1000", "B", 1, 0, 0],
-    ["https://bar.ca!appId=1000", "B", 1, 0, 0],
-    ["http://bar.ca!appId=1000&inBrowser=1", "A", 1, 0, 0],
-    ["https://bar.ca!appId=1000&inBrowser=1", "A", 1, 0, 0],
+    ["http://bar.ca^appId=1000", "B", 1, 0, 0],
+    ["https://bar.ca^appId=1000", "B", 1, 0, 0],
+    ["http://bar.ca^appId=1000&inBrowser=1", "A", 1, 0, 0],
+    ["https://bar.ca^appId=1000&inBrowser=1", "A", 1, 0, 0],
     ["file:///some/path/to/file.html", "A", 1, 0, 0],
     ["file:///another/file.html", "A", 1, 0, 0],
 
     // Because we put ftp://some.subdomain.of.foo.com:8000/some/subdirectory in the history, we should
     // also have these entries
     ["ftp://foo.com:8000", "A", 1, 0, 0],
-    ["ftp://foo.com:8000!appId=1000", "A", 1, 0, 0],
-    ["ftp://foo.com:8000!appId=2000&inBrowser=1", "A", 1, 0, 0],
+    ["ftp://foo.com:8000^appId=1000", "A", 1, 0, 0],
+    ["ftp://foo.com:8000^appId=2000&inBrowser=1", "A", 1, 0, 0],
   ];
 
   let found = expected.map((it) => 0);
 
   // Add some places to the places database
   yield PlacesTestUtils.addVisits(Services.io.newURI("https://foo.com/some/other/subdirectory", null, null));
   yield PlacesTestUtils.addVisits(Services.io.newURI("ftp://some.subdomain.of.foo.com:8000/some/subdirectory", null, null));
 
--- a/toolkit/modules/BrowserUtils.jsm
+++ b/toolkit/modules/BrowserUtils.jsm
@@ -100,17 +100,17 @@ this.BrowserUtils = {
     if (aOriginString.startsWith('[')) {
       throw new Error("principalFromOrigin does not support System and Expanded principals");
     }
 
     if (aOriginString.startsWith("moz-nullprincipal:")) {
       throw new Error("principalFromOrigin does not support nsNullPrincipal");
     }
 
-    var parts = aOriginString.split('!');
+    var parts = aOriginString.split('^');
     if (parts.length > 2) {
       throw new Error("bad origin string: " + aOriginString);
     }
 
     var uri = Services.io.newURI(parts[0], null, null);
     var attrs = {};
     // Parse the parameters string into a dictionary.
     (parts[1] || "").split("&").map((x) => x.split('=')).forEach((x) => attrs[x[0]] = x[1]);