Bug 341946: Safe browsing considers all jar: URI's safe

patch: -jar, -javascript, +about
r=mmchew,sr=bryner

browser/components/safebrowsing/content/phishing-warden.js

@@ -454,8 +454,7 @@ PROT_PhishingWarden.prototype.checkUrl_ = function(url, callback) {
     callback();
     return;
   }
-  if (!this.isSpurious_(url))
-    this.isEvilURL_(url, callback);
+  this.isEvilURL_(url, callback);
 }
 
 /**
@@ -492,20 +491,3 @@ PROT_PhishingWarden.prototype.checkRemoteData = function(callback,
     G_Debug(this, "Remote blacklist miss");
   }
 }
-
-/**
- * Helper function to determine whether a given URL is "spurious" for some
- * definition of "spurious".
- *
- * @param url String containing the URL to check
- * 
- * @returns Boolean indicating whether Fritz thinks it's too boring to notice
- */ 
-PROT_PhishingWarden.prototype.isSpurious_ = function(url) {
-  return (url == "about:blank" ||
-          url == "about:config" ||  
-          url.startsWith("chrome://") ||
-          url.startsWith("file://") ||
-          url.startsWith("jar:") ||
-          url.startsWith("javascript:"));
-}

browser/components/safebrowsing/src/nsDocNavStartProgressListener.cpp

@@ -40,6 +40,7 @@
 #include "nsIChannel.h"
 #include "nsIRequest.h"
 #include "nsITimer.h"
+#include "nsIURI.h"
 #include "nsIWebProgress.h"
 #include "nsServiceManagerUtils.h"
 #include "nsString.h"
@@ -100,6 +101,34 @@ nsDocNavStartProgressListener::DetachListeners()
   return webProgressService->RemoveProgressListener(this);
 }
 
+/**
+ * We ignore about:, chrome: and file: URIs.
+ * It's better to err on the side of checking too many URIs.  So if
+ * something fails, we return false.
+ */
+nsresult
+nsDocNavStartProgressListener::IsSpurious(nsIRequest* aReq, PRBool* isSpurious)
+{
+  nsCOMPtr<nsIChannel> channel;
+  nsresult rv;
+  channel = do_QueryInterface(aReq, &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  nsCOMPtr<nsIURI> uri;
+  rv = channel->GetURI(getter_AddRefs(uri));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  nsCAutoString scheme;
+  rv = uri->GetScheme(scheme);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  *isSpurious = scheme.Equals("about") ||
+                scheme.Equals("chrome") ||
+                scheme.Equals("file");
+
+  return NS_OK;
+}
+
 // nsIDocNavStartProgressCallback ***********************************************
 
 // nsDocNavStartProgressListener::GetEnabled
@@ -275,11 +304,17 @@ nsDocNavStartProgressListener::Observe(nsISupports *subject, const char *topic,
     nsIRequest* request = mRequests[0];
 
     if (mCallback) {
-      nsCAutoString name;
-      nsresult rv = request->GetName(name);
+      PRBool isSpurious;
+      nsresult rv = IsSpurious(request, &isSpurious);
       NS_ENSURE_SUCCESS(rv, rv);
 
-      mCallback->OnDocNavStart(request, name);
+      if (!isSpurious) {
+        nsCAutoString name;
+        rv = request->GetName(name);
+        NS_ENSURE_SUCCESS(rv, rv);
+
+        mCallback->OnDocNavStart(request, name);
+      }
     }
 
     mRequests.RemoveObjectAt(0);

browser/components/safebrowsing/src/nsDocNavStartProgressListener.h

@@ -71,4 +71,7 @@ protected:
 
   nsresult AttachListeners();
   nsresult DetachListeners();
+
+  // Return true if we don't want to check the request url.
+  nsresult IsSpurious(nsIRequest* aReq, PRBool* isSpurious);
 };