Add option to parse decimals and byte sequences strictly We retain lenient mode as the default for backwards compatibility. The corresponding metrics are only recorded in lenient mode, so that they can continue to be used to assess the feasibility of migrations to strict mode. PiperOrigin-RevId: 952844434
diff --git a/quiche/common/structured_headers.cc b/quiche/common/structured_headers.cc index 4af4359..4a6337a 100644 --- a/quiche/common/structured_headers.cc +++ b/quiche/common/structured_headers.cc
@@ -23,7 +23,6 @@ #include "absl/strings/string_view.h" #include "absl/types/span.h" #include "quiche/common/platform/api/quiche_client_stats.h" -#include "quiche/common/platform/api/quiche_flag_utils.h" #include "quiche/common/platform/api/quiche_logging.h" namespace quiche { @@ -107,8 +106,9 @@ kDraft09, kFinal, }; - explicit StructuredHeaderParser(absl::string_view str, DraftVersion version) - : input_(str), version_(version) { + explicit StructuredHeaderParser(absl::string_view str, DraftVersion version, + bool strict) + : input_(str), version_(version), strict_(strict) { // [SH09] 4.2 Step 1. // Discard any leading OWS from input_string. // [RFC8941] 4.2 Step 2. @@ -423,18 +423,19 @@ LogParseError("ReadNumber", "too many digits after decimal"); return std::nullopt; } - // TODO(b/517189418): This is never reached due to an off-by-one error. - if (i == decimal_position) { + const bool has_trailing_decimal = i == decimal_position + 1; + if (!strict_) { + // Counter to track instances of b/517189418 in which trailing decimal + // points are erroneously accepted. + QUICHE_CLIENT_HISTOGRAM_BOOL( + "StructuredHeaders.DecimalWithZeroFractionalDigits", + has_trailing_decimal, + "Whether a decimal point is erroneously accepted without any " + "digits following it."); + } else if (has_trailing_decimal) { LogParseError("ReadNumber", "no digits after decimal"); return std::nullopt; } - // Counter to track instances of b/517189418 in which trailing decimal - // points are erroneously accepted. - QUICHE_CLIENT_HISTOGRAM_BOOL( - "StructuredHeaders.DecimalWithZeroFractionalDigits", - i == decimal_position + 1, - "Whether a decimal point is erroneously accepted without any " - "digits following it."); } absl::string_view output_number_string = input_.substr(0, i); input_.remove_prefix(i); @@ -509,17 +510,20 @@ absl::string_view encoded = input_.substr(0, len); std::optional<std::string> binary = StrictBase64Decode(encoded); - bool is_strict = binary.has_value(); - if (!is_strict) { - binary = LenientBase64Decode(encoded); + const bool strict_decode_succeeded = binary.has_value(); + if (!strict_) { + if (!strict_decode_succeeded) { + binary = LenientBase64Decode(encoded); + } + if (binary.has_value()) { + QUICHE_CLIENT_HISTOGRAM_BOOL( + "StructuredHeaders.Base64DecodingIsStrictCompliant", + strict_decode_succeeded, + "Recorded true when RFC 8941 base64 decoding succeeds, false " + "when it fails but lenient legacy decoding succeeds."); + } } - if (binary) { - QUICHE_CLIENT_HISTOGRAM_BOOL( - "StructuredHeaders.Base64DecodingIsStrictCompliant", is_strict, - "Recorded true when RFC 8941 base64 decoding succeeds, false " - "when it fails but lenient legacy decoding succeeds."); - } - if (!binary) { + if (!binary.has_value()) { QUICHE_DVLOG(1) << "ReadByteSequence: failed to decode base64: " << encoded; return std::nullopt; @@ -575,7 +579,8 @@ } absl::string_view input_; - DraftVersion version_; + const DraftVersion version_; + const bool strict_; }; // Serializer for (a subset of) Structured Field Values for HTTP defined in @@ -912,43 +917,45 @@ } void Dictionary::clear() { members_.clear(); } -std::optional<ParameterizedItem> ParseItem(absl::string_view str) { - StructuredHeaderParser parser(str, StructuredHeaderParser::kFinal); +std::optional<ParameterizedItem> ParseItem(absl::string_view str, bool strict) { + StructuredHeaderParser parser(str, StructuredHeaderParser::kFinal, strict); std::optional<ParameterizedItem> item = parser.ReadItem(); if (item && parser.FinishParsing()) return item; return std::nullopt; } -std::optional<Item> ParseBareItem(absl::string_view str) { - StructuredHeaderParser parser(str, StructuredHeaderParser::kFinal); +std::optional<Item> ParseBareItem(absl::string_view str, bool strict) { + StructuredHeaderParser parser(str, StructuredHeaderParser::kFinal, strict); std::optional<Item> item = parser.ReadBareItem(); if (item && parser.FinishParsing()) return item; return std::nullopt; } std::optional<ParameterisedList> ParseParameterisedList(absl::string_view str) { - StructuredHeaderParser parser(str, StructuredHeaderParser::kDraft09); + StructuredHeaderParser parser(str, StructuredHeaderParser::kDraft09, + /*strict=*/false); std::optional<ParameterisedList> param_list = parser.ReadParameterisedList(); if (param_list && parser.FinishParsing()) return param_list; return std::nullopt; } std::optional<ListOfLists> ParseListOfLists(absl::string_view str) { - StructuredHeaderParser parser(str, StructuredHeaderParser::kDraft09); + StructuredHeaderParser parser(str, StructuredHeaderParser::kDraft09, + /*strict=*/false); std::optional<ListOfLists> list_of_lists = parser.ReadListOfLists(); if (list_of_lists && parser.FinishParsing()) return list_of_lists; return std::nullopt; } -std::optional<List> ParseList(absl::string_view str) { - StructuredHeaderParser parser(str, StructuredHeaderParser::kFinal); +std::optional<List> ParseList(absl::string_view str, bool strict) { + StructuredHeaderParser parser(str, StructuredHeaderParser::kFinal, strict); std::optional<List> list = parser.ReadList(); if (list && parser.FinishParsing()) return list; return std::nullopt; } -std::optional<Dictionary> ParseDictionary(absl::string_view str) { - StructuredHeaderParser parser(str, StructuredHeaderParser::kFinal); +std::optional<Dictionary> ParseDictionary(absl::string_view str, bool strict) { + StructuredHeaderParser parser(str, StructuredHeaderParser::kFinal, strict); std::optional<Dictionary> dictionary = parser.ReadDictionary(); if (dictionary && parser.FinishParsing()) return dictionary; return std::nullopt;
diff --git a/quiche/common/structured_headers.h b/quiche/common/structured_headers.h index a850d71..57f8944 100644 --- a/quiche/common/structured_headers.h +++ b/quiche/common/structured_headers.h
@@ -264,14 +264,22 @@ using List = std::vector<ParameterizedMember>; // Returns the result of parsing the header value as an Item, if it can be -// parsed as one, or nullopt if it cannot. Note that this uses the Draft 15 +// parsed as one, or nullopt if it cannot. Note that this uses the RFC 8941 // parsing rules, and so applies tighter range limits to integers. -QUICHE_EXPORT std::optional<ParameterizedItem> ParseItem(absl::string_view str); +// +// When `strict` is true, trailing decimal points are prohibited and byte +// sequences must strictly conform to the specification. +QUICHE_EXPORT std::optional<ParameterizedItem> ParseItem(absl::string_view str, + bool strict = false); // Returns the result of parsing the header value as an Item with no parameters, -// or nullopt if it cannot. Note that this uses the Draft 15 parsing rules, and +// or nullopt if it cannot. Note that this uses the RFC 8941 parsing rules, and // so applies tighter range limits to integers. -QUICHE_EXPORT std::optional<Item> ParseBareItem(absl::string_view str); +// +// When `strict` is true, trailing decimal points are prohibited and byte +// sequences must strictly conform to the specification. +QUICHE_EXPORT std::optional<Item> ParseBareItem(absl::string_view str, + bool strict = false); // Returns the result of parsing the header value as a Parameterised List, if it // can be parsed as one, or nullopt if it cannot. Note that parameter keys will @@ -291,16 +299,22 @@ absl::string_view str); // Returns the result of parsing the header value as a general List, if it can -// be parsed as one, or nullopt if it cannot. -// Structured-Headers Draft 15 only. -QUICHE_EXPORT std::optional<List> ParseList(absl::string_view str); +// be parsed as one, or nullopt if it cannot. RFC 8941 only. +// +// When `strict` is true, trailing decimal points are prohibited and byte +// sequences must strictly conform to the specification. +QUICHE_EXPORT std::optional<List> ParseList(absl::string_view str, + bool strict = false); // Returns the result of parsing the header value as a general Dictionary, if it -// can be parsed as one, or nullopt if it cannot. Structured-Headers Draft 15 -// only. -QUICHE_EXPORT std::optional<Dictionary> ParseDictionary(absl::string_view str); +// can be parsed as one, or nullopt if it cannot. RFC 8941 only. +// +// When `strict` is true, trailing decimal points are prohibited and byte +// sequences must strictly conform to the specification. +QUICHE_EXPORT std::optional<Dictionary> ParseDictionary(absl::string_view str, + bool strict = false); -// Serialization is implemented for Structured-Headers Draft 15 only. +// Serialization is implemented for RFC 8941 only. QUICHE_EXPORT std::optional<std::string> SerializeItem(const Item& value); QUICHE_EXPORT std::optional<std::string> SerializeItem( const ParameterizedItem& value);
diff --git a/quiche/common/structured_headers_generated_test.cc b/quiche/common/structured_headers_generated_test.cc index a6263e6..f8fe166 100644 --- a/quiche/common/structured_headers_generated_test.cc +++ b/quiche/common/structured_headers_generated_test.cc
@@ -17,7 +17,7 @@ // being automatically translated from the JSON source to C++ unit tests. Please // do not modify, as the contents will be overwritten when this is re-generated. -// Generated on 2026-06-26 from structured-field-tests.git @ +// Generated on 2026-07-14 from structured-field-tests.git @ // 1e280c3ed9ffe0ca5fdb1d97219dddc389007677. namespace quiche { @@ -76,10 +76,10 @@ 9, {{Item("hello", Item::kByteSequenceType), {}}}, ":aGVsbG8=:"}, - {"bad padding dot", ":aGVsbG8.:", 10, std::nullopt, nullptr, "b/393153699"}, + {"bad padding dot", ":aGVsbG8.:", 10, std::nullopt, nullptr}, {"bad end delimiter", ":aGVsbG8=", 9, std::nullopt, nullptr}, {"extra whitespace", ":aGVsb G8=:", 11, std::nullopt, nullptr}, - {"all whitespace", ": :", 6, std::nullopt, nullptr, "b/393408763"}, + {"all whitespace", ": :", 6, std::nullopt, nullptr}, {"extra chars", ":aGVsbG!8=:", 11, std::nullopt, nullptr}, {"suffix chars", ":aGVsbG8=!:", 11, std::nullopt, nullptr}, {"non-zero pad bits", @@ -1711,8 +1711,7 @@ 6, {{Item(-1.123), {}}}, nullptr}, - {"decimal with zero fractional digits", "1.", 2, std::nullopt, nullptr, - "b/517189418"}, + {"decimal with zero fractional digits", "1.", 2, std::nullopt, nullptr}, {"decimal with four fractional digits", "1.1234", 6, std::nullopt, nullptr}, {"negative decimal with four fractional digits", "-1.1234", 7, std::nullopt, nullptr}, @@ -9138,7 +9137,7 @@ if (c.raw) { SCOPED_TRACE(c.name); std::string raw{c.raw, c.raw_len}; - std::optional<ParameterizedItem> result = ParseItem(raw); + std::optional<ParameterizedItem> result = ParseItem(raw, /*strict=*/true); if (c.known_bug) { if (result == c.expected) { ADD_FAILURE() << "Test " << c.name @@ -9157,7 +9156,7 @@ if (c.raw) { SCOPED_TRACE(c.name); std::string raw{c.raw, c.raw_len}; - std::optional<List> result = ParseList(raw); + std::optional<List> result = ParseList(raw, /*strict=*/true); if (c.known_bug) { if (result == c.expected) { ADD_FAILURE() << "Test " << c.name @@ -9176,7 +9175,7 @@ if (c.raw) { SCOPED_TRACE(c.name); std::string raw{c.raw, c.raw_len}; - std::optional<Dictionary> result = ParseDictionary(raw); + std::optional<Dictionary> result = ParseDictionary(raw, /*strict=*/true); if (c.known_bug) { if (result == c.expected) { ADD_FAILURE() << "Test " << c.name
diff --git a/quiche/common/structured_headers_test.cc b/quiche/common/structured_headers_test.cc index 5cfa0ca..2b6a575 100644 --- a/quiche/common/structured_headers_test.cc +++ b/quiche/common/structured_headers_test.cc
@@ -62,6 +62,7 @@ const std::optional<Item> expected; // nullopt if parse error is expected. const char* canonical; // nullptr if parse error is expected, or if canonical // format is identical to raw. + const bool strict = true; } item_test_cases[] = { // Token {"bad token - item", "abc$@%!", std::nullopt, nullptr}, @@ -75,6 +76,8 @@ {"too long integer", "1000000000000000", std::nullopt, nullptr}, {"negative too long integer", "-1000000000000000", std::nullopt, nullptr}, {"integral decimal", "1.0", Item(1.0), nullptr}, + {"trailing decimal", "1.", Item(1.0), "1.0", /*strict=*/false}, + {"trailing decimal strict", "1.", std::nullopt, nullptr, /*strict=*/true}, // String {"basic string", "\"foo\"", Item("foo"), nullptr}, {"non-ascii string", "\"f\xC3\xBC\xC3\xBC\"", std::nullopt, nullptr}, @@ -85,16 +88,15 @@ {"c-style hex escape in string", "\"\\x61\"", std::nullopt, nullptr}, {"valid quoting containing \\u", "\"\\\\u0061\"", Item("\\u0061"), nullptr}, {"c-style unicode escape in string", "\"\\u0061\"", std::nullopt, nullptr}, - // TODO(b/393440282): Remove the following two tests once - // structured_headers_generated_test.cc is updated to include them. - // Manually copied from - // https://github.com/httpwg/structured-field-tests/blob/d2ec605907058638faa895642063581939d84da6/binary.json#L40-L44 // TODO(b/393153699): This input should be rejected. - {"bad padding dot", ":YQ=.:", Item("a", Item::kByteSequenceType), ":YQ==:"}, - // Manually copied from - // https://github.com/httpwg/structured-field-tests/blob/d2ec605907058638faa895642063581939d84da6/binary.json#L58-L62 + {"bad padding dot", ":YQ=.:", Item("a", Item::kByteSequenceType), + ":YQ==:", /*strict=*/false}, + {"bad padding dot strict", ":YQ=.:", std::nullopt, + ":YQ==:", /*strict=*/true}, // TODO(b/393408763): This input should be rejected. - {"all whitespace", ": :", Item("", Item::kByteSequenceType), "::"}, + {"all whitespace", ": :", Item("", Item::kByteSequenceType), + "::", /*strict=*/false}, + {"all whitespace strict", ": :", std::nullopt, "::", /*strict=*/true}, }; const ItemTestCase sh09_item_test_cases[] = { @@ -293,7 +295,7 @@ TEST(StructuredHeaderTest, ParseBareItem) { for (const auto& c : item_test_cases) { SCOPED_TRACE(c.name); - std::optional<Item> result = ParseBareItem(c.raw); + std::optional<Item> result = ParseBareItem(c.raw, c.strict); EXPECT_EQ(result, c.expected); } }