Automated g4 rollback of changelist 425681702.
*** Reason for rollback ***
This causes test failures in Chromium. I would normally fix it, but currently QUICHE roll to Chromium is blocked on another CL and there are two other CLs that are complicated to roll. Please allow me to roll this one back for now, I'll be happy to help with debugging after I am able to roll the latest QUICHE into Chromium.
Error is:
[ RUN ] HeaderValidatorTest.NameHasInvalidChar
../../buildtools/third_party/libc++/trunk/include/array:205: _LIBCPP_ASSERT '__n < _Size' failed. out-of-bounds access in std::array<T, N>
Received signal 6
#0 0x7fe693f3799f base::debug::CollectStackTrace()
#1 0x7fe693c8fd3a base::debug::StackTrace::StackTrace()
#2 0x7fe693c8fcf5 base::debug::StackTrace::StackTrace()
#3 0x7fe693f3746c base::debug::(anonymous namespace)::StackDumpSignalHandler()
#4 0x7fe6911f1200 (/lib/x86_64-linux-gnu/libpthread-2.33.so+0x131ff)
#5 0x7fe690db3891 gsignal
#6 0x7fe690d9d536 abort
#7 0x7fe6912c941c std::__Cr::__libcpp_abort_debug_function()
#8 0x7fe6953c9996 std::__Cr::array<>::operator[]()
#9 0x7fe6953c95c9 http2::adapter::(anonymous namespace)::AllCharsInMap()
#10 0x7fe6953c87ec http2::adapter::(anonymous namespace)::IsValidHeaderName()
#11 0x7fe6953c7e15 http2::adapter::HeaderValidator::ValidateSingleHeader()
#12 0x56309a95f81d http2::adapter::test::HeaderValidatorTest_NameHasInvalidChar_Test::TestBody()
#13 0x56309bf0b83b testing::internal::HandleSehExceptionsInMethodIfSupported<>()
#14 0x56309befc767 testing::internal::HandleExceptionsInMethodIfSupported<>()
#15 0x56309beeaac1 testing::Test::Run()
#16 0x56309beeb1d7 testing::TestInfo::Run()
#17 0x56309beeb83d testing::TestSuite::Run()
#18 0x56309bef5e7a testing::internal::UnitTestImpl::RunAllTests()
#19 0x56309bf0fa3b testing::internal::HandleSehExceptionsInMethodIfSupported<>()
#20 0x56309befe337 testing::internal::HandleExceptionsInMethodIfSupported<>()
#21 0x56309bef5a11 testing::UnitTest::Run()
#22 0x56309c167aa1 RUN_ALL_TESTS()
*** Original change description ***
Use static char maps in HeaderValidator::ValidateSingleHeader().
When validating header names/values against allowed characters,
switching to static char maps may help with performance.
This CL is otherwise not a functional change.
***
PiperOrigin-RevId: 425943183
diff --git a/http2/adapter/header_validator.cc b/http2/adapter/header_validator.cc
index 470b9f7..dd605f4 100644
--- a/http2/adapter/header_validator.cc
+++ b/http2/adapter/header_validator.cc
@@ -1,7 +1,5 @@
#include "http2/adapter/header_validator.h"
-#include <array>
-
#include "absl/strings/escaping.h"
#include "absl/strings/numbers.h"
#include "common/platform/api/quiche_logging.h"
@@ -27,51 +25,26 @@
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-._~%!$&'()["
"]*+,;=:";
-using CharMap = std::array<bool, 256>;
-
-CharMap BuildValidCharMap(absl::string_view valid_chars) {
- CharMap map;
- map.fill(false);
- for (char c : valid_chars) {
- map[c] = true;
- }
- return map;
-}
-
-bool AllCharsInMap(absl::string_view str, const CharMap& map) {
- for (char c : str) {
- if (!map[c]) {
+// Returns whether `authority` contains only characters from the `host` ABNF
+// from RFC 3986 section 3.2.2.
+bool IsValidAuthority(absl::string_view authority) {
+ static const bool* valid_chars = []() {
+ using ValidCharArray = bool[256];
+ bool* chars = new ValidCharArray;
+ memset(chars, 0, sizeof(ValidCharArray));
+ for (char c : kValidAuthorityChars) {
+ chars[static_cast<uint8_t>(c)] = true;
+ }
+ return chars;
+ }();
+ for (char c : authority) {
+ if (!valid_chars[static_cast<uint8_t>(c)]) {
return false;
}
}
return true;
}
-// Returns whether `authority` contains only characters from the `host` ABNF
-// from RFC 3986 section 3.2.2.
-bool IsValidAuthority(absl::string_view authority) {
- static const CharMap valid_chars = BuildValidCharMap(kValidAuthorityChars);
- return AllCharsInMap(authority, valid_chars);
-}
-
-bool IsValidHeaderName(absl::string_view name) {
- static const CharMap valid_chars =
- BuildValidCharMap(kHttp2HeaderNameAllowedChars);
- return AllCharsInMap(name, valid_chars);
-}
-
-bool IsValidHeaderValue(absl::string_view value) {
- static const CharMap valid_chars =
- BuildValidCharMap(kHttp2HeaderValueAllowedChars);
- return AllCharsInMap(value, valid_chars);
-}
-
-bool IsValidStatus(absl::string_view status) {
- static const CharMap valid_chars =
- BuildValidCharMap(kHttp2StatusValueAllowedChars);
- return AllCharsInMap(status, valid_chars);
-}
-
bool ValidateRequestHeaders(const std::vector<std::string>& pseudo_headers,
absl::string_view method, absl::string_view path,
bool allow_connect) {
@@ -139,19 +112,23 @@
return HEADER_FIELD_TOO_LONG;
}
const absl::string_view validated_key = key[0] == ':' ? key.substr(1) : key;
- if (!IsValidHeaderName(validated_key)) {
+ if (validated_key.find_first_not_of(kHttp2HeaderNameAllowedChars) !=
+ absl::string_view::npos) {
QUICHE_VLOG(2) << "invalid chars in header name: ["
<< absl::CEscape(validated_key) << "]";
return HEADER_FIELD_INVALID;
}
- if (!IsValidHeaderValue(value)) {
+ if (value.find_first_not_of(kHttp2HeaderValueAllowedChars) !=
+ absl::string_view::npos) {
QUICHE_VLOG(2) << "invalid chars in header value: [" << absl::CEscape(value)
<< "]";
return HEADER_FIELD_INVALID;
}
if (key[0] == ':') {
if (key == ":status") {
- if (value.size() != 3 || !IsValidStatus(value)) {
+ if (value.size() != 3 ||
+ value.find_first_not_of(kHttp2StatusValueAllowedChars) !=
+ absl::string_view::npos) {
QUICHE_VLOG(2) << "malformed status value: [" << absl::CEscape(value)
<< "]";
return HEADER_FIELD_INVALID;
