Treat HSTS and HPKP state independently.

net/http/http_security_headers.cc

Index: net/http/http_security_headers.cc
diff --git a/net/http/http_security_headers.cc b/net/http/http_security_headers.cc
index 8d0c1465307f8adb5da5eb61907e2a0b8b2d9da6..56fa82b16ab00ee4b1f8a9d1f2414961e322ffef 100644
--- a/net/http/http_security_headers.cc
+++ b/net/http/http_security_headers.cc
@@ -285,6 +285,7 @@ bool ParseHPKPHeader(const std::string& value,
 
   std::string source = value;
 
+  hashes->clear();

[Ryan Sleevi, 2015/01/13 21:56:40]

BUG: You should not clear this unless things parse successfully. Doesn't this
have the side-effect of nuking the effective pins if a header fails to parse?
Either way, it's "surprising" (note how we intentionally don't update return
values until line 325, when we're guaranteed success)

[davidben, 2015/01/13 23:30:47]

Nah. AddHPKPHeader only ever calls it on a temporary now. (What I meant when I
said I accidentally fixed the pins overlaying.) Though I agree that shouldn't
nuke it. Done.

   while (!source.empty()) {
     StringPair semicolon = Split(source, ';');
     semicolon.first = Strip(semicolon.first);
@@ -325,18 +326,7 @@ bool ParseHPKPHeader(const std::string& value,
   *include_subdomains = include_subdomains_candidate;
   for (HashValueVector::const_iterator i = pins.begin();
        i != pins.end(); ++i) {
-    bool found = false;
-
-    for (HashValueVector::const_iterator j = hashes->begin();
-         j != hashes->end(); ++j) {
-      if (j->Equals(*i)) {
-        found = true;
-        break;
-      }
-    }
-
-    if (!found)
-      hashes->push_back(*i);
+    hashes->push_back(*i);

[Ryan Sleevi, 2015/01/13 21:56:40]

These are both HashValueVectors.

Why don't you just hashes->swap(pins)?

[davidben, 2015/01/13 23:30:47]

Yeah, that's better. Done.

   }
 
   return true;