Bug 904607: Add protocol parser for -digest256 lists (r=gcp).
authorMonica Chew <mmc@mozilla.com>
Fri, 06 Sep 2013 17:12:33 -0700
changeset 146041 2be3551a5d8040c76b92302eb14cdb0852da393d
parent 146040 5e861b43ebf1a6d63b5621c41a6047ef3bec5120
child 146042 356866ae2f6885933451a92aae838af00d659679
push idunknown
push userunknown
push dateunknown
reviewersgcp
bugs904607
milestone26.0a1
Bug 904607: Add protocol parser for -digest256 lists (r=gcp).
netwerk/base/public/nsIURIClassifier.idl
toolkit/components/url-classifier/ChunkSet.h
toolkit/components/url-classifier/Classifier.cpp
toolkit/components/url-classifier/Entries.h
toolkit/components/url-classifier/HashStore.cpp
toolkit/components/url-classifier/HashStore.h
toolkit/components/url-classifier/LookupCache.cpp
toolkit/components/url-classifier/ProtocolParser.cpp
toolkit/components/url-classifier/ProtocolParser.h
toolkit/components/url-classifier/content/listmanager.js
toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
toolkit/components/url-classifier/nsIUrlClassifierHashCompleter.idl
toolkit/components/url-classifier/nsIUrlClassifierStreamUpdater.idl
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
toolkit/components/url-classifier/nsUrlClassifierDBService.h
toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.h
toolkit/components/url-classifier/tests/unit/data/digest1.chunk
toolkit/components/url-classifier/tests/unit/data/digest2.chunk
toolkit/components/url-classifier/tests/unit/test_digest256.js
toolkit/components/url-classifier/tests/unit/xpcshell.ini
--- a/netwerk/base/public/nsIURIClassifier.idl
+++ b/netwerk/base/public/nsIURIClassifier.idl
@@ -29,17 +29,17 @@ interface nsIURIClassifierCallback : nsI
 /**
  * The URI classifier service checks a URI against lists of phishing
  * and malware sites.
  */
 [scriptable, uuid(617f1002-ec55-42c4-a7b0-ebb221ba9fa2)]
 interface nsIURIClassifier : nsISupports
 {
   /**
-   * Classify a Principal using it's URI, appId and InBrowserElement state.
+   * Classify a Principal using its URI.
    *
    * @param aPrincipal
    *        The principal that should be checked by the URI classifier.
    * @param aCallback
    *        The URI classifier will call this callback when the URI has been
    *        classified.
    *
    * @return <code>false</code> if classification is not necessary.  The
--- a/toolkit/components/url-classifier/ChunkSet.h
+++ b/toolkit/components/url-classifier/ChunkSet.h
@@ -10,19 +10,20 @@
 #include "Entries.h"
 #include "nsString.h"
 #include "nsTArray.h"
 
 namespace mozilla {
 namespace safebrowsing {
 
 /**
- * Store the chunks as an array of uint32_t.
- * XXX: We should optimize this further to compress the
- * many consecutive numbers.
+ * Store the chunk numbers as an array of uint32_t. We need chunk numbers in
+ * order to ask for incremental updates from the server.
+ * XXX: We should optimize this further to compress the many consecutive
+ * numbers.
  */
 class ChunkSet {
 public:
   ChunkSet() {}
   ~ChunkSet() {}
 
   nsresult Serialize(nsACString& aStr);
   nsresult Set(uint32_t aChunk);
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -194,17 +194,19 @@ Classifier::TableRequest(nsACString& aRe
   }
 }
 
 nsresult
 Classifier::Check(const nsACString& aSpec, LookupResultArray& aResults)
 {
   Telemetry::AutoTimer<Telemetry::URLCLASSIFIER_CL_CHECK_TIME> timer;
 
-  // Get the set of fragments to look up.
+  // Get the set of fragments based on the url. This is necessary because we
+  // only look up at most 5 URLs per aSpec, even if aSpec has more than 5
+  // components.
   nsTArray<nsCString> fragments;
   nsresult rv = LookupCache::GetLookupFragments(aSpec, &fragments);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsTArray<nsCString> activeTables;
   ActiveTables(activeTables);
 
   nsTArray<LookupCache*> cacheArray;
@@ -221,25 +223,26 @@ Classifier::Check(const nsACString& aSpe
   for (uint32_t i = 0; i < fragments.Length(); i++) {
     Completion lookupHash;
     lookupHash.FromPlaintext(fragments[i], mCryptoHash);
 
     // Get list of host keys to look up
     Completion hostKey;
     rv = LookupCache::GetKey(fragments[i], &hostKey, mCryptoHash);
     if (NS_FAILED(rv)) {
-      // Local host on the network
+      // Local host on the network.
       continue;
     }
 
 #if DEBUG && defined(PR_LOGGING)
     if (LOG_ENABLED()) {
       nsAutoCString checking;
-      lookupHash.ToString(checking);
-      LOG(("Checking %s (%X)", checking.get(), lookupHash.ToUint32()));
+      lookupHash.ToHexString(checking);
+      LOG(("Checking fragment %s, hash %s (%X)", fragments[i].get(),
+           checking.get(), lookupHash.ToUint32()));
     }
 #endif
     for (uint32_t i = 0; i < cacheArray.Length(); i++) {
       LookupCache *cache = cacheArray[i];
       bool has, complete;
       rv = cache->Has(lookupHash, &has, &complete);
       NS_ENSURE_SUCCESS(rv, rv);
       if (has) {
@@ -537,18 +540,17 @@ Classifier::RecoverBackups()
 
 /*
  * This will consume+delete updates from the passed nsTArray.
 */
 nsresult
 Classifier::ApplyTableUpdates(nsTArray<TableUpdate*>* aUpdates,
                               const nsACString& aTable)
 {
-  LOG(("Classifier::ApplyTableUpdates(%s)",
-       PromiseFlatCString(aTable).get()));
+  LOG(("Classifier::ApplyTableUpdates(%s)", PromiseFlatCString(aTable).get()));
 
   nsAutoPtr<HashStore> store(new HashStore(aTable, mStoreDirectory));
 
   if (!store)
     return NS_ERROR_FAILURE;
 
   // take the quick exit if there is no valid update for us
   // (common case)
@@ -562,16 +564,17 @@ Classifier::ApplyTableUpdates(nsTArray<T
       aUpdates->ElementAt(i) = nullptr;
       delete update;
       continue;
     }
     validupdates++;
   }
 
   if (!validupdates) {
+    // This can happen if the update was only valid for one table.
     return NS_OK;
   }
 
   nsresult rv = store->Open();
   NS_ENSURE_SUCCESS(rv, rv);
   rv = store->BeginUpdate();
   NS_ENSURE_SUCCESS(rv, rv);
 
--- a/toolkit/components/url-classifier/Entries.h
+++ b/toolkit/components/url-classifier/Entries.h
@@ -1,13 +1,17 @@
 //* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+// This header file defines the storage types of the actual safebrowsing
+// chunk data, which may be either 32-bit hashes or complete 256-bit hashes.
+// Chunk numbers are represented in ChunkSet.h.
+
 #ifndef SBEntries_h__
 #define SBEntries_h__
 
 #include "nsTArray.h"
 #include "nsString.h"
 #include "nsICryptoHash.h"
 #include "nsNetUtil.h"
 
@@ -16,16 +20,17 @@
 #endif
 
 namespace mozilla {
 namespace safebrowsing {
 
 #define PREFIX_SIZE   4
 #define COMPLETE_SIZE 32
 
+// This is the struct that contains 4-byte hash prefixes.
 template <uint32_t S, class Comparator>
 struct SafebrowsingHash
 {
   static const uint32_t sHashSize = S;
   typedef SafebrowsingHash<S, Comparator> self_type;
   uint8_t buf[S];
 
   nsresult FromPlaintext(const nsACString& aPlainText, nsICryptoHash* aHash) {
@@ -77,16 +82,29 @@ struct SafebrowsingHash
 
 #ifdef DEBUG
   void ToString(nsACString& aStr) const {
     uint32_t len = ((sHashSize + 2) / 3) * 4;
     aStr.SetCapacity(len + 1);
     PL_Base64Encode((char*)buf, sHashSize, aStr.BeginWriting());
     aStr.BeginWriting()[len] = '\0';
   }
+
+  void ToHexString(nsACString& aStr) const {
+    static const char* const lut = "0123456789ABCDEF";
+    // 32 bytes is the longest hash
+    size_t len = 32;
+
+    aStr.SetCapacity(2 * len);
+    for (size_t i = 0; i < len; ++i) {
+      const char c = static_cast<const char>(buf[i]);
+      aStr.Append(lut[(c >> 4) & 0x0F]);
+      aStr.Append(lut[c & 15]);
+    }
+  }
 #endif
   uint32_t ToUint32() const {
       return *((uint32_t*)buf);
   }
   void FromUint32(uint32_t aHash) {
       *((uint32_t*)buf) = aHash;
   }
 };
@@ -100,117 +118,125 @@ public:
           return 1;
       } else if (first == second) {
           return 0;
       } else {
           return -1;
       }
   }
 };
+// Use this for 4-byte hashes
 typedef SafebrowsingHash<PREFIX_SIZE, PrefixComparator> Prefix;
 typedef nsTArray<Prefix> PrefixArray;
 
 class CompletionComparator {
 public:
   static int Compare(const uint8_t* a, const uint8_t* b) {
     return memcmp(a, b, COMPLETE_SIZE);
   }
 };
+// Use this for 32-byte hashes
 typedef SafebrowsingHash<COMPLETE_SIZE, CompletionComparator> Completion;
 typedef nsTArray<Completion> CompletionArray;
 
 struct AddPrefix {
+  // The truncated hash.
   Prefix prefix;
+  // The chunk number to which it belongs.
   uint32_t addChunk;
 
   AddPrefix() : addChunk(0) {}
 
+  // Returns the chunk number.
   uint32_t Chunk() const { return addChunk; }
   const Prefix &PrefixHash() const { return prefix; }
 
   template<class T>
   int Compare(const T& other) const {
     int cmp = prefix.Compare(other.PrefixHash());
     if (cmp != 0) {
       return cmp;
     }
     return addChunk - other.addChunk;
   }
 };
 
 struct AddComplete {
-  union {
-    Prefix prefix;
-    Completion complete;
-  } hash;
+  Completion complete;
   uint32_t addChunk;
 
   AddComplete() : addChunk(0) {}
 
   uint32_t Chunk() const { return addChunk; }
-  const Prefix &PrefixHash() const { return hash.prefix; }
-  const Completion &CompleteHash() const { return hash.complete; }
+  // The 4-byte prefix of the sha256 hash.
+  uint32_t ToUint32() const { return complete.ToUint32(); }
+  // The 32-byte sha256 hash.
+  const Completion &CompleteHash() const { return complete; }
 
   template<class T>
   int Compare(const T& other) const {
-    int cmp = hash.complete.Compare(other.CompleteHash());
+    int cmp = complete.Compare(other.CompleteHash());
     if (cmp != 0) {
       return cmp;
     }
     return addChunk - other.addChunk;
   }
 };
 
 struct SubPrefix {
+  // The hash to subtract.
   Prefix prefix;
+  // The chunk number of the add chunk to which the hash belonged.
   uint32_t addChunk;
+  // The chunk number of this sub chunk.
   uint32_t subChunk;
 
   SubPrefix(): addChunk(0), subChunk(0) {}
 
   uint32_t Chunk() const { return subChunk; }
   uint32_t AddChunk() const { return addChunk; }
   const Prefix &PrefixHash() const { return prefix; }
 
   template<class T>
+  // Returns 0 if and only if the chunks are the same in every way.
   int Compare(const T& aOther) const {
     int cmp = prefix.Compare(aOther.PrefixHash());
     if (cmp != 0)
       return cmp;
     if (addChunk != aOther.addChunk)
       return addChunk - aOther.addChunk;
     return subChunk - aOther.subChunk;
   }
 
   template<class T>
   int CompareAlt(const T& aOther) const {
-    int cmp = prefix.Compare(aOther.PrefixHash());
+    Prefix other;
+    other.FromUint32(aOther.ToUint32());
+    int cmp = prefix.Compare(other);
     if (cmp != 0)
       return cmp;
     return addChunk - aOther.addChunk;
   }
 };
 
 struct SubComplete {
-  union {
-    Prefix prefix;
-    Completion complete;
-  } hash;
+  Completion complete;
   uint32_t addChunk;
   uint32_t subChunk;
 
   SubComplete() : addChunk(0), subChunk(0) {}
 
   uint32_t Chunk() const { return subChunk; }
   uint32_t AddChunk() const { return addChunk; }
-  const Prefix &PrefixHash() const { return hash.prefix; }
-  const Completion &CompleteHash() const { return hash.complete; }
+  const Completion &CompleteHash() const { return complete; }
+  // The 4-byte prefix of the sha256 hash.
+  uint32_t ToUint32() const { return complete.ToUint32(); }
 
   int Compare(const SubComplete& aOther) const {
-    int cmp = hash.complete.Compare(aOther.hash.complete);
+    int cmp = complete.Compare(aOther.complete);
     if (cmp != 0)
       return cmp;
     if (addChunk != aOther.addChunk)
       return addChunk - aOther.addChunk;
     return subChunk - aOther.subChunk;
   }
 };
 
--- a/toolkit/components/url-classifier/HashStore.cpp
+++ b/toolkit/components/url-classifier/HashStore.cpp
@@ -141,25 +141,25 @@ TableUpdate::NewSubPrefix(uint32_t aAddC
   sub->subChunk = aSubChunk;
 }
 
 void
 TableUpdate::NewAddComplete(uint32_t aAddChunk, const Completion& aHash)
 {
   AddComplete *add = mAddCompletes.AppendElement();
   add->addChunk = aAddChunk;
-  add->hash.complete = aHash;
+  add->complete = aHash;
 }
 
 void
 TableUpdate::NewSubComplete(uint32_t aAddChunk, const Completion& aHash, uint32_t aSubChunk)
 {
   SubComplete *sub = mSubCompletes.AppendElement();
   sub->addChunk = aAddChunk;
-  sub->hash.complete = aHash;
+  sub->complete = aHash;
   sub->subChunk = aSubChunk;
 }
 
 
 HashStore::HashStore(const nsACString& aTableName, nsIFile* aStoreDir)
   : mTableName(aTableName)
   , mStoreDirectory(aStoreDir)
   , mInUpdate(false)
@@ -318,16 +318,18 @@ HashStore::CalculateChecksum(nsAutoCStri
   nsresult rv = seekable->Seek(nsISeekableStream::NS_SEEK_SET, 0);
 
   nsCOMPtr<nsICryptoHash> hash = do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Size of MD5 hash in bytes
   const uint32_t CHECKSUM_SIZE = 16;
 
+  // MD5 is not a secure hash function, but since this is a filesystem integrity
+  // check, this usage is ok.
   rv = hash->Init(nsICryptoHash::MD5);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (!aChecksumPresent) {
     // Hash entire file
     rv = hash->UpdateFromStream(mInputStream, UINT32_MAX);
   } else {
     // Hash everything but last checksum bytes
@@ -357,19 +359,17 @@ HashStore::UpdateHeader()
   mHeader.numSubPrefixes = mSubPrefixes.Length();
   mHeader.numAddCompletes = mAddCompletes.Length();
   mHeader.numSubCompletes = mSubCompletes.Length();
 }
 
 nsresult
 HashStore::ReadChunkNumbers()
 {
-  if (!mInputStream) {
-    return NS_OK;
-  }
+  NS_ENSURE_STATE(mInputStream);
 
   nsCOMPtr<nsISeekableStream> seekable = do_QueryInterface(mInputStream);
   nsresult rv = seekable->Seek(nsISeekableStream::NS_SEEK_SET,
                                sizeof(Header));
 
   rv = mAddChunks.Read(mInputStream, mHeader.numAddChunks);
   NS_ENSURE_SUCCESS(rv, rv);
   NS_ASSERTION(mAddChunks.Length() == mHeader.numAddChunks, "Read the right amount of add chunks.");
@@ -380,16 +380,18 @@ HashStore::ReadChunkNumbers()
 
   return NS_OK;
 }
 
 nsresult
 HashStore::ReadHashes()
 {
   if (!mInputStream) {
+    // BeginUpdate has been called but Open hasn't initialized mInputStream,
+    // because the existing HashStore is empty.
     return NS_OK;
   }
 
   nsCOMPtr<nsISeekableStream> seekable = do_QueryInterface(mInputStream);
 
   uint32_t offset = sizeof(Header);
   offset += (mHeader.numAddChunks + mHeader.numSubChunks) * sizeof(uint32_t);
   nsresult rv = seekable->Seek(nsISeekableStream::NS_SEEK_SET, offset);
@@ -814,24 +816,24 @@ HashStore::WriteFile()
   rv = NS_NewCheckSummedOutputStream(getter_AddRefs(out), storeFile,
                                      PR_WRONLY | PR_TRUNCATE | PR_CREATE_FILE);
   NS_ENSURE_SUCCESS(rv, rv);
 
   uint32_t written;
   rv = out->Write(reinterpret_cast<char*>(&mHeader), sizeof(mHeader), &written);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // Write chunk numbers...
+  // Write chunk numbers.
   rv = mAddChunks.Write(out);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = mSubChunks.Write(out);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // Write hashes..
+  // Write hashes.
   rv = WriteAddPrefixes(out);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = WriteSubPrefixes(out);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = WriteTArray(out, mAddCompletes);
   NS_ENSURE_SUCCESS(rv, rv);
@@ -997,17 +999,17 @@ HashStore::ProcessSubs()
   LOG(("All databases seem to have a consistent sort order."));
 #endif
 
   RemoveMatchingPrefixes(mSubPrefixes, &mAddCompletes);
   RemoveMatchingPrefixes(mSubPrefixes, &mSubCompletes);
 
   // Remove any remaining subbed prefixes from both addprefixes
   // and addcompletes.
-  KnockoutSubs(&mSubPrefixes,  &mAddPrefixes);
+  KnockoutSubs(&mSubPrefixes, &mAddPrefixes);
   KnockoutSubs(&mSubCompletes, &mAddCompletes);
 
   // Remove any remaining subprefixes referring to addchunks that
   // we have (and hence have been processed above).
   RemoveDeadSubPrefixes(mSubPrefixes, mAddChunks);
 
 #ifdef DEBUG
   EnsureSorted(&mAddPrefixes);
--- a/toolkit/components/url-classifier/HashStore.h
+++ b/toolkit/components/url-classifier/HashStore.h
@@ -12,16 +12,19 @@
 #include "nsTArray.h"
 #include "nsIFile.h"
 #include "nsIFileStreams.h"
 #include "nsCOMPtr.h"
 
 namespace mozilla {
 namespace safebrowsing {
 
+// A table update is built from a single update chunk from the server. As the
+// protocol parser processes each chunk, it constructs a table update with the
+// new hashes.
 class TableUpdate {
 public:
   TableUpdate(const nsACString& aTable)
       : mTable(aTable), mLocalUpdate(false) {}
   const nsCString& TableName() const { return mTable; }
 
   bool Empty() const {
     return mAddChunks.Length() == 0 &&
@@ -29,64 +32,80 @@ public:
       mAddExpirations.Length() == 0 &&
       mSubExpirations.Length() == 0 &&
       mAddPrefixes.Length() == 0 &&
       mSubPrefixes.Length() == 0 &&
       mAddCompletes.Length() == 0 &&
       mSubCompletes.Length() == 0;
   }
 
+  // Throughout, uint32_t aChunk refers only to the chunk number. Chunk data is
+  // stored in the Prefix structures.
   void NewAddChunk(uint32_t aChunk) { mAddChunks.Set(aChunk); }
   void NewSubChunk(uint32_t aChunk) { mSubChunks.Set(aChunk); }
 
   void NewAddExpiration(uint32_t aChunk) { mAddExpirations.Set(aChunk); }
   void NewSubExpiration(uint32_t aChunk) { mSubExpirations.Set(aChunk); }
 
   void NewAddPrefix(uint32_t aAddChunk, const Prefix& aPrefix);
   void NewSubPrefix(uint32_t aAddChunk, const Prefix& aPrefix, uint32_t aSubChunk);
+
   void NewAddComplete(uint32_t aChunk, const Completion& aCompletion);
   void NewSubComplete(uint32_t aAddChunk, const Completion& aCompletion,
                       uint32_t aSubChunk);
   void SetLocalUpdate(void) { mLocalUpdate = true; }
   bool IsLocalUpdate(void) { return mLocalUpdate; }
 
   ChunkSet& AddChunks() { return mAddChunks; }
   ChunkSet& SubChunks() { return mSubChunks; }
 
+  // Expirations for chunks.
   ChunkSet& AddExpirations() { return mAddExpirations; }
   ChunkSet& SubExpirations() { return mSubExpirations; }
 
+  // Hashes associated with this chunk.
   AddPrefixArray& AddPrefixes() { return mAddPrefixes; }
   SubPrefixArray& SubPrefixes() { return mSubPrefixes; }
   AddCompleteArray& AddCompletes() { return mAddCompletes; }
   SubCompleteArray& SubCompletes() { return mSubCompletes; }
 
 private:
   nsCString mTable;
   // Update not from the remote server (no freshness)
   bool mLocalUpdate;
 
+  // The list of chunk numbers that we have for each of the type of chunks.
   ChunkSet mAddChunks;
   ChunkSet mSubChunks;
   ChunkSet mAddExpirations;
   ChunkSet mSubExpirations;
+
+  // 4-byte sha256 prefixes.
   AddPrefixArray mAddPrefixes;
   SubPrefixArray mSubPrefixes;
+
+  // 32-byte hashes.
   AddCompleteArray mAddCompletes;
   SubCompleteArray mSubCompletes;
 };
 
+// There is one hash store per table.
 class HashStore {
 public:
   HashStore(const nsACString& aTableName, nsIFile* aStoreFile);
   ~HashStore();
 
   const nsCString& TableName() const { return mTableName; }
 
   nsresult Open();
+  // Add Prefixes are stored partly in the PrefixSet (contains the
+  // Prefix data organized for fast lookup/low RAM usage) and partly in the
+  // HashStore (Add Chunk numbers - only used for updates, slow retrieval).
+  // AugmentAdds function joins the separate datasets into one complete
+  // prefixes+chunknumbers dataset.
   nsresult AugmentAdds(const nsTArray<uint32_t>& aPrefixes);
 
   ChunkSet& AddChunks() { return mAddChunks; }
   ChunkSet& SubChunks() { return mSubChunks; }
   AddPrefixArray& AddPrefixes() { return mAddPrefixes; }
   AddCompleteArray& AddCompletes() { return mAddCompletes; }
   SubPrefixArray& SubPrefixes() { return mSubPrefixes; }
   SubCompleteArray& SubCompletes() { return mSubCompletes; }
@@ -121,52 +140,61 @@ private:
   nsresult SanityCheck();
   nsresult CalculateChecksum(nsAutoCString& aChecksum, uint32_t aFileSize,
                              bool aChecksumPresent);
   nsresult CheckChecksum(nsIFile* aStoreFile, uint32_t aFileSize);
   void UpdateHeader();
 
   nsresult ReadChunkNumbers();
   nsresult ReadHashes();
+
   nsresult ReadAddPrefixes();
   nsresult ReadSubPrefixes();
 
   nsresult WriteAddPrefixes(nsIOutputStream* aOut);
   nsresult WriteSubPrefixes(nsIOutputStream* aOut);
 
   nsresult ProcessSubs();
 
+ // This is used for checking that the database is correct and for figuring out
+ // the number of chunks, etc. to read from disk on restart.
   struct Header {
     uint32_t magic;
     uint32_t version;
     uint32_t numAddChunks;
     uint32_t numSubChunks;
     uint32_t numAddPrefixes;
     uint32_t numSubPrefixes;
     uint32_t numAddCompletes;
     uint32_t numSubCompletes;
   };
 
   Header mHeader;
 
+  // The name of the table (must end in -shavar or -digest256, or evidently
+  // -simple for unittesting.
   nsCString mTableName;
   nsCOMPtr<nsIFile> mStoreDirectory;
 
   bool mInUpdate;
 
   nsCOMPtr<nsIInputStream> mInputStream;
 
+  // Chunk numbers, stored as uint32_t arrays.
   ChunkSet mAddChunks;
   ChunkSet mSubChunks;
 
   ChunkSet mAddExpirations;
   ChunkSet mSubExpirations;
 
+  // Chunk data for shavar tables. See Entries.h for format.
   AddPrefixArray mAddPrefixes;
+  SubPrefixArray mSubPrefixes;
+
+  // See bug 806422 for background. We must be able to distinguish between
+  // updates from the completion server and updates from the regular server.
   AddCompleteArray mAddCompletes;
-  SubPrefixArray mSubPrefixes;
   SubCompleteArray mSubCompletes;
 };
 
 }
 }
-
 #endif
--- a/toolkit/components/url-classifier/LookupCache.cpp
+++ b/toolkit/components/url-classifier/LookupCache.cpp
@@ -510,17 +510,17 @@ LookupCache::GetLookupFragments(const ns
   paths.AppendElement(EmptyCString());
 
   for (uint32_t hostIndex = 0; hostIndex < hosts.Length(); hostIndex++) {
     for (uint32_t pathIndex = 0; pathIndex < paths.Length(); pathIndex++) {
       nsCString key;
       key.Assign(hosts[hostIndex]);
       key.Append('/');
       key.Append(paths[pathIndex]);
-      LOG(("Chking %s", key.get()));
+      LOG(("Checking fragment %s", key.get()));
 
       aFragments->AppendElement(key);
     }
   }
 
   return NS_OK;
 }
 
--- a/toolkit/components/url-classifier/ProtocolParser.cpp
+++ b/toolkit/components/url-classifier/ProtocolParser.cpp
@@ -217,16 +217,17 @@ ProtocolParser::ProcessControl(bool* aDo
 
     if (line.EqualsLiteral("e:pleaserekey")) {
       mRekeyRequested = true;
       return NS_OK;
     } else if (mHMAC && mServerMAC.IsEmpty()) {
       rv = ProcessMAC(line);
       NS_ENSURE_SUCCESS(rv, rv);
     } else if (StringBeginsWith(line, NS_LITERAL_CSTRING("i:"))) {
+      // Set the table name from the table header line.
       SetCurrentTable(Substring(line, 2));
     } else if (StringBeginsWith(line, NS_LITERAL_CSTRING("n:"))) {
       if (PR_sscanf(line.get(), "n:%d", &mUpdateWait) != 1) {
         LOG(("Error parsing n: '%s' (%d)", line.get(), mUpdateWait));
         mUpdateWait = 0;
       }
     } else if (line.EqualsLiteral("r:pleasereset")) {
       mResetRequested = true;
@@ -325,22 +326,40 @@ ProtocolParser::ProcessChunkControl(cons
     return NS_ERROR_FAILURE;
   }
 
   if (!(mChunkState.hashSize == PREFIX_SIZE || mChunkState.hashSize == COMPLETE_SIZE)) {
     NS_WARNING("Invalid hash size specified in update.");
     return NS_ERROR_FAILURE;
   }
 
-  mChunkState.type = (command == 'a') ? CHUNK_ADD : CHUNK_SUB;
-
-  if (mChunkState.type == CHUNK_ADD) {
-    mTableUpdate->NewAddChunk(mChunkState.num);
-  } else {
-    mTableUpdate->NewSubChunk(mChunkState.num);
+  if (StringEndsWith(mTableUpdate->TableName(),
+                     NS_LITERAL_CSTRING("-shavar")) ||
+      StringEndsWith(mTableUpdate->TableName(),
+                     NS_LITERAL_CSTRING("-simple"))) {
+    // Accommodate test tables ending in -simple for now.
+    mChunkState.type = (command == 'a') ? CHUNK_ADD : CHUNK_SUB;
+  } else if (StringEndsWith(mTableUpdate->TableName(),
+    NS_LITERAL_CSTRING("-digest256"))) {
+    LOG(("Processing digest256 data"));
+    mChunkState.type = (command == 'a') ? CHUNK_ADD_DIGEST : CHUNK_SUB_DIGEST;
+  }
+  switch (mChunkState.type) {
+    case CHUNK_ADD:
+      mTableUpdate->NewAddChunk(mChunkState.num);
+      break;
+    case CHUNK_SUB:
+      mTableUpdate->NewSubChunk(mChunkState.num);
+      break;
+    case CHUNK_ADD_DIGEST:
+      mTableUpdate->NewAddChunk(mChunkState.num);
+      break;
+    case CHUNK_SUB_DIGEST:
+      mTableUpdate->NewSubChunk(mChunkState.num);
+      break;
   }
 
   return NS_OK;
 }
 
 nsresult
 ProtocolParser::ProcessForward(const nsCString& aLine)
 {
@@ -401,21 +420,25 @@ ProtocolParser::ProcessChunk(bool* aDone
   nsAutoCString chunk;
   chunk.Assign(Substring(mPending, 0, mChunkState.length));
   mPending = Substring(mPending, mChunkState.length);
 
   *aDone = false;
   mState = PROTOCOL_STATE_CONTROL;
 
   //LOG(("Handling a %d-byte chunk", chunk.Length()));
-  if (StringEndsWith(mTableUpdate->TableName(), NS_LITERAL_CSTRING("-shavar"))) {
+  if (StringEndsWith(mTableUpdate->TableName(),
+                     NS_LITERAL_CSTRING("-shavar"))) {
     return ProcessShaChunk(chunk);
-  } else {
-    return ProcessPlaintextChunk(chunk);
   }
+  if (StringEndsWith(mTableUpdate->TableName(),
+             NS_LITERAL_CSTRING("-digest256"))) {
+    return ProcessDigestChunk(chunk);
+  }
+  return ProcessPlaintextChunk(chunk);
 }
 
 /**
  * Process a plaintext chunk (currently only used in unit tests).
  */
 nsresult
 ProtocolParser::ProcessPlaintextChunk(const nsACString& aChunk)
 {
@@ -503,16 +526,71 @@ ProtocolParser::ProcessShaChunk(const ns
     }
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   return NS_OK;
 }
 
 nsresult
+ProtocolParser::ProcessDigestChunk(const nsACString& aChunk)
+{
+  if (mChunkState.type == CHUNK_ADD_DIGEST) {
+    return ProcessDigestAdd(aChunk);
+  }
+  if (mChunkState.type == CHUNK_SUB_DIGEST) {
+    return ProcessDigestSub(aChunk);
+  }
+  return NS_ERROR_UNEXPECTED;
+}
+
+nsresult
+ProtocolParser::ProcessDigestAdd(const nsACString& aChunk)
+{
+  // The ABNF format for add chunks is (HASH)+, where HASH is 32 bytes.
+  MOZ_ASSERT(aChunk.Length() % 32 == 0,
+             "Chunk length in bytes must be divisible by 4");
+  uint32_t start = 0;
+  while (start < aChunk.Length()) {
+    Completion hash;
+    hash.Assign(Substring(aChunk, start, COMPLETE_SIZE));
+    start += COMPLETE_SIZE;
+    mTableUpdate->NewAddComplete(mChunkState.num, hash);
+  }
+  return NS_OK;
+}
+
+nsresult
+ProtocolParser::ProcessDigestSub(const nsACString& aChunk)
+{
+  // The ABNF format for sub chunks is (ADDCHUNKNUM HASH)+, where ADDCHUNKNUM
+  // is a 4 byte chunk number, and HASH is 32 bytes.
+  MOZ_ASSERT(aChunk.Length() % 36 == 0,
+             "Chunk length in bytes must be divisible by 36");
+  uint32_t start = 0;
+  while (start < aChunk.Length()) {
+    // Read ADDCHUNKNUM
+    const nsCSubstring& addChunkStr = Substring(aChunk, start, 4);
+    start += 4;
+
+    uint32_t addChunk;
+    memcpy(&addChunk, addChunkStr.BeginReading(), 4);
+    addChunk = PR_ntohl(addChunk);
+
+    // Read the hash
+    Completion hash;
+    hash.Assign(Substring(aChunk, start, COMPLETE_SIZE));
+    start += COMPLETE_SIZE;
+
+    mTableUpdate->NewSubComplete(addChunk, hash, mChunkState.num);
+  }
+  return NS_OK;
+}
+
+nsresult
 ProtocolParser::ProcessHostAdd(const Prefix& aDomain, uint8_t aNumEntries,
                                const nsACString& aChunk, uint32_t* aStart)
 {
   NS_ASSERTION(mChunkState.hashSize == PREFIX_SIZE,
                "ProcessHostAdd should only be called for prefix hashes.");
 
   if (aNumEntries == 0) {
     mTableUpdate->NewAddPrefix(mChunkState.num, aDomain);
@@ -585,16 +663,17 @@ nsresult
 ProtocolParser::ProcessHostAddComplete(uint8_t aNumEntries,
                                        const nsACString& aChunk, uint32_t* aStart)
 {
   NS_ASSERTION(mChunkState.hashSize == COMPLETE_SIZE,
                "ProcessHostAddComplete should only be called for complete hashes.");
 
   if (aNumEntries == 0) {
     // this is totally comprehensible.
+    // My sarcasm detector is going off!
     NS_WARNING("Expected > 0 entries for a 32-byte hash add.");
     return NS_OK;
   }
 
   if (*aStart + (COMPLETE_SIZE * aNumEntries) > aChunk.Length()) {
     NS_WARNING("Chunk is not long enough to contain the expected entries.");
     return NS_ERROR_FAILURE;
   }
@@ -679,10 +758,10 @@ ProtocolParser::GetTableUpdate(const nsA
   // updates can be transferred to DBServiceWorker, which passes
   // them back to Classifier when doing the updates, and that
   // will free them.
   TableUpdate *update = new TableUpdate(aTable);
   mTableUpdates.AppendElement(update);
   return update;
 }
 
-}
-}
+} // namespace safebrowsing
+} // namespace mozilla
--- a/toolkit/components/url-classifier/ProtocolParser.h
+++ b/toolkit/components/url-classifier/ProtocolParser.h
@@ -54,39 +54,51 @@ public:
 private:
   nsresult ProcessControl(bool* aDone);
   nsresult ProcessMAC(const nsCString& aLine);
   nsresult ProcessExpirations(const nsCString& aLine);
   nsresult ProcessChunkControl(const nsCString& aLine);
   nsresult ProcessForward(const nsCString& aLine);
   nsresult AddForward(const nsACString& aUrl, const nsACString& aMac);
   nsresult ProcessChunk(bool* done);
+  // Remove this, it's only used for testing
   nsresult ProcessPlaintextChunk(const nsACString& aChunk);
   nsresult ProcessShaChunk(const nsACString& aChunk);
   nsresult ProcessHostAdd(const Prefix& aDomain, uint8_t aNumEntries,
                           const nsACString& aChunk, uint32_t* aStart);
   nsresult ProcessHostSub(const Prefix& aDomain, uint8_t aNumEntries,
                           const nsACString& aChunk, uint32_t* aStart);
   nsresult ProcessHostAddComplete(uint8_t aNumEntries, const nsACString& aChunk,
                                   uint32_t *aStart);
   nsresult ProcessHostSubComplete(uint8_t numEntries, const nsACString& aChunk,
                                   uint32_t* start);
+  // Digest chunks are very similar to shavar chunks, except digest chunks
+  // always contain the full hash, so there is no need for chunk data to
+  // contain prefix sizes.
+  nsresult ProcessDigestChunk(const nsACString& aChunk);
+  nsresult ProcessDigestAdd(const nsACString& aChunk);
+  nsresult ProcessDigestSub(const nsACString& aChunk);
   bool NextLine(nsACString& aLine);
 
   void CleanupUpdates();
 
   enum ParserState {
     PROTOCOL_STATE_CONTROL,
     PROTOCOL_STATE_CHUNK
   };
   ParserState mState;
 
   enum ChunkType {
+    // Types for shavar tables.
     CHUNK_ADD,
-    CHUNK_SUB
+    CHUNK_SUB,
+    // Types for digest256 tables. digest256 tables differ in format from
+    // shavar tables since they only contain complete hashes.
+    CHUNK_ADD_DIGEST,
+    CHUNK_SUB_DIGEST
   };
 
   struct ChunkState {
     ChunkType type;
     uint32_t num;
     uint32_t hashSize;
     uint32_t length;
     void Clear() { num = 0; hashSize = 0; length = 0; }
@@ -101,16 +113,18 @@ private:
   nsCOMPtr<nsICryptoHMAC> mHMAC;
   nsCString mServerMAC;
 
   uint32_t mUpdateWait;
   bool mResetRequested;
   bool mRekeyRequested;
 
   nsTArray<ForwardedUpdate> mForwards;
+  // Keep track of updates to apply before passing them to the DBServiceWorkers.
   nsTArray<TableUpdate*> mTableUpdates;
+  // Updates to apply to the current table being parsed.
   TableUpdate *mTableUpdate;
 };
 
 }
 }
 
 #endif
--- a/toolkit/components/url-classifier/content/listmanager.js
+++ b/toolkit/components/url-classifier/content/listmanager.js
@@ -1,13 +1,14 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 
+// This is the only implementation of nsIUrlListManager.
 // A class that manages lists, namely white and black lists for
 // phishing or malware protection. The ListManager knows how to fetch,
 // update, and store lists.
 //
 // There is a single listmanager for the whole application.
 //
 // TODO more comprehensive update tests, for example add unittest check 
 //      that the listmanagers tables are properly written on updates
--- a/toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
+++ b/toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
@@ -74,19 +74,19 @@ interface nsIUrlClassifierUpdateObserver
  * This is a proxy class that is instantiated and called from the JS thread.
  * It provides async methods for querying and updating the database.  As the
  * methods complete, they call the callback function.
  */
 [scriptable, uuid(e326ec41-46fd-4127-ad3c-3c58b2cdf196)]
 interface nsIUrlClassifierDBService : nsISupports
 {
   /**
-   * Looks up a key in the database.
+   * Looks up a URI in the database.
    *
-   * @param key: The principal containing the information to search.
+   * @param principal: The principal containing the URI to search.
    * @param c: The callback will be called with a comma-separated list
    *        of tables to which the key belongs.
    */
   void lookup(in nsIPrincipal principal,
               in nsIUrlClassifierCallback c);
 
   /**
    * Lists the tables along with which chunks are available in each table.
--- a/toolkit/components/url-classifier/nsIUrlClassifierHashCompleter.idl
+++ b/toolkit/components/url-classifier/nsIUrlClassifierHashCompleter.idl
@@ -38,19 +38,21 @@ interface nsIUrlClassifierHashCompleterC
    *        NS_OK if the request completed successfully, or an error code.
    */
   void completionFinished(in nsresult status);
 };
 
 /**
  * Clients updating the url-classifier database have the option of sending
  * partial (32-bit) hashes of URL fragments to be blacklisted.  If the
- * url-classifier encounters one of these truncated hashes, it will ask
- * an nsIUrlClassifierCompleter instance to asynchronously provide the
- * complete hash, along with some associated metadata.
+ * url-classifier encounters one of these truncated hashes, it will ask an
+ * nsIUrlClassifierCompleter instance to asynchronously provide the complete
+ * hash, along with some associated metadata.
+ * This is only ever used for testing and should absolutely be deleted (I
+ * think).
  */
 [scriptable, uuid(ade9b72b-3562-44f5-aba6-e63246be53ae)]
 interface nsIUrlClassifierHashCompleter : nsISupports
 {
   /**
    * Request a completed hash.
    *
    * @param partialHash
--- a/toolkit/components/url-classifier/nsIUrlClassifierStreamUpdater.idl
+++ b/toolkit/components/url-classifier/nsIUrlClassifierStreamUpdater.idl
@@ -18,16 +18,17 @@ interface nsIUrlClassifierStreamUpdater 
    * The Url to download from.  Should be plain ascii text.
    */
   attribute ACString updateUrl;
 
   /**
    * Try to download updates from updateUrl.  Only one instance of this
    * runs at a time, so we return false if another instance is already
    * running.
+   * This is used in nsIUrlListManager as well as in testing.
    * @param aRequestTables Comma-separated list of tables included in this
    *        update.
    * @param aRequestBody The body for the request.
    * @param aClientKey The client key for checking the update's MAC.
    * @param aSuccessCallback Called after a successful update.
    * @param aUpdateErrorCallback Called for problems applying the update
    * @param aDownloadErrorCallback Called if we get an http error or a
    *        connection refused error.
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -150,16 +150,17 @@ private:
   nsresult AddNoise(const Prefix aPrefix,
                     const nsCString tableName,
                     uint32_t aCount,
                     LookupResultArray& results);
 
   nsCOMPtr<nsICryptoHash> mCryptoHash;
 
   nsAutoPtr<Classifier> mClassifier;
+  // The class that actually parses the update chunks.
   nsAutoPtr<ProtocolParser> mProtocolParser;
 
   // Directory where to store the SB databases.
   nsCOMPtr<nsIFile> mCacheDir;
 
   // XXX: maybe an array of autoptrs.  Or maybe a class specifically
   // storing a series of updates.
   nsTArray<TableUpdate*> mTableUpdates;
@@ -453,16 +454,17 @@ nsUrlClassifierDBServiceWorker::BeginUpd
     rv = nsUrlClassifierUtils::DecodeClientKey(clientKey, mUpdateClientKey);
     NS_ENSURE_SUCCESS(rv, rv);
     LOG(("clientKey present, marking update key"));
   }
 
   return NS_OK;
 }
 
+// Called from the stream updater.
 NS_IMETHODIMP
 nsUrlClassifierDBServiceWorker::BeginStream(const nsACString &table,
                                             const nsACString &serverMAC)
 {
   LOG(("nsUrlClassifierDBServiceWorker::BeginStream"));
 
   if (gShuttingDownThread)
     return NS_ERROR_NOT_INITIALIZED;
@@ -534,16 +536,17 @@ nsUrlClassifierDBServiceWorker::UpdateSt
 {
   if (gShuttingDownThread)
     return NS_ERROR_NOT_INITIALIZED;
 
   NS_ENSURE_STATE(mInStream);
 
   HandlePendingLookups();
 
+  // Feed the chunk to the parser.
   return mProtocolParser->AppendStream(chunk);
 }
 
 NS_IMETHODIMP
 nsUrlClassifierDBServiceWorker::FinishStream()
 {
   if (gShuttingDownThread)
     return NS_ERROR_NOT_INITIALIZED;
@@ -714,19 +717,19 @@ nsUrlClassifierDBServiceWorker::CacheCom
       if (tables[table].Equals(resultsPtr->ElementAt(i).table)) {
         activeTable = true;
         break;
       }
     }
     if (activeTable) {
       TableUpdate * tu = pParse->GetTableUpdate(resultsPtr->ElementAt(i).table);
       LOG(("CacheCompletion Addchunk %d hash %X", resultsPtr->ElementAt(i).entry.addChunk,
-           resultsPtr->ElementAt(i).entry.hash.prefix));
+           resultsPtr->ElementAt(i).entry.ToUint32()));
       tu->NewAddComplete(resultsPtr->ElementAt(i).entry.addChunk,
-                         resultsPtr->ElementAt(i).entry.hash.complete);
+                         resultsPtr->ElementAt(i).entry.complete);
       tu->NewAddChunk(resultsPtr->ElementAt(i).entry.addChunk);
       tu->SetLocalUpdate();
       updates.AppendElement(tu);
       pParse->ForgetTableUpdates();
     } else {
       LOG(("Completion received, but table is not active, so not caching."));
     }
    }
@@ -914,17 +917,17 @@ nsUrlClassifierLookupCallback::Completio
     mCacheResults = new CacheResultArray();
     if (!mCacheResults)
       return NS_ERROR_OUT_OF_MEMORY;
   }
 
   if (verified) {
     CacheResult result;
     result.entry.addChunk = chunkId;
-    result.entry.hash.complete = hash;
+    result.entry.complete = hash;
     result.table = tableName;
 
     // OK if this fails, we just won't cache the item.
     mCacheResults->AppendElement(result);
   }
 
   // Check if this matched any of our results.
   for (uint32_t i = 0; i < mResults->Length(); i++) {
@@ -1295,16 +1298,17 @@ nsUrlClassifierDBService::LookupURI(nsIP
   nsCOMPtr<nsIUrlClassifierLookupCallback> proxyCallback =
     new UrlClassifierLookupCallbackProxy(callback);
 
   // Queue this lookup and call the lookup function to flush the queue if
   // necessary.
   rv = mWorker->QueueLookup(key, proxyCallback);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  // This seems to just call HandlePendingLookups.
   return mWorkerProxy->Lookup(nullptr, nullptr);
 }
 
 NS_IMETHODIMP
 nsUrlClassifierDBService::GetTables(nsIUrlClassifierCallback* c)
 {
   NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
 
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.h
@@ -65,17 +65,18 @@ public:
 
 private:
   // No subclassing
   ~nsUrlClassifierDBService();
 
   // Disallow copy constructor
   nsUrlClassifierDBService(nsUrlClassifierDBService&);
 
-  nsresult LookupURI(nsIPrincipal* aPrincipal, nsIUrlClassifierCallback* c,
+  nsresult LookupURI(nsIPrincipal* aPrincipal,
+                     nsIUrlClassifierCallback* c,
                      bool forceCheck, bool *didCheck);
 
   // Close db connection and join the background thread if it exists.
   nsresult Shutdown();
 
   // Check if the key is on a known-clean host.
   nsresult CheckClean(const nsACString &lookupKey,
                       bool *clean);
--- a/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
@@ -22,16 +22,18 @@ static const char* gQuitApplicationMessa
 #if defined(PR_LOGGING)
 static const PRLogModuleInfo *gUrlClassifierStreamUpdaterLog = nullptr;
 #define LOG(args) PR_LOG(gUrlClassifierStreamUpdaterLog, PR_LOG_DEBUG, args)
 #else
 #define LOG(args)
 #endif
 
 
+// This class does absolutely nothing, except pass requests onto the DBService.
+
 ///////////////////////////////////////////////////////////////////////////////
 // nsIUrlClassiferStreamUpdater implementation
 // Handles creating/running the stream listener
 
 nsUrlClassifierStreamUpdater::nsUrlClassifierStreamUpdater()
   : mIsUpdating(false), mInitialized(false), mDownloadError(false),
     mBeganStream(false), mUpdateUrl(nullptr), mChannel(nullptr)
 {
@@ -102,23 +104,25 @@ nsUrlClassifierStreamUpdater::FetchUpdat
   uint32_t loadFlags = nsIChannel::INHIBIT_CACHING |
                        nsIChannel::LOAD_BYPASS_CACHE;
   rv = NS_NewChannel(getter_AddRefs(mChannel), aUpdateUrl, nullptr, nullptr, this,
                      loadFlags);
   NS_ENSURE_SUCCESS(rv, rv);
 
   mBeganStream = false;
 
+  // If aRequestBody is empty, construct it for the test.
   if (!aRequestBody.IsEmpty()) {
     rv = AddRequestBody(aRequestBody);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   // Set the appropriate content type for file/data URIs, for unit testing
   // purposes.
+  // This is only used for testing and should be deleted.
   bool match;
   if ((NS_SUCCEEDED(aUpdateUrl->SchemeIs("file", &match)) && match) ||
       (NS_SUCCEEDED(aUpdateUrl->SchemeIs("data", &match)) && match)) {
     mChannel->SetContentType(NS_LITERAL_CSTRING("application/vnd.google.safebrowsing-update"));
   }
 
   // Make the request
   rv = mChannel->AsyncOpen(this, nullptr);
@@ -209,18 +213,19 @@ nsUrlClassifierStreamUpdater::DownloadUp
 
   mIsUpdating = true;
   *_retval = true;
 
   nsAutoCString urlSpec;
   mUpdateUrl->GetAsciiSpec(urlSpec);
 
   LOG(("FetchUpdate: %s", urlSpec.get()));
-  //LOG(("requestBody: %s", aRequestBody.get()));
+  //LOG(("requestBody: %s", aRequestBody.Data()));
 
+  LOG(("Calling into FetchUpdate"));
   return FetchUpdate(mUpdateUrl, aRequestBody, EmptyCString(), EmptyCString());
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // nsIUrlClassifierUpdateObserver implementation
 
 NS_IMETHODIMP
 nsUrlClassifierStreamUpdater::UpdateUrlRequested(const nsACString &aUrl,
@@ -233,16 +238,19 @@ nsUrlClassifierStreamUpdater::UpdateUrlR
   if (!update)
     return NS_ERROR_OUT_OF_MEMORY;
 
   // Allow data: and file: urls for unit testing purposes, otherwise assume http
   if (StringBeginsWith(aUrl, NS_LITERAL_CSTRING("data:")) ||
       StringBeginsWith(aUrl, NS_LITERAL_CSTRING("file:"))) {
     update->mUrl = aUrl;
   } else {
+    // This must be fixed when bug 783047 is fixed. However, for unittesting
+    // update urls to localhost should use http, not https (otherwise the
+    // connection will fail silently, since there will be no cert available).
     update->mUrl = NS_LITERAL_CSTRING("http://") + aUrl;
   }
   update->mTable = aTable;
   update->mServerMAC = aServerMAC;
 
   return NS_OK;
 }
 
@@ -413,16 +421,17 @@ nsUrlClassifierStreamUpdater::OnStartReq
       NS_ENSURE_SUCCESS(rv, rv);
 
       if (!succeeded) {
         // 404 or other error, pass error status back
         LOG(("HTTP request returned failure code."));
 
         uint32_t requestStatus;
         rv = httpChannel->GetResponseStatus(&requestStatus);
+        LOG(("HTTP request returned failure code: %d.", requestStatus));
         NS_ENSURE_SUCCESS(rv, rv);
 
         strStatus.AppendInt(requestStatus);
         downloadError = true;
       }
     }
   }
 
@@ -457,17 +466,16 @@ nsUrlClassifierStreamUpdater::OnDataAvai
   nsresult rv;
 
   // Copy the data into a nsCString
   nsCString chunk;
   rv = NS_ConsumeStream(aIStream, aLength, chunk);
   NS_ENSURE_SUCCESS(rv, rv);
 
   //LOG(("Chunk (%d): %s\n\n", chunk.Length(), chunk.get()));
-
   rv = mDBService->UpdateStream(chunk);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsUrlClassifierStreamUpdater::OnStopRequest(nsIRequest *request, nsISupports* context,
--- a/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.h
@@ -47,25 +47,28 @@ private:
   // to reset the stream updater.
   void DownloadDone();
 
   // Disallow copy constructor
   nsUrlClassifierStreamUpdater(nsUrlClassifierStreamUpdater&);
 
   nsresult AddRequestBody(const nsACString &aRequestBody);
 
+  // Fetches an update for a single table.
   nsresult FetchUpdate(nsIURI *aURI,
                        const nsACString &aRequestBody,
                        const nsACString &aTable,
                        const nsACString &aServerMAC);
+  // Dumb wrapper so we don't have to create URIs.
   nsresult FetchUpdate(const nsACString &aURI,
                        const nsACString &aRequestBody,
                        const nsACString &aTable,
                        const nsACString &aServerMAC);
 
+  // Fetches the next table, from mPendingUpdates.
   nsresult FetchNext();
 
   bool mIsUpdating;
   bool mInitialized;
   bool mDownloadError;
   bool mBeganStream;
   nsCOMPtr<nsIURI> mUpdateUrl;
   nsCString mStreamTable;
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..3850373c19efeca42b55542bd7f9385fbd74e696
GIT binary patch
literal 939
zc$_P?GO{uTA~P-q1_u7Uj0LkA?S9YMZN9CyWtu;SY-!RTr{|nW8Zr(Sim$2dDz-8P
ztF{CymlW?mtNG%1oZ9p|vJrbO%%4;|_t28?2KU5DOPNIt-i{}MYNUK44g`xFoqaZ9
zY5&!|{U<ag#kZ77otXQf+*5#UBVTnSNKGh@+tn>kWiD_2|Mh&&f`}X2b!Bz*)irWM
ztfuZxeh`)cQj@TG>Y~fXv*WpzyOe0pe&;#wc1Ll<(#7&(n;iDtVBFUVa!=l*T{~=*
zSxzynFb@dje!J7`FT0zD@;`C+>a>Vgh06;-<}`Y25PbS0V~WMoaFN*StbT2WN}ej0
zIRxdfdp4L(?$iXC)2F4*nm0L!;X}6No98l9r&a!p-TX9^K}~T_Q=V0zw;@Q)jBu;E
zYM&X*^FBw%nOe9pEt&W}=wR!q)LCkM!e9O|{{z{z?A@8`%QuE^ZPGs%zI^i~(XHvb
zuLykCxfxw6JV!e6PajCl_oy%Shqdy=`MIiZ+}Zia$8*i`^xKd2$9xw`wd*|`eFkLL
z&lIMGEBg2zzSfKlKK^LSt((o~)DE&tn*FkR>67-jXzgMv6GS8`Z~by)|EdqG_q}J?
zKDGK?>s^NaGg<a7ueZ&9COYTJilv;zR;CEmawgxx7tOfZx~%)g8HR&G^WzsK<Sj2=
zeyxLp_3OTXL#2YnR%Rg8h9)K;<pNyEMtjqrSM9pEk3s2X9#dj^W32d+{h=qbRvgUg
z>HcC0RKqp*u%V>-{)Q8i7kFw<`0;G6)Y}4QW<h<Ms@RQ-&-fiK1FB&;5UI3_-E@E1
zr}Ab?fwb2b=SS{8<^6oiWS@zi?RP5~rUBJ(znZ#9`gM$euf+bq&Morg%eKVK&{^V9
zCo_R(?-8}V*Bpzj%n=?DJuLa<pDL$jL^XS)^`4JK`yQ;acUWEj-u1+S>^ZAX&AeG`
zWdRH(1Fm8#ODJt<0Mc%51kx_O?XjTo`>NEb4HNFH=vke8bN`R;!Rt4;*X)xOJvsTP
z@@0^k*Be-(pQIgJ=4#`~tW+d;#Wp42rSO*pou-x-1lW_Vl>v=3M7T&dYVP;t7N!g;
z76<hW|3^l*?Y#f$RP?gF2aJxNe*4_vl^9StQs&dWy|B2le_P)v?+_PZ<;=joq}kI|
RAOAUhTXS3Et?2XB+5jv}i+%tA
new file mode 100644
--- /dev/null
+++ b/toolkit/components/url-classifier/tests/unit/data/digest2.chunk
@@ -0,0 +1,2 @@
+a:5:32:32
+_H^a7]=#nmnoQ
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/toolkit/components/url-classifier/tests/unit/test_digest256.js
@@ -0,0 +1,144 @@
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+
+XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
+                                  "resource://gre/modules/NetUtil.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "Promise",
+                                  "resource://gre/modules/Promise.jsm");
+// Global test server for serving safebrowsing updates.
+let gHttpServ = null;
+// Global nsIUrlClassifierDBService
+let gDbService = Cc["@mozilla.org/url-classifier/dbservice;1"]
+  .getService(Ci.nsIUrlClassifierDBService);
+// Security manager for creating nsIPrincipals from URIs
+let gSecMan = Cc["@mozilla.org/scriptsecuritymanager;1"]
+  .getService(Ci.nsIScriptSecurityManager);
+
+// A map of tables to arrays of update redirect urls.
+let gTables = {};
+
+// Construct an update from a file.
+function readFileToString(aFilename) {
+  let f = do_get_file(aFilename);
+  let stream = Cc["@mozilla.org/network/file-input-stream;1"]
+    .createInstance(Ci.nsIFileInputStream);
+  stream.init(f, -1, 0, 0);
+  let buf = NetUtil.readInputStreamToString(stream, stream.available());
+  return buf;
+}
+
+// Registers a table for which to serve update chunks. Returns a promise that
+// resolves when that chunk has been downloaded.
+function registerTableUpdate(aTable, aFilename) {
+  let deferred = Promise.defer();
+  // If we haven't been given an update for this table yet, add it to the map
+  if (!(aTable in gTables)) {
+    gTables[aTable] = [];
+  }
+
+  // The number of chunks associated with this table.
+  let numChunks = gTables[aTable].length + 1;
+  let redirectPath = "/" + aTable + "-" + numChunks;
+  let redirectUrl = "localhost:4444" + redirectPath;
+
+  // Store redirect url for that table so we can return it later when we
+  // process an update request.
+  gTables[aTable].push(redirectUrl);
+
+  gHttpServ.registerPathHandler(redirectPath, function(request, response) {
+    do_print("Mock safebrowsing server handling request for " + redirectPath);
+    let contents = readFileToString(aFilename);
+    response.setHeader("Content-Type",
+                       "application/vnd.google.safebrowsing-update", false);
+    response.setStatusLine(request.httpVersion, 200, "OK");
+    response.bodyOutputStream.write(contents, contents.length);
+    deferred.resolve(contents);
+  });
+  return deferred.promise;
+}
+
+// Construct a response with redirect urls.
+function processUpdateRequest() {
+  let response = "n:1000\n";
+  for (let table in gTables) {
+    response += "i:" + table + "\n";
+    for (let i = 0; i < gTables[table].length; ++i) {
+      response += "u:" + gTables[table][i] + "\n";
+    }
+  }
+  do_print("Returning update response: " + response);
+  return response;
+}
+
+// Set up our test server to handle update requests.
+function run_test() {
+  gHttpServ = new HttpServer();
+  gHttpServ.registerDirectory("/", do_get_cwd());
+
+  gHttpServ.registerPathHandler("/downloads", function(request, response) {
+    let buf = NetUtil.readInputStreamToString(request.bodyInputStream,
+      request.bodyInputStream.available());
+    let blob = processUpdateRequest();
+    response.setHeader("Content-Type",
+                       "application/vnd.google.safebrowsing-update", false);
+    response.setStatusLine(request.httpVersion, 200, "OK");
+    response.bodyOutputStream.write(blob, blob.length);
+  });
+
+  gHttpServ.start(4444);
+  run_next_test();
+}
+
+function createURI(s) {
+  let service = Cc["@mozilla.org/network/io-service;1"]
+    .getService(Ci.nsIIOService);
+  return service.newURI(s, null, null);
+}
+
+// Just throw if we ever get an update or download error.
+function handleError(aEvent) {
+  do_throw("We didn't download or update correctly: " + aEvent);
+}
+
+add_test(function test_update() {
+  let streamUpdater = Cc["@mozilla.org/url-classifier/streamupdater;1"]
+    .getService(Ci.nsIUrlClassifierStreamUpdater);
+  streamUpdater.updateUrl = "http://localhost:4444/downloads";
+
+  // Load up some update chunks for the safebrowsing server to serve.
+  registerTableUpdate("goog-downloadwhite-digest256", "data/digest1.chunk");
+  registerTableUpdate("goog-downloadwhite-digest256", "data/digest2.chunk");
+
+  // Download some updates, and don't continue until the downloads are done.
+  function updateSuccess(aEvent) {
+    // Timeout of n:1000 is constructed in processUpdateRequest above and
+    // passed back in the callback in nsIUrlClassifierStreamUpdater on success.
+    do_check_eq("1000", aEvent);
+    do_print("All data processed");
+    run_next_test();
+  }
+  streamUpdater.downloadUpdates(
+    "goog-downloadwhite-digest256",
+    "goog-downloadwhite-digest256;\n", "",
+    updateSuccess, handleError, handleError);
+});
+
+add_test(function test_url_not_whitelisted() {
+  let uri = createURI("http://example.com");
+  let principal = gSecMan.getNoAppCodebasePrincipal(uri);
+  gDbService.lookup(principal, function handleEvent(aEvent) {
+    // This URI is not on any lists.
+    do_check_eq("", aEvent);
+    run_next_test();
+  });
+});
+
+add_test(function test_url_whitelisted() {
+  // Hash of "whitelisted.com/" (canonicalized URL) is:
+  // 93CA5F48E15E9861CD37C2D95DB43D23CC6E6DE5C3F8FA6E8BE66F97CC518907
+  let uri = createURI("http://whitelisted.com");
+  let principal = gSecMan.getNoAppCodebasePrincipal(uri);
+  gDbService.lookup(principal, function handleEvent(aEvent) {
+    do_check_eq("goog-downloadwhite-digest256", aEvent);
+    run_next_test();
+  });
+});
--- a/toolkit/components/url-classifier/tests/unit/xpcshell.ini
+++ b/toolkit/components/url-classifier/tests/unit/xpcshell.ini
@@ -6,8 +6,9 @@ tail = tail_urlclassifier.js
 [test_backoff.js]
 [test_dbservice.js]
 [test_hashcompleter.js]
 # Bug 752243: Profile cleanup frequently fails
 skip-if = os == "mac" || os == "linux"
 [test_partial.js]
 [test_prefixset.js]
 [test_streamupdater.js]
+[test_digest256.js]