Bug 454850. Make sure that whenever nsPrincipal::Equals would return true for a pair of principals their nsPrincipal::GetHashValue returns are also equal. r+sr=bzbarsky

caps/include/nsScriptSecurityManager.h

@@ -402,14 +402,18 @@ public:
      * false otherwise.
      */
     static PRBool SecurityCompareURIs(nsIURI* aSourceURI, nsIURI* aTargetURI);
+    static PRUint32 SecurityHashURI(nsIURI* aURI);
 
     static nsresult 
     ReportError(JSContext* cx, const nsAString& messageTag,
                 nsIURI* aSource, nsIURI* aTarget);
+
     static nsresult
     CheckSameOriginPrincipal(nsIPrincipal* aSubject,
                              nsIPrincipal* aObject,
                              PRBool aIsCheckConnect);
+    static PRUint32
+    HashPrincipalByOrigin(nsIPrincipal* aPrincipal);
 
     static PRBool
     GetStrictFileOriginPolicy()

caps/src/nsPrincipal.cpp

@@ -719,9 +719,7 @@ nsPrincipal::GetHashValue(PRUint32* aValue)
     *aValue = nsCRT::HashCode(mCert->fingerprint.get());
   }
   else {
-    nsCAutoString str;
-    mCodebase->GetSpec(str);
-    *aValue = nsCRT::HashCode(str.get());
+    *aValue = nsScriptSecurityManager::HashPrincipalByOrigin(this);
   }
 
   return NS_OK;

caps/src/nsScriptSecurityManager.cpp

@@ -319,6 +319,14 @@ nsScriptSecurityManager::SecurityCompareURIs(nsIURI* aSourceURI,
     return NS_SecurityCompareURIs(aSourceURI, aTargetURI, sStrictFileOriginPolicy);
 }
 
+// SecurityHashURI is consistent with SecurityCompareURIs because NS_SecurityHashURI
+// is consistent with NS_SecurityCompareURIs.  See nsNetUtil.h.
+PRUint32
+nsScriptSecurityManager::SecurityHashURI(nsIURI* aURI)
+{
+    return NS_SecurityHashURI(aURI);
+}
+
 NS_IMETHODIMP
 nsScriptSecurityManager::GetChannelPrincipal(nsIChannel* aChannel,
                                              nsIPrincipal** aPrincipal)
@@ -900,6 +908,27 @@ nsScriptSecurityManager::CheckSameOriginPrincipal(nsIPrincipal* aSubject,
     return NS_ERROR_DOM_PROP_ACCESS_DENIED;
 }
 
+// It's important that
+//
+//   CheckSameOriginPrincipal(A, B, PR_FALSE) == NS_OK
+//
+// imply
+//
+//   HashPrincipalByOrigin(A) == HashPrincipalByOrigin(B)
+//
+// if principals A and B could ever be used as keys in a hashtable.
+// Violation of this invariant leads to spurious failures of hashtable
+// lookups.  See bug 454850.
+
+/*static*/ PRUint32
+nsScriptSecurityManager::HashPrincipalByOrigin(nsIPrincipal* aPrincipal)
+{
+    nsCOMPtr<nsIURI> uri;
+    aPrincipal->GetDomain(getter_AddRefs(uri));
+    if (!uri)
+        aPrincipal->GetURI(getter_AddRefs(uri));
+    return SecurityHashURI(uri);
+}
 
 nsresult
 nsScriptSecurityManager::CheckSameOriginDOMProp(nsIPrincipal* aSubject,

netwerk/base/public/nsNetUtil.h

@@ -48,6 +48,7 @@
 #include "nsCOMPtr.h"
 #include "prio.h" // for read/write flags, permissions, etc.
 
+#include "nsCRT.h"
 #include "nsIURI.h"
 #include "nsIInputStream.h"
 #include "nsIOutputStream.h"
@@ -1466,6 +1467,60 @@ NS_OfflineAppAllowed(nsIURI *aURI, nsIPrefBranch *aPrefBranch = nsnull)
     return allowed;
 }
 
+static inline PRInt32
+GetEffectivePort(nsIURI* aURI)
+{
+    PRInt32 port;
+
+    nsCOMPtr<nsIURI> baseURI = NS_GetInnermostURI(aURI);
+    if (NS_SUCCEEDED(baseURI->GetPort(&port)) && port != -1)
+        return port;
+
+    nsCAutoString scheme;
+    if (NS_FAILED(baseURI->GetScheme(scheme)))
+        return -1;
+
+    return NS_GetDefaultPort(scheme.get());
+}
+
+// NS_SecurityHashURI must return the same hash value for any two URIs that
+// compare equal according to NS_SecurityCompareURIs.  Unfortunately, in the
+// case of files, it's not clear we can do anything better than returning
+// the schemeHash, so hashing files degenerates to storing them in a list.
+inline PRUint32
+NS_SecurityHashURI(nsIURI* aURI)
+{
+    nsCOMPtr<nsIURI> baseURI = NS_GetInnermostURI(aURI);
+
+    nsCAutoString scheme;
+    PRUint32 schemeHash = 0;
+    if (NS_SUCCEEDED(baseURI->GetScheme(scheme)))
+        schemeHash = nsCRT::HashCode(scheme.get());
+
+    // TODO figure out how to hash file:// URIs
+    if (scheme.EqualsLiteral("file"))
+        return schemeHash; // sad face
+
+    if (scheme.EqualsLiteral("imap") ||
+        scheme.EqualsLiteral("mailbox") ||
+        scheme.EqualsLiteral("news"))
+    {
+        nsCAutoString spec;
+        PRUint32 specHash = baseURI->GetSpec(spec);
+        if (NS_SUCCEEDED(specHash))
+            specHash = nsCRT::HashCode(spec.get());
+        return specHash;
+    }
+
+    nsCAutoString host;
+    PRUint32 hostHash = 0;
+    if (NS_SUCCEEDED(baseURI->GetHost(host)))
+        hostHash = nsCRT::HashCode(host.get());
+
+    // XOR to combine hash values
+    return schemeHash ^ hostHash ^ GetEffectivePort(aURI);
+}
+
 inline PRBool
 NS_SecurityCompareURIs(nsIURI* aSourceURI,
                        nsIURI* aTargetURI,
@@ -1563,31 +1618,7 @@ NS_SecurityCompareURIs(nsIURI* aSourceURI,
         return PR_FALSE;
     }
 
-    // Compare ports
-    PRInt32 targetPort;
-    nsresult rv = targetBaseURI->GetPort(&targetPort);
-    PRInt32 sourcePort;
-    if (NS_SUCCEEDED(rv))
-        rv = sourceBaseURI->GetPort(&sourcePort);
-    PRBool result = NS_SUCCEEDED(rv) && targetPort == sourcePort;
-    // If the port comparison failed, see if either URL has a
-    // port of -1. If so, replace -1 with the default port
-    // for that scheme.
-    if (NS_SUCCEEDED(rv) && !result &&
-        (sourcePort == -1 || targetPort == -1))
-    {
-        PRInt32 defaultPort = NS_GetDefaultPort(targetScheme.get());
-        if (defaultPort == -1)
-            return PR_FALSE; // No default port for this scheme
-
-        if (sourcePort == -1)
-            sourcePort = defaultPort;
-        else if (targetPort == -1)
-            targetPort = defaultPort;
-        result = targetPort == sourcePort;
-    }
-
-    return result;
+    return GetEffectivePort(targetBaseURI) == GetEffectivePort(sourceBaseURI);
 }
 
 #endif // !nsNetUtil_h__