Bug 904607: Add protocol parser for -digest256 lists (r=gcp).
authorMonica Chew <mmc@mozilla.com>
Fri, 06 Sep 2013 17:12:33 -0700
changeset 159868 2be3551a5d8040c76b92302eb14cdb0852da393d
parent 159867 5e861b43ebf1a6d63b5621c41a6047ef3bec5120
child 159869 356866ae2f6885933451a92aae838af00d659679
push id407
push userlsblakk@mozilla.com
push dateTue, 03 Dec 2013 03:32:50 +0000
treeherdermozilla-release@babf8c9ebc52 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs904607
milestone26.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 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]