Bug 815640 - Make permission manager aware of "file://". r=sicking

extensions/cookie/nsPermissionManager.cpp

@@ -87,9 +87,14 @@ GetPrincipal(const nsACString& aHost, uint32_t aAppId, bool aIsInBrowserElement,
   NS_ENSURE_TRUE(secMan, NS_ERROR_FAILURE);
 
   nsCOMPtr<nsIURI> uri;
-  // NOTE: we use "http://" as a protocal but we will just use the host so it
-  // doesn't really matter.
-  NS_NewURI(getter_AddRefs(uri), NS_LITERAL_CSTRING("http://") + aHost);
+  nsresult rv = NS_NewURI(getter_AddRefs(uri), aHost);
+  if (NS_FAILED(rv)) {
+    // NOTE: most callers will end up here because we don't append "http://" for
+    // hosts. It's fine to arbitrary use "http://" because, for those entries,
+    // we will actually just use the host.
+    rv = NS_NewURI(getter_AddRefs(uri), NS_LITERAL_CSTRING("http://") + aHost);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
 
   return secMan->GetAppCodebasePrincipal(uri, aAppId, aIsInBrowserElement, aPrincipal);
 }
@@ -120,11 +125,17 @@ GetHostForPrincipal(nsIPrincipal* aPrincipal, nsACString& aHost)
   NS_ENSURE_TRUE(uri, NS_ERROR_FAILURE);
 
   rv = uri->GetAsciiHost(aHost);
-  if (NS_FAILED(rv) || aHost.IsEmpty()) {
-    return NS_ERROR_UNEXPECTED;
+  if (NS_SUCCEEDED(rv) && !aHost.IsEmpty()) {
+    return NS_OK;
   }
 
-  return NS_OK;
+  // Some entries like "file://" uses the origin.
+  rv = aPrincipal->GetOrigin(getter_Copies(aHost));
+  if (NS_SUCCEEDED(rv) && !aHost.IsEmpty()) {
+    return NS_OK;
+  }
+
+  return NS_ERROR_UNEXPECTED;
 }
 
 nsCString
@@ -1018,26 +1029,9 @@ nsPermissionManager::CommonTestPermission(nsIPrincipal* aPrincipal,
   // set the default
   *aPermission = nsIPermissionManager::UNKNOWN_ACTION;
 
-  nsCOMPtr<nsIURI> uri;
-  nsresult rv = aPrincipal->GetURI(getter_AddRefs(uri));
-  NS_ENSURE_SUCCESS(rv, rv);
-
   nsAutoCString host;
-  rv = GetHostForPrincipal(aPrincipal, host);
-
-  // No host doesn't mean an error. Just return the default. Unless this is
-  // a file uri. In that case use a magic host.
-  if (NS_FAILED(rv)) {
-    bool isFile;
-    rv = uri->SchemeIs("file", &isFile);
-    NS_ENSURE_SUCCESS(rv, rv);
-    if (isFile) {
-      host.AssignLiteral("<file>");
-    }
-    else {
-      return NS_OK;
-    }
-  }
+  nsresult rv = GetHostForPrincipal(aPrincipal, host);
+  NS_ENSURE_SUCCESS(rv, rv);
 
   int32_t typeIndex = GetTypeIndex(aType, false);
   // If type == -1, the type isn't known,
@@ -1104,14 +1098,32 @@ nsPermissionManager::GetPermissionHashKey(const nsACString& aHost,
     }
   }
 
-  if (!entry && !aExactHostMatch) {
+  if (entry) {
+    return entry;
+  }
+
+  // If we haven't found an entry, depending on the host, we could try a bit
+  // harder.
+  // If this is a file:// URI, we can check for the presence of the magic entry
+  // <file> which gives permission to all file://. This hack might disappear,
+  // see bug 817007. Note that we don't require aExactHostMatch to be true for
+  // that to keep retro-compatibility.
+  // If this is not a file:// URI, and that aExactHostMatch wasn't true, we can
+  // check if the base domain has a permission entry.
+
+  if (StringBeginsWith(aHost, NS_LITERAL_CSTRING("file://"))) {
+    return GetPermissionHashKey(NS_LITERAL_CSTRING("<file>"), aAppId, aIsInBrowserElement, aType, true);
+  }
+
+  if (!aExactHostMatch) {
     nsCString domain = GetNextSubDomainForHost(aHost);
     if (!domain.IsEmpty()) {
       return GetPermissionHashKey(domain, aAppId, aIsInBrowserElement, aType, aExactHostMatch);
     }
   }
 
-  return entry;
+  // No entry, really...
+  return nullptr;
 }
 
 // helper struct for passing arguments into hash enumeration callback.

extensions/cookie/test/unit/test_permmanager_local_files.js

@@ -0,0 +1,48 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that permissions work for file:// URIs (aka local files).
+
+function getPrincipalFromURIString(uriStr)
+{
+  return Services.scriptSecurityManager.getNoAppCodebasePrincipal(NetUtil.newURI(uriStr));
+}
+
+function run_test() {
+  let pm = Services.permissions;
+
+  // If we add a permission to a file:// URI, the test should return true.
+  let principal = getPrincipalFromURIString("file:///foo/bar");
+  pm.addFromPrincipal(principal, "test/local-files", pm.ALLOW_ACTION, 0, 0);
+  do_check_eq(pm.testPermissionFromPrincipal(principal, "test/local-files"), pm.ALLOW_ACTION);
+
+  // Another file:// URI should have the same permission.
+  let witnessPrincipal = getPrincipalFromURIString("file:///bar/foo");
+  do_check_eq(pm.testPermissionFromPrincipal(witnessPrincipal, "test/local-files"), pm.UNKNOWN_ACTION);
+
+  // Giving "file:///" a permission shouldn't give it to all file:// URIs.
+  let rootPrincipal = getPrincipalFromURIString("file:///");
+  pm.addFromPrincipal(rootPrincipal, "test/local-files", pm.ALLOW_ACTION, 0, 0);
+  do_check_eq(pm.testPermissionFromPrincipal(witnessPrincipal, "test/local-files"), pm.UNKNOWN_ACTION);
+
+  // Giving "file://" a permission shouldn't give it to all file:// URIs.
+  let schemeRootPrincipal = getPrincipalFromURIString("file://");
+  pm.addFromPrincipal(schemeRootPrincipal, "test/local-files", pm.ALLOW_ACTION, 0, 0);
+  do_check_eq(pm.testPermissionFromPrincipal(witnessPrincipal, "test/local-files"), pm.UNKNOWN_ACTION);
+
+  // Giving 'node' a permission shouldn't give it to its 'children'.
+  let fileInDirPrincipal = getPrincipalFromURIString("file:///foo/bar/foobar.txt");
+  do_check_eq(pm.testPermissionFromPrincipal(fileInDirPrincipal, "test/local-files"), pm.UNKNOWN_ACTION);
+
+  // Revert "file:///foo/bar" permission and check that it has been correctly taken into account.
+  pm.removeFromPrincipal(principal, "test/local-files");
+  do_check_eq(pm.testPermissionFromPrincipal(principal, "test/local-files"), pm.UNKNOWN_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(witnessPrincipal, "test/local-files"), pm.UNKNOWN_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(fileInDirPrincipal, "test/local-files"), pm.UNKNOWN_ACTION);
+
+  // Add the magic "<file>" permission and make sure all "file://" now have the permission.
+  pm.addFromPrincipal(getPrincipalFromURIString("http://<file>"), "test/local-files", pm.ALLOW_ACTION, 0, 0);
+  do_check_eq(pm.testPermissionFromPrincipal(principal, "test/local-files"), pm.ALLOW_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(witnessPrincipal, "test/local-files"), pm.ALLOW_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(fileInDirPrincipal, "test/local-files"), pm.ALLOW_ACTION);
+}

extensions/cookie/test/unit/xpcshell.ini

@@ -22,6 +22,7 @@ tail =
 skip-if = debug == true
 [test_permmanager_idn.js]
 [test_permmanager_subdomains.js]
+[test_permmanager_local_files.js]
 [test_permmanager_cleardata.js]
 [test_schema_2_migration.js]
 [test_schema_3_migration.js]