Refine the header validation performance further

Author: jasnell

src/workerd/api/headers.c++

@@ -4,21 +4,17 @@
 
 #include <workerd/io/features.h>
 #include <workerd/io/io-context.h>
+#include <workerd/util/header-validation.h>
 #include <workerd/util/strings.h>
 
-#ifdef _MSC_VER
-#define strncasecmp _strnicmp
-#define strcasecmp _stricmp
-#endif
-
 namespace workerd::api {
 
 namespace {
 // If any more headers are added to the CommonHeaderName enum later, we should be careful about
 // introducing them into serialization. We need to roll out a change that recognizes the new IDs
 // before rolling out a change that sends them. MAX_COMMON_HEADER_ID is the max value we're willing
 // to send.
-static constexpr size_t MAX_COMMON_HEADER_ID =
+constexpr size_t MAX_COMMON_HEADER_ID =
     static_cast<size_t>(capnp::CommonHeaderName::WWW_AUTHENTICATE);
 
 #define COMMON_HEADERS(V)                                                                          \
@@ -74,47 +70,98 @@ static constexpr size_t MAX_COMMON_HEADER_ID =
 // and must be kept in sync with the ordinal values defined in http-over-capnp.capnp). Since
 // it is extremely unlikely that those will change often, we hardcode them here for runtime
 // efficiency.
-#define V(Name) Name,
-static constexpr const char* COMMON_HEADER_NAMES[] = {nullptr,  // 0: invalid
+//
+// TODO(perf): We can potentially optimize this further by using the mechanisms within
+// http-over-capnp, which also has a mapping of common header names to kj::HttpHeaderIds.
+// However, accessing that functionality requires some amount of new API to be added to
+// capnproto which needs to be carefully weighed. There's also the fact that, currently,
+// the HttpOverCapnpFactory is accessed via IoContext and the Headers object can be
+// created outside of an IoContext. Some amount of additional refactoring would be needed
+// to make it work. For now, this hardcoded table is sufficient and efficient enough.
+#define V(Name) Name##_kj,
+constexpr kj::StringPtr COMMON_HEADER_NAMES[] = {nullptr,  // 0: invalid
   COMMON_HEADERS(V)};
 #undef V
 
-constexpr size_t constexprStrlen(const char* str) {
-  return *str ? 1 + constexprStrlen(str + 1) : 0;
+inline constexpr kj::StringPtr getCommonHeaderName(uint id) {
+  KJ_ASSERT(id > 0 && id <= MAX_COMMON_HEADER_ID, "Invalid common header ID");
+  return COMMON_HEADER_NAMES[id];
 }
 
-// Helper to avoid recalculating lengths of common headers at runtime repeatedly
-static constexpr size_t COMMON_HEADER_NAME_LENGTHS[] = {0,  // 0: invalid (nullptr)
-#define V(n) constexprStrlen(n),
-  COMMON_HEADERS(V)
-#undef V
-};
+constexpr bool strcaseeq(kj::StringPtr a, kj::StringPtr b) {
+  if (a.size() != b.size()) return false;
+  for (size_t i = 0; i < a.size(); ++i) {
+    char ca = a[i];
+    char cb = b[i];
+    // Convert to lowercase for comparison
+    if ('A' <= ca && ca <= 'Z') ca += 32;
+    if ('A' <= cb && cb <= 'Z') cb += 32;
+    if (ca != cb) return false;
+  }
+  return true;
+}
 
-inline constexpr kj::StringPtr getCommonHeaderName(uint id) {
-  KJ_ASSERT(id > 0 && id <= MAX_COMMON_HEADER_ID, "Invalid common header ID");
-  kj::StringPtr name = COMMON_HEADER_NAMES[id];
-  KJ_DASSERT(name != nullptr);
-  return name;
-}
-
-// Case-insensitive lookup of common header ID. This avoids allocating a lowercase copy
-// when the header is common. Returns kj::none if not a common header.
-// TODO(perf): It's possible to optimize this further with a good hash function but
-// for now a linear scan is sufficient.
-constexpr kj::Maybe<uint> getCommonHeaderId(kj::StringPtr name) {
-  size_t len = name.size();
-  if (len == 0) return kj::none;
-  for (uint i = 1; i <= MAX_COMMON_HEADER_ID; ++i) {
-    KJ_DASSERT(COMMON_HEADER_NAMES[i] != nullptr);
-    // If the lengths don't match or the first character doesn't match, skip full comparison
-    if (len != COMMON_HEADER_NAME_LENGTHS[i]) continue;
-    if (strncasecmp(name.begin(), COMMON_HEADER_NAMES[i], len) == 0) {
-      return i;
-    }
+constexpr uint caseInsensitiveHash(kj::StringPtr name) {
+  uint hash = 2166136261u;
+  for (size_t i = 0; i < name.size(); ++i) {
+    char c = name[i];
+    if ('A' <= c && c <= 'Z') c += 32;
+    hash ^= static_cast<uint8_t>(c);
+    hash *= 16777619u;
   }
-  return kj::none;
+  return hash;
 }
 
+constexpr size_t HEADER_MAP_SIZE = 128;
+
+// Constexpr hash table for case-insensitive mapping of header names to their
+// common header id (if any).
+struct HeaderHashTable final {
+  struct Entry {
+    kj::StringPtr name;
+    uint id;
+  };
+
+  Entry entries[HEADER_MAP_SIZE] = {};
+
+  constexpr HeaderHashTable() {
+    for (size_t i = 0; i < HEADER_MAP_SIZE; ++i) {
+      entries[i] = {nullptr, 0};
+    }
+
+    for (uint i = 1; i <= MAX_COMMON_HEADER_ID; ++i) {
+      auto name = COMMON_HEADER_NAMES[i];
+      size_t slot = caseInsensitiveHash(name) % HEADER_MAP_SIZE;
+      while (entries[slot].id != 0) {
+        slot = (slot + 1) % HEADER_MAP_SIZE;
+      }
+      entries[slot] = {name, i};
+    }
+  }
+
+  constexpr uint find(kj::StringPtr name) const {
+    if (name == nullptr) return 0;
+
+    size_t slot = caseInsensitiveHash(name) % HEADER_MAP_SIZE;
+
+    // Linear probe until we find a match or empty slot
+    for (size_t probes = 0; probes < HEADER_MAP_SIZE; ++probes) {
+      const auto& entry = entries[slot];
+      if (entry.id == 0) return 0;
+      if (entry.name.size() == name.size() && strcaseeq(entry.name, name)) {
+        return entry.id;
+      }
+      slot = (slot + 1) % HEADER_MAP_SIZE;
+    }
+    return 0;  // Not found
+  }
+};
+
+constexpr HeaderHashTable HEADER_HASH_TABLE;
+// Quick check to verify that the hash table is constructed correctly.
+static_assert(HEADER_HASH_TABLE.find("accept-charset"_kj) == 1);
+static_assert(HEADER_HASH_TABLE.find("AcCePt-ChArSeT"_kj) == 1);
+
 static_assert(std::size(COMMON_HEADER_NAMES) == (MAX_COMMON_HEADER_ID + 1));
 
 void warnIfBadHeaderString(const jsg::ByteString& byteString) {
@@ -156,14 +203,15 @@ void warnIfBadHeaderString(const jsg::ByteString& byteString) {
   }
 }
 
-// TODO(perf): This can be optimized further using a lookup table.
-constexpr bool isHttpWhitespace(char c) {
-  return c == '\t' || c == '\r' || c == '\n' || c == ' ';
+inline constexpr void requireValidHeaderValue(kj::StringPtr value) {
+  JSG_REQUIRE(workerd::util::isValidHeaderValue(value), TypeError, "Invalid header value.");
 }
 
-// TODO(perf): This can be optimized further using a lookup table.
-constexpr bool isValidHeaderValueChar(char c) {
-  return c != '\0' && c != '\r' && c != '\n';
+inline constexpr void requireValidHeaderName(const jsg::ByteString& name) {
+  warnIfBadHeaderString(name);
+  for (char c: name) {
+    JSG_REQUIRE(util::isHttpTokenChar(c), TypeError, "Invalid header name.");
+  }
 }
 
 // Left- and right-trim HTTP whitespace from `value`.
@@ -175,105 +223,33 @@ jsg::ByteString normalizeHeaderValue(jsg::Lock& js, jsg::ByteString value) {
   char* begin = value.begin();
   char* end = value.end();
 
-  while (begin < end && isHttpWhitespace(*begin)) ++begin;
-  while (begin < end && isHttpWhitespace(*(end - 1))) --end;
+  while (begin < end && util::isHttpWhitespace(*begin)) ++begin;
+  while (begin < end && util::isHttpWhitespace(*(end - 1))) --end;
 
   size_t newSize = end - begin;
   if (newSize == value.size()) return kj::mv(value);
 
   return jsg::ByteString(kj::str(kj::ArrayPtr(begin, newSize)));
 }
-
-// Fast lookup table for valid HTTP token characters (RFC 2616).
-// Valid token chars are: !#$%&'*+-.0-9A-Z^_`a-z|~
-// (i.e., any CHAR except CTLs or separators)
-static constexpr uint8_t HTTP_TOKEN_CHAR_TABLE[] = {
-  // Control characters 0x00-0x1F and 0x7F are invalid
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0x00-0x07
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0x08-0x0F
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0x10-0x17
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0x18-0x1F
-  0, 1, 0, 1, 1, 1, 1, 1,  // 0x20-0x27: SP!"#$%&'
-  0, 0, 1, 1, 0, 1, 1, 0,  // 0x28-0x2F: ()*+,-./
-  1, 1, 1, 1, 1, 1, 1, 1,  // 0x30-0x37: 01234567
-  1, 1, 0, 0, 0, 0, 0, 0,  // 0x38-0x3F: 89:;<=>?
-  0, 1, 1, 1, 1, 1, 1, 1,  // 0x40-0x47: @ABCDEFG
-  1, 1, 1, 1, 1, 1, 1, 1,  // 0x48-0x4F: HIJKLMNO
-  1, 1, 1, 1, 1, 1, 1, 1,  // 0x50-0x57: PQRSTUVW
-  1, 1, 1, 0, 0, 0, 1, 1,  // 0x58-0x5F: XYZ[\]^_
-  1, 1, 1, 1, 1, 1, 1, 1,  // 0x60-0x67: `abcdefg
-  1, 1, 1, 1, 1, 1, 1, 1,  // 0x68-0x6F: hijklmno
-  1, 1, 1, 1, 1, 1, 1, 1,  // 0x70-0x77: pqrstuvw
-  1, 1, 1, 0, 1, 0, 1, 0,  // 0x78-0x7F: xyz{|}~DEL
-  // Extended ASCII 0x80-0xFF are all invalid per RFC 2616
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0x80-0x87
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0x88-0x8F
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0x90-0x97
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0x98-0x9F
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0xA0-0xA7
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0xA8-0xAF
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0xB0-0xB7
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0xB8-0xBF
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0xC0-0xC7
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0xC8-0xCF
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0xD0-0xD7
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0xD8-0xDF
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0xE0-0xE7
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0xE8-0xEF
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0xF0-0xF7
-  0, 0, 0, 0, 0, 0, 0, 0,  // 0xF8-0xFF
-};
-
-inline void requireValidHeaderName(const jsg::ByteString& name) {
-  // TODO(cleanup): Code duplication with kj/compat/http.c++
-  warnIfBadHeaderString(name);
-
-  for (char c: name) {
-    JSG_REQUIRE(HTTP_TOKEN_CHAR_TABLE[static_cast<uint8_t>(c)], TypeError, "Invalid header name.");
-  }
-}
-
-inline void requireValidHeaderValue(kj::StringPtr value) {
-  for (char c: value) {
-    JSG_REQUIRE(isValidHeaderValueChar(c), TypeError, "Invalid header value.");
-  }
-}
 }  // namespace
 
-Headers::UncommonHeaderKey::UncommonHeaderKey(kj::String name)
-    : name(kj::mv(name)),
-      hash(kj::hashCode(this->name)) {}
-
-Headers::UncommonHeaderKey::UncommonHeaderKey(kj::StringPtr name)
-    : name(kj::str(name)),
-      hash(kj::hashCode(this->name)) {}
-
-bool Headers::UncommonHeaderKey::operator==(const UncommonHeaderKey& other) const {
-  // The same hash code is a necessary but not sufficient condition for equality.
-  return hash == other.hash && name == other.name;
-}
-
-bool Headers::UncommonHeaderKey::operator==(kj::StringPtr otherName) const {
-  if (name.size() != otherName.size()) return false;
-  return strncasecmp(name.begin(), otherName.begin(), name.size()) == 0;
-}
-
 Headers::HeaderKey Headers::getHeaderKeyFor(kj::StringPtr name) {
-  KJ_IF_SOME(commonId, getCommonHeaderId(name)) {
+  if (uint commonId = HEADER_HASH_TABLE.find(name)) {
+    KJ_DASSERT(commonId > 0 && commonId <= MAX_COMMON_HEADER_ID);
     return commonId;
   }
 
   // Not a common header, so allocate lowercase copy for uncommon header
-  return UncommonHeaderKey(toLower(name));
+  return toLower(name);
 }
 
 Headers::HeaderKey Headers::cloneHeaderKey(const HeaderKey& key) {
   KJ_SWITCH_ONEOF(key) {
     KJ_CASE_ONEOF(commonId, uint) {
       return commonId;
     }
-    KJ_CASE_ONEOF(uncommonKey, UncommonHeaderKey) {
-      return uncommonKey.clone();
+    KJ_CASE_ONEOF(uncommonKey, kj::String) {
+      return kj::str(uncommonKey);
     }
   }
   KJ_UNREACHABLE;
@@ -284,7 +260,7 @@ bool Headers::isSetCookie(const HeaderKey& key) {
     KJ_CASE_ONEOF(commonId, uint) {
       return commonId == static_cast<uint>(capnp::CommonHeaderName::SET_COOKIE);
     }
-    KJ_CASE_ONEOF(uncommonKey, UncommonHeaderKey) {
+    KJ_CASE_ONEOF(uncommonKey, kj::String) {
       // This case really shouldn't happen since "set-cookie" is a common header,
       // but just in case...
       return uncommonKey == "set-cookie";
@@ -294,21 +270,7 @@ bool Headers::isSetCookie(const HeaderKey& key) {
 }
 
 bool Headers::headerKeyEquals(const HeaderKey& a, const HeaderKey& b) {
-  KJ_SWITCH_ONEOF(a) {
-    KJ_CASE_ONEOF(aCommonId, uint) {
-      KJ_IF_SOME(bCommonId, b.tryGet<uint>()) {
-        return aCommonId == bCommonId;
-      }
-      return false;
-    }
-    KJ_CASE_ONEOF(aUncommonKey, UncommonHeaderKey) {
-      KJ_IF_SOME(bUncommonKey, b.tryGet<UncommonHeaderKey>()) {
-        return aUncommonKey == bUncommonKey;
-      }
-      return false;
-    }
-  }
-  KJ_UNREACHABLE;
+  return a == b;
 }
 
 Headers::Header::Header(jsg::ByteString name, kj::Vector<jsg::ByteString> values)
@@ -354,8 +316,8 @@ kj::StringPtr Headers::Header::Header::getKeyName() const {
     KJ_CASE_ONEOF(commonId, uint) {
       return COMMON_HEADER_NAMES[commonId];
     }
-    KJ_CASE_ONEOF(uncommonKey, UncommonHeaderKey) {
-      return uncommonKey.getName();
+    KJ_CASE_ONEOF(uncommonKey, kj::String) {
+      return uncommonKey;
     }
   }
   KJ_UNREACHABLE;
@@ -394,15 +356,15 @@ kj::uint Headers::HeaderCallbacks::hashCode(const HeaderKey& key) {
     KJ_CASE_ONEOF(commonId, uint) {
       return kj::hashCode(commonId);
     }
-    KJ_CASE_ONEOF(uncommonKey, UncommonHeaderKey) {
-      return uncommonKey.hashCode();
+    KJ_CASE_ONEOF(uncommonKey, kj::String) {
+      return kj::hashCode(uncommonKey);
     }
   }
   KJ_UNREACHABLE;
 }
 
 kj::uint Headers::HeaderCallbacks::hashCode(capnp::CommonHeaderName commondId) {
-  return kj::hashCode(static_cast<uint>(commondId));
+  return kj::hashCode(commondId);
 }
 
 Headers::Headers(jsg::Lock& js, jsg::Dict<jsg::ByteString, jsg::ByteString> dict)
@@ -580,7 +542,7 @@ kj::ArrayPtr<jsg::ByteString> Headers::getSetCookie() {
 kj::ArrayPtr<jsg::ByteString> Headers::getAll(jsg::ByteString name) {
   requireValidHeaderName(name);
 
-  if (strcasecmp(name.cStr(), "set-cookie") != 0) {
+  if (!strcaseeq(name, "set-cookie"_kj)) {
     JSG_FAIL_REQUIRE(TypeError, "getAll() can only be used with the header name \"Set-Cookie\".");
   }
 
@@ -776,7 +738,7 @@ void Headers::serialize(jsg::Lock& js, jsg::Serializer& serializer) {
         KJ_CASE_ONEOF(commonId, uint) {
           serializer.writeRawUint32(commonId);
         }
-        KJ_CASE_ONEOF(uncommonKey, UncommonHeaderKey) {
+        KJ_CASE_ONEOF(_, kj::String) {
           serializer.writeRawUint32(0);
           serializer.writeLengthDelimited(header.getHeaderName());
         }