Fix Known-Length BHTTP frame decoding The current logic assumes the framing indicator is only one byte, but [RFC 9292](https://www.rfc-editor.org/rfc/rfc9292.html) states that the request and response framing indicators use the variable-length integer encoding from [RFC 9000](https://www.rfc-editor.org/rfc/rfc9000#section-16) and that values do not need to be encoded on the minimum number of bytes necessary. This means the logic has to also handle 2, 4, and 8-byte long values. Protected by Tests, this change still handles the 1-byte framing indicator. PiperOrigin-RevId: 774991715
diff --git a/quiche/binary_http/binary_http_message.cc b/quiche/binary_http/binary_http_message.cc index cf95e34..815b30e 100644 --- a/quiche/binary_http/binary_http_message.cc +++ b/quiche/binary_http/binary_http_message.cc
@@ -24,8 +24,8 @@ namespace quiche { namespace { -constexpr uint8_t kKnownLengthRequestFraming = 0; -constexpr uint8_t kKnownLengthResponseFraming = 1; +constexpr uint64_t kKnownLengthRequestFraming = 0; +constexpr uint64_t kKnownLengthResponseFraming = 1; bool ReadStringValue(quiche::QuicheDataReader& reader, std::string& data) { absl::string_view data_view; @@ -270,7 +270,7 @@ std::string data; data.resize(EncodedSize()); quiche::QuicheDataWriter writer(data.size(), data.data()); - if (!writer.WriteUInt8(kKnownLengthResponseFraming)) { + if (!writer.WriteVarInt62(kKnownLengthResponseFraming)) { return absl::InvalidArgumentError("Failed to write framing indicator"); } // Informational response @@ -293,7 +293,7 @@ } size_t BinaryHttpResponse::EncodedSize() const { - size_t size = sizeof(kKnownLengthResponseFraming); + size_t size = QuicheDataWriter::GetVarInt62Len(kKnownLengthResponseFraming); for (const auto& informational : informational_response_control_data_) { size += informational.EncodedSize(); } @@ -363,8 +363,9 @@ } size_t BinaryHttpRequest::EncodedSize() const { - return sizeof(kKnownLengthRequestFraming) + EncodedControlDataSize() + - EncodedKnownLengthFieldsAndBodySize() + num_padding_bytes(); + return QuicheDataWriter::GetVarInt62Len(kKnownLengthRequestFraming) + + EncodedControlDataSize() + EncodedKnownLengthFieldsAndBodySize() + + num_padding_bytes(); } // https://www.ietf.org/archive/id/draft-ietf-httpbis-binary-message-06.html#name-known-length-messages @@ -372,7 +373,7 @@ std::string data; data.resize(EncodedSize()); quiche::QuicheDataWriter writer(data.size(), data.data()); - if (!writer.WriteUInt8(kKnownLengthRequestFraming)) { + if (!writer.WriteVarInt62(kKnownLengthRequestFraming)) { return absl::InvalidArgumentError("Failed to encode framing indicator."); } if (const absl::Status status = EncodeControlData(writer); !status.ok()) { @@ -390,8 +391,8 @@ absl::StatusOr<BinaryHttpRequest> BinaryHttpRequest::Create( absl::string_view data) { quiche::QuicheDataReader reader(data); - uint8_t framing; - if (!reader.ReadUInt8(&framing)) { + uint64_t framing; + if (!reader.ReadVarInt62(&framing)) { return absl::InvalidArgumentError("Missing framing indicator."); } if (framing == kKnownLengthRequestFraming) { @@ -404,8 +405,8 @@ absl::StatusOr<BinaryHttpResponse> BinaryHttpResponse::Create( absl::string_view data) { quiche::QuicheDataReader reader(data); - uint8_t framing; - if (!reader.ReadUInt8(&framing)) { + uint64_t framing; + if (!reader.ReadVarInt62(&framing)) { return absl::InvalidArgumentError("Missing framing indicator."); } if (framing == kKnownLengthResponseFraming) {
diff --git a/quiche/binary_http/binary_http_message_test.cc b/quiche/binary_http/binary_http_message_test.cc index 5841b2a..0ae8e88 100644 --- a/quiche/binary_http/binary_http_message_test.cc +++ b/quiche/binary_http/binary_http_message_test.cc
@@ -7,6 +7,8 @@ #include <vector> #include "absl/container/flat_hash_map.h" +#include "absl/strings/escaping.h" +#include "absl/strings/string_view.h" #include "quiche/common/platform/api/quiche_test.h" using ::testing::ContainerEq; @@ -84,21 +86,27 @@ } TEST(BinaryHttpRequest, DecodeGetNoBody) { - const uint32_t words[] = { - 0x00034745, 0x54056874, 0x74707300, 0x0a2f6865, 0x6c6c6f2e, 0x74787440, - 0x6c0a7573, 0x65722d61, 0x67656e74, 0x34637572, 0x6c2f372e, 0x31362e33, - 0x206c6962, 0x6375726c, 0x2f372e31, 0x362e3320, 0x4f70656e, 0x53534c2f, - 0x302e392e, 0x376c207a, 0x6c69622f, 0x312e322e, 0x3304686f, 0x73740f77, - 0x77772e65, 0x78616d70, 0x6c652e63, 0x6f6d0f61, 0x63636570, 0x742d6c61, - 0x6e677561, 0x67650665, 0x6e2c206d, 0x69000000}; + absl::string_view known_length_request = + "80000000" // 4-byte framing. The framing indicator is normally encoded + // using a single byte but we intentionally use a + // multiple-byte value here to test the decoding logic. + "03474554" // :method = GET + "056874747073" // :scheme = https + "00" // :authority = "" + "0A2F68656C6C6F2E747874" // :path = /hello.txt + "406C" // headers section length + "0A757365722D6167656E74" // user-agent + "346375726C2F372E31362E33206C69626375726C2F372E31362E33204F70656E53534C2F" + "302E392E376C207A6C69622F312E322E33" // curl/7.16.3 libcurl/7.16.3 + // OpenSSL/0.9.7l zlib/1.2.3 + "04686F7374" // host + "0F7777772E6578616D706C652E636F6D" // www.example.com + "0F6163636570742D6C616E6775616765" // accept-language + "06656E2C206D69" // en, mi + "000000"; // padding + std::string data; - for (const auto& word : words) { - data += WordToBytes(word); - } - - // Remove all padding - data.resize(data.size() - 3); - + ASSERT_TRUE(absl::HexStringToBytes(known_length_request, &data)); const auto request_so = BinaryHttpRequest::Create(data); ASSERT_TRUE(request_so.ok()); const BinaryHttpRequest request = *request_so; @@ -409,16 +417,23 @@ } TEST(BinaryHttpResponse, DecodeNoBody) { + absl::string_view known_length_request = + "80000001" // 4-byte framing. The framing indicator is normally encoded + // using a single byte but we intentionally use a + // multiple-byte value here to test the decoding logic. + "4194" // status 404 + "0E" // field section length + "06736572766572" // server + "06417061636865" // Apache + "000000"; // padding + + std::string data; + ASSERT_TRUE(absl::HexStringToBytes(known_length_request, &data)); /* HTTP/1.1 404 Not Found Server: Apache */ - const uint32_t words[] = {0x0141940e, 0x06736572, 0x76657206, 0x41706163, - 0x68650000}; - std::string data; - for (const auto& word : words) { - data += WordToBytes(word); - } + const auto response_so = BinaryHttpResponse::Create(data); ASSERT_TRUE(response_so.ok()); const BinaryHttpResponse response = *response_so;