Separate http_security_headers from transport_security_state

net/base/hash_value.cc

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/base/hash_value.h"
+
+#include <cstdlib>
+#include <cstring>
+
+#include "base/base64.h"
+#include "base/logging.h"
+#include "base/sha1.h"
+#include "base/string_split.h"
+#include "base/string_util.h"
+
+namespace net {
+
+bool HashValue::Equals(const HashValue& other) const {
+  if (tag != other.tag)
+    return false;
+  switch (tag) {
+    case HASH_VALUE_SHA1:
+      return fingerprint.sha1.Equals(other.fingerprint.sha1);
+    case HASH_VALUE_SHA256:
+      return fingerprint.sha256.Equals(other.fingerprint.sha256);
+    default:
+      NOTREACHED() << "Unknown HashValueTag " << tag;
+      return false;
+  }
+}
+
+bool HashValue::FromString(const std::string& value) {
+  std::string base64_str;

[Ryan Sleevi, 2012/11/13 19:02:32]

nit: (minor) You can use a base::StringPiece here, and avoid a copy

[unsafe, 2012/11/13 23:20:18]

Leaving std::string's in-place for now as I don't have a good understanding of
StringPiece.

[Ryan Sleevi, 2012/11/13 23:32:05]

base::StringPiece behaves like a string, except it does not copy - it just
stores a char* and length. It means you can slice and dice at will without
having to take a full copy.

std::strings are implicitly convertable to std::StringPiece, so it shouldn't
require any callsite changes along the way.

+  if (value.substr(0, 5) == "sha1/") {
+    tag = HASH_VALUE_SHA1;
+    base64_str = value.substr(5, 28);  // length of base64 string
+  } else if (value.substr(0, 7) == "sha256/") {
+    tag = HASH_VALUE_SHA256;
+    base64_str = value.substr(7, 44);  // length of base64 string

[Ryan Sleevi, 2012/11/13 19:02:32]

DESIGN: By taking the fixed length .substr() of a base64 string, you're
discarding any potentially invalid inputs from the tail of the string. This
potentially creates a place for implementation variance - where a string of 44
'A's followed by 'bababooey' would be accepted here, but perhaps rejected by
another implementation.

My inclination is that you should consider the entire string, and not truncate
at all.

[unsafe, 2012/11/13 23:20:18]

Done.

+  } else {
+    return false;
+  }
+
+  std::string decoded;
+  if (!base::Base64Decode(base64_str, &decoded) ||
+      decoded.size() != size()) {
+    return false;
+  }
+  memcpy(data(), decoded.data(), size());
+  return true;
+}
+
+std::string HashValue::ToString() const {
+  std::string base64_str;
+  base::Base64Encode(base::StringPiece(reinterpret_cast<const char*>(data()),
+                                       size()), &base64_str);
+  switch (tag) {
+  case HASH_VALUE_SHA1:
+    return std::string("sha1/" + base64_str);
+  case HASH_VALUE_SHA256:
+    return std::string("sha256/" + base64_str);
+  default:
+    NOTREACHED() << "Unknown HashValueTag " << tag;
+    return std::string("unknown/" + base64_str);
+  }

[Ryan Sleevi, 2012/11/13 19:02:32]

nit: the case labels of switch statements get indented one space (
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Loops_...
)

[unsafe, 2012/11/13 23:20:18]

Hm, where does it say that?  I see it saying 2-space indent?

[Ryan Sleevi, 2012/11/13 23:32:05]

Sorry, you're right - one indent, two spaces. So the inner code block is two
indents - four spaces.

+}
+
+size_t HashValue::size() const {
+  switch (tag) {
+    case HASH_VALUE_SHA1:
+      return sizeof(fingerprint.sha1.data);
+    case HASH_VALUE_SHA256:
+      return sizeof(fingerprint.sha256.data);
+    default:
+      NOTREACHED() << "Unknown HashValueTag " << tag;
+      // Although this is NOTREACHED, this function might be inlined and its
+      // return value can be passed to memset as the length argument. If we
+      // returned 0 here, it might result in what appears (in some stages of
+      // compilation) to be a call to to memset with a length argument of 0,
+      // which results in a warning. Therefore, we return a dummy value
+      // here.
+      return sizeof(fingerprint.sha1.data);
+  }
+}
+
+unsigned char* HashValue::data() {
+  return const_cast<unsigned char*>(const_cast<const HashValue*>(this)->data());
+}
+
+const unsigned char* HashValue::data() const {
+  switch (tag) {
+    case HASH_VALUE_SHA1:
+      return fingerprint.sha1.data;
+    case HASH_VALUE_SHA256:
+      return fingerprint.sha256.data;
+    default:
+      NOTREACHED() << "Unknown HashValueTag " << tag;
+      return NULL;
+  }
+}
+
+namespace {

[Ryan Sleevi, 2012/11/13 19:02:32]

This should be moved to ~line 17 (unnamed namespaces = top of file)

[unsafe, 2012/11/13 23:20:18]

Done.

+
+// CompareSHA1Hashes is a helper function for using bsearch() with an array of
+// SHA1 hashes.
+int CompareSHA1Hashes(const void* a, const void* b) {
+  return memcmp(a, b, base::kSHA1Length);
+}
+
+}  // namespace
+
+bool IsSHA1HashInSortedArray(const SHA1HashValue& hash,
+                             const uint8* array,
+                             size_t array_byte_len) {
+  DCHECK_EQ(0u, array_byte_len % base::kSHA1Length);
+  const size_t arraylen = array_byte_len / base::kSHA1Length;
+  return NULL != bsearch(hash.data, array, arraylen, base::kSHA1Length,

[palmer, 2012/11/13 19:35:04]

Perhaps it's better to use std::binary_search and <algorithm> instead of bsearch
and <cstdlib>, for maximum C++ "goodness" ?

[unsafe, 2012/11/13 23:20:18]

Maybe - I'm leaving this as-is, if that's OK.  This is only used in 2 places
(cert_verify_proc_win and cert_verify_proc_mac).  I wonder if it should just be
duplicated into those .cc files, and removed from hash_value.h/.cc?

+                         CompareSHA1Hashes);
+}
+
+}  // namespace net
+