Assert that HttpHeaders instances know about indexed ids
The HttpHeaderTable::Builder class allows you to reference its future table before it is fully built. But that HttpHeaderTable must be used to make HttpHeaders instances only after HttpHeaderTable::Builder::build() is invoked under peril of index out of bounds error in debug mode or segfault in release mode. These errors can be somewhat mysterious since they usually occur on a different stack from where the HttpHeaders instance was made. This commit adds an assertion that makes this situation more evident and logs useful diagnostic info about which header id was added after the construction. Hopefully, it should downgrade a segfault to failed Promise or thrown Exception for most users.
diff --git a/c++/src/kj/compat/http-test.c++ b/c++/src/kj/compat/http-test.c++
index 3e832fd..9a973b6 100644
--- a/c++/src/kj/compat/http-test.c++
+++ b/c++/src/kj/compat/http-test.c++
@@ -296,6 +296,48 @@
}
}
+KJ_TEST("HttpHeaders require valid HttpHeaderTable") {
+ const auto ERROR_MESSAGE =
+ "HttpHeaders object was constructed from HttpHeaderTable "
+ "that wasn't fully built yet at the time of construction"_kj;
+
+ {
+ // A tabula rasa is valid.
+ HttpHeaderTable table;
+ KJ_REQUIRE(table.isReady());
+
+ HttpHeaders headers(table);
+ }
+
+ {
+ // A future table is not valid.
+ HttpHeaderTable::Builder builder;
+
+ auto& futureTable = builder.getFutureTable();
+ KJ_REQUIRE(!futureTable.isReady());
+
+ auto makeHeadersThenBuild = [&]() {
+ HttpHeaders headers(futureTable);
+ auto table = builder.build();
+ };
+ KJ_EXPECT_THROW_MESSAGE(ERROR_MESSAGE, makeHeadersThenBuild());
+ }
+
+ {
+ // A well built table is valid.
+ HttpHeaderTable::Builder builder;
+
+ auto& futureTable = builder.getFutureTable();
+ KJ_REQUIRE(!futureTable.isReady());
+
+ auto ownedTable = builder.build();
+ KJ_REQUIRE(futureTable.isReady());
+ KJ_REQUIRE(ownedTable->isReady());
+
+ HttpHeaders headers(futureTable);
+ }
+}
+
KJ_TEST("HttpHeaders validation") {
auto table = HttpHeaderTable::Builder().build();
HttpHeaders headers(*table);
diff --git a/c++/src/kj/compat/http.c++ b/c++/src/kj/compat/http.c++
index da8ea2a..15de2ee 100644
--- a/c++/src/kj/compat/http.c++
+++ b/c++/src/kj/compat/http.c++
@@ -528,7 +528,9 @@
};
HttpHeaderTable::Builder::Builder()
- : table(kj::heap<HttpHeaderTable>()) {}
+ : table(kj::heap<HttpHeaderTable>()) {
+ table->buildStatus = BuildStatus::BUILDING;
+}
HttpHeaderId HttpHeaderTable::Builder::add(kj::StringPtr name) {
requireValidHeaderName(name);
@@ -576,7 +578,11 @@
HttpHeaders::HttpHeaders(const HttpHeaderTable& table)
: table(&table),
- indexedHeaders(kj::heapArray<kj::StringPtr>(table.idCount())) {}
+ indexedHeaders(kj::heapArray<kj::StringPtr>(table.idCount())) {
+ KJ_ASSERT(
+ table.isReady(), "HttpHeaders object was constructed from "
+ "HttpHeaderTable that wasn't fully built yet at the time of construction");
+}
void HttpHeaders::clear() {
for (auto& header: indexedHeaders) {
diff --git a/c++/src/kj/compat/http.h b/c++/src/kj/compat/http.h
index a89fbf2..a59932e 100644
--- a/c++/src/kj/compat/http.h
+++ b/c++/src/kj/compat/http.h
@@ -234,9 +234,20 @@
kj::StringPtr idToString(HttpHeaderId id) const;
// Get the canonical string name for the given ID.
+ bool isReady() const;
+ // Returns true if this HttpHeaderTable either was default constructed or its Builder has
+ // invoked `build()` and released it.
+
private:
kj::Vector<kj::StringPtr> namesById;
kj::Own<IdsByNameMap> idsByName;
+
+ enum class BuildStatus {
+ UNSTARTED = 0,
+ BUILDING = 1,
+ FINISHED = 2,
+ };
+ BuildStatus buildStatus = BuildStatus::UNSTARTED;
};
class HttpHeaders {
@@ -1077,10 +1088,22 @@
"the provided HttpHeaderId is from the wrong HttpHeaderTable");
}
-inline kj::Own<HttpHeaderTable> HttpHeaderTable::Builder::build() { return kj::mv(table); }
+inline kj::Own<HttpHeaderTable> HttpHeaderTable::Builder::build() {
+ table->buildStatus = BuildStatus::FINISHED;
+ return kj::mv(table);
+}
inline HttpHeaderTable& HttpHeaderTable::Builder::getFutureTable() { return *table; }
inline uint HttpHeaderTable::idCount() const { return namesById.size(); }
+inline bool HttpHeaderTable::isReady() const {
+ switch (buildStatus) {
+ case BuildStatus::UNSTARTED: return true;
+ case BuildStatus::BUILDING: return false;
+ case BuildStatus::FINISHED: return true;
+ }
+
+ KJ_UNREACHABLE;
+}
inline kj::StringPtr HttpHeaderTable::idToString(HttpHeaderId id) const {
id.requireFrom(*this);
