Add `Dictionary(Dictionary::unique_keys, std::vector<DictionaryMember>)` constructor and deprecate the untagged vector constructor. Previously, `Dictionary(std::vector<DictionaryMember>)` accepted a vector without checking or deduplicating keys, allowing callers to accidentally violate `Dictionary`'s unique-keys invariant (where key-based accessors return the first entry for a duplicate key while `SerializeDictionary()` emits all entries, resolving to the last entry upon re-parsing per RFC 8941 Section 4.2.2). Introduce a tagged `Dictionary(Dictionary::unique_keys, std::vector<DictionaryMember>)` constructor (modeled after `Item`'s tag constructors and `base::sorted_unique`) that makes the unique-keys precondition explicit at call sites and enforces it with a `QUICHE_DCHECK`. Mark the existing untagged `Dictionary(std::vector<DictionaryMember>)` constructor as deprecated in favor of `Dictionary::unique_keys` (when keys are guaranteed unique) or `operator[](absl::string_view)` (otherwise), and migrate `StructuredHeaderParser::ReadDictionary()` and `webtransport::SerializeInitHeader()` to the tagged constructor. PiperOrigin-RevId: 1019378991
diff --git a/quiche/common/structured_headers.cc b/quiche/common/structured_headers.cc index fe95910..23f99c3 100644 --- a/quiche/common/structured_headers.cc +++ b/quiche/common/structured_headers.cc
@@ -16,6 +16,7 @@ #include "absl/algorithm/container.h" #include "absl/base/attributes.h" #include "absl/container/flat_hash_map.h" +#include "absl/container/flat_hash_set.h" #include "absl/functional/overload.h" #include "absl/strings/ascii.h" #include "absl/strings/escaping.h" @@ -239,7 +240,7 @@ SkipOWS(); if (input_.empty()) return std::nullopt; } - return Dictionary(std::move(members)); + return Dictionary(Dictionary::unique_keys, std::move(members)); } // Parses a Parameterised List ([SH09] 4.2.5). @@ -994,6 +995,20 @@ Dictionary& Dictionary::operator=(const Dictionary&) = default; Dictionary::Dictionary(Dictionary&&) noexcept = default; Dictionary& Dictionary::operator=(Dictionary&&) noexcept = default; +Dictionary::Dictionary(unique_keys_t, + std::vector<DictionaryMember> members) noexcept + : members_(std::move(members)) { + QUICHE_DCHECK([this] { + absl::flat_hash_set<absl::string_view> keys; + keys.reserve(members_.size()); + for (const auto& [key, unused_value] : members_) { + if (!keys.insert(key).second) { + return false; + } + } + return true; + }()) << "Dictionary initialized with duplicate keys"; +} Dictionary::Dictionary(std::vector<DictionaryMember> members) noexcept : members_(std::move(members)) {} Dictionary::~Dictionary() = default;
diff --git a/quiche/common/structured_headers.h b/quiche/common/structured_headers.h index 42af3a0..33464e0 100644 --- a/quiche/common/structured_headers.h +++ b/quiche/common/structured_headers.h
@@ -350,17 +350,30 @@ using mapped_type = ParameterizedMember; using value_type = DictionaryMember; + // Tag type to assert that the vector passed to the constructor already + // contains unique keys. + struct unique_keys_t { + constexpr explicit unique_keys_t() = default; + }; + static constexpr unique_keys_t unique_keys{}; + Dictionary() noexcept; Dictionary(const Dictionary&); Dictionary(Dictionary&&) noexcept; - // NOTE(b/580863224): |members| should not contain duplicate keys. This - // constructor currently stores |members| without deduplicating, causing - // key-based accessors (find(), at(), operator[]) to return the first entry - // for a duplicate key while SerializeDictionary() emits all entries (which - // resolves to the last entry when re-parsed per RFC 8941 Section 4.2.2). - // Use operator[](absl::string_view) when duplicate keys need to overwrite - // earlier values or when you cannot statically guarantee that the keys are - // unique. + + // |members| must not contain duplicate keys (enforced via QUICHE_DCHECK). Use + // operator[](absl::string_view) when duplicate keys need to overwrite earlier + // values or when you cannot statically guarantee that the keys are unique. + Dictionary(unique_keys_t, std::vector<DictionaryMember> members) noexcept; + + // Deprecated (b/580863224): Prefer Dictionary(Dictionary::unique_keys, ...) + // when keys are guaranteed unique, or operator[](absl::string_view) when + // duplicate keys need to overwrite earlier values or when you cannot + // statically guarantee that the keys are unique. This constructor stores + // |members| without deduplicating, causing key-based accessors (find(), at(), + // operator[]) to return the first entry for a duplicate key while + // SerializeDictionary() emits all entries (which resolves to the last entry + // when re-parsed per RFC 8941 Section 4.2.2). explicit Dictionary(std::vector<DictionaryMember> members) noexcept; ~Dictionary(); Dictionary& operator=(const Dictionary&);
diff --git a/quiche/common/structured_headers_test.cc b/quiche/common/structured_headers_test.cc index 37ed657..9dc4cf8 100644 --- a/quiche/common/structured_headers_test.cc +++ b/quiche/common/structured_headers_test.cc
@@ -717,6 +717,23 @@ EXPECT_EQ(2U, dict_init.size()); EXPECT_EQ(member0, dict_init.at(key0)); EXPECT_EQ(member1, dict_init.at(key1)); + + const Dictionary dict_unique(Dictionary::unique_keys, + {{key0, member0}, {key1, member1}}); + EXPECT_FALSE(dict_unique.empty()); + EXPECT_EQ(2U, dict_unique.size()); + EXPECT_EQ(member0, dict_unique.at(key0)); + EXPECT_EQ(member1, dict_unique.at(key1)); +} + +TEST(StructuredHeaderDeathTest, DictionaryUniqueKeysDuplicate) { + const ParameterizedMember member0(Item(Item::string, "Applepie")); + const ParameterizedMember member1(Item(Item::byte_sequence, "hello")); + + EXPECT_QUICHE_DEBUG_DEATH( + Dictionary(Dictionary::unique_keys, + {{"key0", member0}, {"key1", member1}, {"key0", member0}}), + "Dictionary initialized with duplicate keys"); } TEST(StructuredHeaderTest, DictionaryClear) {
diff --git a/quiche/web_transport/web_transport_headers.cc b/quiche/web_transport/web_transport_headers.cc index 4fa6b2a..a647481 100644 --- a/quiche/web_transport/web_transport_headers.cc +++ b/quiche/web_transport/web_transport_headers.cc
@@ -195,7 +195,7 @@ } std::optional<std::string> result = quiche::structured_headers::SerializeDictionary( - Dictionary(std::move(members))); + Dictionary(Dictionary::unique_keys, std::move(members))); if (!result.has_value()) { return absl::InternalError("Failed to serialize the dictionary"); }