Plumb error code from QpackDecoder to QuicSpdyStream.
Also, make QpackProgressiveDecoder::OnError() private.
Also, change four string_view() constructors to testing::Eq() helpers in
qpack_decoder_test.cc to be consistent with the other thirty EXPECT_CALLs.
(Either one is sufficient, but compilation fails in Chromium when a C-style
string literal is passed directly to EXPECT_CALL.)
Protected by FLAGS_quic_reloadable_flag_quic_reject_invalid_chars_in_field_value.
PiperOrigin-RevId: 398298181
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc
index f4fa3ee..365ad44 100644
--- a/quic/core/http/quic_spdy_stream.cc
+++ b/quic/core/http/quic_spdy_stream.cc
@@ -20,7 +20,6 @@
#include "quic/core/http/web_transport_http3.h"
#include "quic/core/qpack/qpack_decoder.h"
#include "quic/core/qpack/qpack_encoder.h"
-#include "quic/core/quic_error_codes.h"
#include "quic/core/quic_types.h"
#include "quic/core/quic_utils.h"
#include "quic/core/quic_versions.h"
@@ -565,14 +564,14 @@
}
}
-void QuicSpdyStream::OnHeaderDecodingError(absl::string_view error_message) {
+void QuicSpdyStream::OnHeaderDecodingError(QuicErrorCode error_code,
+ absl::string_view error_message) {
qpack_decoded_headers_accumulator_.reset();
std::string connection_close_error_message = absl::StrCat(
"Error decoding ", headers_decompressed_ ? "trailers" : "headers",
" on stream ", id(), ": ", error_message);
- OnUnrecoverableError(QUIC_QPACK_DECOMPRESSION_FAILED,
- connection_close_error_message);
+ OnUnrecoverableError(error_code, connection_close_error_message);
}
void QuicSpdyStream::MaybeSendPriorityUpdateFrame() {
diff --git a/quic/core/http/quic_spdy_stream.h b/quic/core/http/quic_spdy_stream.h
index e3e3d6a..50befb0 100644
--- a/quic/core/http/quic_spdy_stream.h
+++ b/quic/core/http/quic_spdy_stream.h
@@ -24,6 +24,7 @@
#include "quic/core/http/quic_header_list.h"
#include "quic/core/http/quic_spdy_stream_body_manager.h"
#include "quic/core/qpack/qpack_decoded_headers_accumulator.h"
+#include "quic/core/quic_error_codes.h"
#include "quic/core/quic_packets.h"
#include "quic/core/quic_stream.h"
#include "quic/core/quic_stream_sequencer.h"
@@ -217,7 +218,8 @@
// QpackDecodedHeadersAccumulator::Visitor implementation.
void OnHeadersDecoded(QuicHeaderList headers,
bool header_list_size_limit_exceeded) override;
- void OnHeaderDecodingError(absl::string_view error_message) override;
+ void OnHeaderDecodingError(QuicErrorCode error_code,
+ absl::string_view error_message) override;
QuicSpdySession* spdy_session() const { return spdy_session_; }
diff --git a/quic/core/qpack/fuzzer/qpack_decoder_fuzzer.cc b/quic/core/qpack/fuzzer/qpack_decoder_fuzzer.cc
index 9c51f61..466966a 100644
--- a/quic/core/qpack/fuzzer/qpack_decoder_fuzzer.cc
+++ b/quic/core/qpack/fuzzer/qpack_decoder_fuzzer.cc
@@ -9,6 +9,7 @@
#include "absl/strings/string_view.h"
#include "quic/core/qpack/qpack_decoder.h"
+#include "quic/core/quic_error_codes.h"
#include "quic/platform/api/quic_fuzzed_data_provider.h"
#include "quic/test_tools/qpack/qpack_decoder_test_utils.h"
#include "quic/test_tools/qpack/qpack_test_utils.h"
@@ -61,7 +62,8 @@
QUICHE_CHECK_EQ(1u, result);
}
- void OnDecodingErrorDetected(absl::string_view /*error_message*/) override {
+ void OnDecodingErrorDetected(QuicErrorCode /*error_code*/,
+ absl::string_view /*error_message*/) override {
*error_detected_ = true;
}
diff --git a/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc b/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
index b4ed2ef..c327bdc 100644
--- a/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
+++ b/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
@@ -24,6 +24,20 @@
namespace quic {
namespace test {
+namespace {
+
+// Find the first occurrence of invalid characters NUL, LF, CR in |*value| and
+// remove that and the remaining of the string.
+void TruncateValueOnInvalidChars(std::string* value) {
+ for (auto it = value->begin(); it != value->end(); ++it) {
+ if (*it == '\0' || *it == '\n' || *it == '\r') {
+ value->erase(it, value->end());
+ return;
+ }
+ }
+}
+
+} // anonymous namespace
// Class to hold QpackEncoder and its DecoderStreamErrorDelegate.
class EncodingEndpoint {
@@ -278,8 +292,10 @@
visitor_->OnHeaderBlockDecoded(stream_id_);
}
- void OnHeaderDecodingError(absl::string_view error_message) override {
- QUICHE_CHECK(false) << error_message;
+ void OnHeaderDecodingError(QuicErrorCode error_code,
+ absl::string_view error_message) override {
+ QUICHE_CHECK(false) << QuicErrorCodeToString(error_code) << " "
+ << error_message;
}
void Decode(absl::string_view data) { accumulator_.Decode(data); }
@@ -509,6 +525,7 @@
// Header name not in the static table, fuzzed header value.
name = "foo";
value = provider->ConsumeRandomLengthString(128);
+ TruncateValueOnInvalidChars(&value);
break;
case 11:
// Another header name not in the static table, empty header value.
@@ -525,11 +542,13 @@
// Another header name not in the static table, fuzzed header value.
name = "bar";
value = provider->ConsumeRandomLengthString(128);
+ TruncateValueOnInvalidChars(&value);
break;
default:
// Fuzzed header name and header value.
name = provider->ConsumeRandomLengthString(128);
value = provider->ConsumeRandomLengthString(128);
+ TruncateValueOnInvalidChars(&value);
}
header_list.AppendValueOrAddHeader(name, value);
diff --git a/quic/core/qpack/qpack_decoded_headers_accumulator.cc b/quic/core/qpack/qpack_decoded_headers_accumulator.cc
index 9a5af7e..4e50e72 100644
--- a/quic/core/qpack/qpack_decoded_headers_accumulator.cc
+++ b/quic/core/qpack/qpack_decoded_headers_accumulator.cc
@@ -67,13 +67,13 @@
}
void QpackDecodedHeadersAccumulator::OnDecodingErrorDetected(
- absl::string_view error_message) {
+ QuicErrorCode error_code, absl::string_view error_message) {
QUICHE_DCHECK(!error_detected_);
QUICHE_DCHECK(!headers_decoded_);
error_detected_ = true;
// Might destroy |this|.
- visitor_->OnHeaderDecodingError(error_message);
+ visitor_->OnHeaderDecodingError(error_code, error_message);
}
void QpackDecodedHeadersAccumulator::Decode(absl::string_view data) {
diff --git a/quic/core/qpack/qpack_decoded_headers_accumulator.h b/quic/core/qpack/qpack_decoded_headers_accumulator.h
index b22da2a..57d1839 100644
--- a/quic/core/qpack/qpack_decoded_headers_accumulator.h
+++ b/quic/core/qpack/qpack_decoded_headers_accumulator.h
@@ -11,6 +11,7 @@
#include "absl/strings/string_view.h"
#include "quic/core/http/quic_header_list.h"
#include "quic/core/qpack/qpack_progressive_decoder.h"
+#include "quic/core/quic_error_codes.h"
#include "quic/core/quic_types.h"
#include "quic/platform/api/quic_export.h"
@@ -45,7 +46,8 @@
bool header_list_size_limit_exceeded) = 0;
// Called when an error has occurred.
- virtual void OnHeaderDecodingError(absl::string_view error_message) = 0;
+ virtual void OnHeaderDecodingError(QuicErrorCode error_code,
+ absl::string_view error_message) = 0;
};
QpackDecodedHeadersAccumulator(QuicStreamId id,
@@ -59,7 +61,8 @@
void OnHeaderDecoded(absl::string_view name,
absl::string_view value) override;
void OnDecodingCompleted() override;
- void OnDecodingErrorDetected(absl::string_view error_message) override;
+ void OnDecodingErrorDetected(QuicErrorCode error_code,
+ absl::string_view error_message) override;
// Decode payload data.
// Must not be called if an error has been detected.
diff --git a/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc b/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
index 217f866..0aa08a0 100644
--- a/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
+++ b/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
@@ -46,9 +46,8 @@
OnHeadersDecoded,
(QuicHeaderList headers, bool header_list_size_limit_exceeded),
(override));
- MOCK_METHOD(void,
- OnHeaderDecodingError,
- (absl::string_view error_message),
+ MOCK_METHOD(void, OnHeaderDecodingError,
+ (QuicErrorCode error_code, absl::string_view error_message),
(override));
};
@@ -78,7 +77,8 @@
// HEADERS frame payload must have a complete Header Block Prefix.
TEST_F(QpackDecodedHeadersAccumulatorTest, EmptyPayload) {
EXPECT_CALL(visitor_,
- OnHeaderDecodingError(Eq("Incomplete header data prefix.")));
+ OnHeaderDecodingError(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Incomplete header data prefix.")));
accumulator_.EndHeaderBlock();
}
@@ -87,7 +87,8 @@
accumulator_.Decode(absl::HexStringToBytes("00"));
EXPECT_CALL(visitor_,
- OnHeaderDecodingError(Eq("Incomplete header data prefix.")));
+ OnHeaderDecodingError(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Incomplete header data prefix.")));
accumulator_.EndHeaderBlock();
}
@@ -110,14 +111,16 @@
TEST_F(QpackDecodedHeadersAccumulatorTest, TruncatedPayload) {
accumulator_.Decode(absl::HexStringToBytes("00002366"));
- EXPECT_CALL(visitor_, OnHeaderDecodingError(Eq("Incomplete header block.")));
+ EXPECT_CALL(visitor_, OnHeaderDecodingError(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Incomplete header block.")));
accumulator_.EndHeaderBlock();
}
// This payload is invalid because it refers to a non-existing static entry.
TEST_F(QpackDecodedHeadersAccumulatorTest, InvalidPayload) {
EXPECT_CALL(visitor_,
- OnHeaderDecodingError(Eq("Static table entry not found.")));
+ OnHeaderDecodingError(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Static table entry not found.")));
accumulator_.Decode(absl::HexStringToBytes("0000ff23ff24"));
}
@@ -241,7 +244,8 @@
qpack_decoder_.OnSetDynamicTableCapacity(kMaxDynamicTableCapacity);
// Adding dynamic table entry unblocks decoding. Error is detected.
- EXPECT_CALL(visitor_, OnHeaderDecodingError(Eq("Invalid relative index.")));
+ EXPECT_CALL(visitor_, OnHeaderDecodingError(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Invalid relative index.")));
qpack_decoder_.OnInsertWithoutNameReference("foo", "bar");
}
diff --git a/quic/core/qpack/qpack_decoder_test.cc b/quic/core/qpack/qpack_decoder_test.cc
index 772b1d2..8f4b092 100644
--- a/quic/core/qpack/qpack_decoder_test.cc
+++ b/quic/core/qpack/qpack_decoder_test.cc
@@ -49,8 +49,9 @@
void SetUp() override {
// Destroy QpackProgressiveDecoder on error to test that it does not crash.
// See https://crbug.com/1025209.
- ON_CALL(handler_, OnDecodingErrorDetected(_))
- .WillByDefault(Invoke([this](absl::string_view /* error_message */) {
+ ON_CALL(handler_, OnDecodingErrorDetected(_, _))
+ .WillByDefault(Invoke([this](QuicErrorCode /* error_code */,
+ absl::string_view /* error_message */) {
progressive_decoder_.reset();
}));
}
@@ -114,7 +115,8 @@
TEST_P(QpackDecoderTest, NoPrefix) {
EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("Incomplete header data prefix.")));
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Incomplete header data prefix.")));
// Header Data Prefix is at least two bytes long.
DecodeHeaderBlock(absl::HexStringToBytes("00"));
@@ -126,7 +128,8 @@
StartDecoding();
EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("Encoded integer too large.")));
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Encoded integer too large.")));
// Encoded Required Insert Count in Header Data Prefix is too large.
DecodeData(absl::HexStringToBytes("ffffffffffffffffffffffffffff"));
@@ -188,7 +191,8 @@
// Name Length value is too large for varint decoder to decode.
TEST_P(QpackDecoderTest, NameLenTooLargeForVarintDecoder) {
EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("Encoded integer too large.")));
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Encoded integer too large.")));
DecodeHeaderBlock(absl::HexStringToBytes("000027ffffffffffffffffffff"));
}
@@ -196,7 +200,8 @@
// Name Length value can be decoded by varint decoder but exceeds 1 MB limit.
TEST_P(QpackDecoderTest, NameLenExceedsLimit) {
EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("String literal too long.")));
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("String literal too long.")));
DecodeHeaderBlock(absl::HexStringToBytes("000027ffff7f"));
}
@@ -204,7 +209,8 @@
// Value Length value is too large for varint decoder to decode.
TEST_P(QpackDecoderTest, ValueLenTooLargeForVarintDecoder) {
EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("Encoded integer too large.")));
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Encoded integer too large.")));
DecodeHeaderBlock(
absl::HexStringToBytes("000023666f6f7fffffffffffffffffffff"));
@@ -213,14 +219,29 @@
// Value Length value can be decoded by varint decoder but exceeds 1 MB limit.
TEST_P(QpackDecoderTest, ValueLenExceedsLimit) {
EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("String literal too long.")));
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("String literal too long.")));
DecodeHeaderBlock(absl::HexStringToBytes("000023666f6f7fffff7f"));
}
+TEST_P(QpackDecoderTest, LineFeedInValue) {
+ if (GetQuicReloadableFlag(quic_reject_invalid_chars_in_field_value)) {
+ EXPECT_CALL(handler_,
+ OnDecodingErrorDetected(QUIC_INVALID_CHARACTER_IN_FIELD_VALUE,
+ "Invalid character in field value."));
+ } else {
+ EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("ba\nr")));
+ EXPECT_CALL(handler_, OnDecodingCompleted());
+ }
+
+ DecodeHeaderBlock(absl::HexStringToBytes("000023666f6f0462610a72"));
+}
+
TEST_P(QpackDecoderTest, IncompleteHeaderBlock) {
EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("Incomplete header block.")));
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Incomplete header block.")));
DecodeHeaderBlock(absl::HexStringToBytes("00002366"));
}
@@ -251,8 +272,9 @@
}
TEST_P(QpackDecoderTest, HuffmanNameDoesNotHaveEOSPrefix) {
- EXPECT_CALL(handler_, OnDecodingErrorDetected(absl::string_view(
- "Error in Huffman-encoded string.")));
+ EXPECT_CALL(handler_,
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Error in Huffman-encoded string.")));
// 'y' ends in 0b0 on the most significant bit of the last byte.
// The remaining 7 bits must be a prefix of EOS, which is all 1s.
@@ -261,8 +283,9 @@
}
TEST_P(QpackDecoderTest, HuffmanValueDoesNotHaveEOSPrefix) {
- EXPECT_CALL(handler_, OnDecodingErrorDetected(absl::string_view(
- "Error in Huffman-encoded string.")));
+ EXPECT_CALL(handler_,
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Error in Huffman-encoded string.")));
// 'e' ends in 0b101, taking up the 3 most significant bits of the last byte.
// The remaining 5 bits must be a prefix of EOS, which is all 1s.
@@ -271,8 +294,9 @@
}
TEST_P(QpackDecoderTest, HuffmanNameEOSPrefixTooLong) {
- EXPECT_CALL(handler_, OnDecodingErrorDetected(absl::string_view(
- "Error in Huffman-encoded string.")));
+ EXPECT_CALL(handler_,
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Error in Huffman-encoded string.")));
// The trailing EOS prefix must be at most 7 bits long. Appending one octet
// with value 0xff is invalid, even though 0b111111111111111 (15 bits) is a
@@ -282,8 +306,9 @@
}
TEST_P(QpackDecoderTest, HuffmanValueEOSPrefixTooLong) {
- EXPECT_CALL(handler_, OnDecodingErrorDetected(absl::string_view(
- "Error in Huffman-encoded string.")));
+ EXPECT_CALL(handler_,
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Error in Huffman-encoded string.")));
// The trailing EOS prefix must be at most 7 bits long. Appending one octet
// with value 0xff is invalid, even though 0b1111111111111 (13 bits) is a
@@ -321,7 +346,8 @@
// Addressing entry 99 should trigger an error.
EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("Static table entry not found.")));
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Static table entry not found.")));
DecodeHeaderBlock(absl::HexStringToBytes("0000ff23ff24"));
}
@@ -430,6 +456,7 @@
DecodeEncoderStreamData(absl::HexStringToBytes("3f01"));
EXPECT_CALL(handler_, OnDecodingErrorDetected(
+ QUIC_QPACK_DECOMPRESSION_FAILED,
Eq("Dynamic table entry already evicted.")));
DecodeHeaderBlock(absl::HexStringToBytes(
@@ -497,7 +524,8 @@
}
TEST_P(QpackDecoderTest, InvalidDynamicEntryWhenBaseIsZero) {
- EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq("Invalid relative index.")));
+ EXPECT_CALL(handler_, OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Invalid relative index.")));
// Set dynamic table capacity to 1024.
DecodeEncoderStreamData(absl::HexStringToBytes("3fe107"));
@@ -512,7 +540,8 @@
}
TEST_P(QpackDecoderTest, InvalidNegativeBase) {
- EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq("Error calculating Base.")));
+ EXPECT_CALL(handler_, OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Error calculating Base.")));
// Required Insert Count 1, Delta Base 1 with sign bit set, Base would
// be 1 - 1 - 1 = -1, but it is not allowed to be negative.
@@ -525,7 +554,8 @@
// Add literal entry with name "foo" and value "bar".
DecodeEncoderStreamData(absl::HexStringToBytes("6294e703626172"));
- EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq("Invalid relative index.")));
+ EXPECT_CALL(handler_, OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Invalid relative index.")));
DecodeHeaderBlock(absl::HexStringToBytes(
"0200" // Required Insert Count 1 and Delta Base 0.
@@ -533,7 +563,8 @@
"81")); // Indexed Header Field instruction addressing relative index 1.
// This is absolute index -1, which is invalid.
- EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq("Invalid relative index.")));
+ EXPECT_CALL(handler_, OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Invalid relative index.")));
DecodeHeaderBlock(absl::HexStringToBytes(
"0200" // Required Insert Count 1 and Delta Base 0.
@@ -554,6 +585,7 @@
DecodeEncoderStreamData(absl::HexStringToBytes("00000000"));
EXPECT_CALL(handler_, OnDecodingErrorDetected(
+ QUIC_QPACK_DECOMPRESSION_FAILED,
Eq("Dynamic table entry already evicted.")));
DecodeHeaderBlock(absl::HexStringToBytes(
@@ -563,6 +595,7 @@
// This is absolute index 1. Such entry does not exist.
EXPECT_CALL(handler_, OnDecodingErrorDetected(
+ QUIC_QPACK_DECOMPRESSION_FAILED,
Eq("Dynamic table entry already evicted.")));
DecodeHeaderBlock(absl::HexStringToBytes(
@@ -573,6 +606,7 @@
// entry does not exist.
EXPECT_CALL(handler_, OnDecodingErrorDetected(
+ QUIC_QPACK_DECOMPRESSION_FAILED,
Eq("Dynamic table entry already evicted.")));
DecodeHeaderBlock(absl::HexStringToBytes(
@@ -583,6 +617,7 @@
// does not exist.
EXPECT_CALL(handler_, OnDecodingErrorDetected(
+ QUIC_QPACK_DECOMPRESSION_FAILED,
Eq("Dynamic table entry already evicted.")));
DecodeHeaderBlock(absl::HexStringToBytes(
@@ -614,6 +649,7 @@
// Required Insert Count is decoded modulo 2 * MaxEntries, that is, modulo 64.
// A value of 1 cannot be encoded as 65 even though it has the same remainder.
EXPECT_CALL(handler_, OnDecodingErrorDetected(
+ QUIC_QPACK_DECOMPRESSION_FAILED,
Eq("Error decoding Required Insert Count.")));
DecodeHeaderBlock(absl::HexStringToBytes("4100"));
}
@@ -622,6 +658,7 @@
// after a Header Block Prefix with an invalid Encoded Required Insert Count.
TEST_P(QpackDecoderTest, DataAfterInvalidEncodedRequiredInsertCount) {
EXPECT_CALL(handler_, OnDecodingErrorDetected(
+ QUIC_QPACK_DECOMPRESSION_FAILED,
Eq("Error decoding Required Insert Count.")));
// Header Block Prefix followed by some extra data.
DecodeHeaderBlock(absl::HexStringToBytes("410000"));
@@ -666,7 +703,8 @@
EXPECT_CALL(handler_, OnHeaderDecoded(Eq(":method"), Eq("GET")));
EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("Required Insert Count too large.")));
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Required Insert Count too large.")));
DecodeHeaderBlock(absl::HexStringToBytes(
"0200" // Required Insert Count is 1.
@@ -682,6 +720,7 @@
EXPECT_CALL(
handler_,
OnDecodingErrorDetected(
+ QUIC_QPACK_DECOMPRESSION_FAILED,
Eq("Absolute Index must be smaller than Required Insert Count.")));
DecodeHeaderBlock(absl::HexStringToBytes(
@@ -694,6 +733,7 @@
EXPECT_CALL(
handler_,
OnDecodingErrorDetected(
+ QUIC_QPACK_DECOMPRESSION_FAILED,
Eq("Absolute Index must be smaller than Required Insert Count.")));
DecodeHeaderBlock(absl::HexStringToBytes(
@@ -707,6 +747,7 @@
EXPECT_CALL(
handler_,
OnDecodingErrorDetected(
+ QUIC_QPACK_DECOMPRESSION_FAILED,
Eq("Absolute Index must be smaller than Required Insert Count.")));
DecodeHeaderBlock(absl::HexStringToBytes(
@@ -720,6 +761,7 @@
EXPECT_CALL(
handler_,
OnDecodingErrorDetected(
+ QUIC_QPACK_DECOMPRESSION_FAILED,
Eq("Absolute Index must be smaller than Required Insert Count.")));
DecodeHeaderBlock(absl::HexStringToBytes(
@@ -743,7 +785,8 @@
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar")));
EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("Required Insert Count too large.")));
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Required Insert Count too large.")));
DecodeHeaderBlock(absl::HexStringToBytes(
"0300" // Required Insert Count 2 and Delta Base 0.
@@ -755,7 +798,8 @@
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("")));
EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("Required Insert Count too large.")));
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Required Insert Count too large.")));
DecodeHeaderBlock(absl::HexStringToBytes(
"0300" // Required Insert Count 2 and Delta Base 0.
@@ -767,7 +811,8 @@
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar")));
EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("Required Insert Count too large.")));
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Required Insert Count too large.")));
DecodeHeaderBlock(absl::HexStringToBytes(
"0481" // Required Insert Count 3 and Delta Base 1 with sign bit set.
@@ -779,7 +824,8 @@
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("")));
EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("Required Insert Count too large.")));
+ OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Required Insert Count too large.")));
DecodeHeaderBlock(absl::HexStringToBytes(
"0481" // Required Insert Count 3 and Delta Base 1 with sign bit set.
@@ -864,7 +910,8 @@
// Count of the header block. |handler_| methods are called immediately for
// the already consumed part of the header block.
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar")));
- EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq("Invalid relative index.")));
+ EXPECT_CALL(handler_, OnDecodingErrorDetected(QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Invalid relative index.")));
DecodeEncoderStreamData(absl::HexStringToBytes("6294e703626172"));
}
@@ -905,8 +952,10 @@
auto progressive_decoder1 = CreateProgressiveDecoder(/* stream_id = */ 1);
progressive_decoder1->Decode(data);
- EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq(
- "Limit on number of blocked streams exceeded.")));
+ EXPECT_CALL(handler_,
+ OnDecodingErrorDetected(
+ QUIC_QPACK_DECOMPRESSION_FAILED,
+ Eq("Limit on number of blocked streams exceeded.")));
auto progressive_decoder2 = CreateProgressiveDecoder(/* stream_id = */ 2);
progressive_decoder2->Decode(data);
diff --git a/quic/core/qpack/qpack_progressive_decoder.cc b/quic/core/qpack/qpack_progressive_decoder.cc
index 3e7d0ad..96a3da5 100644
--- a/quic/core/qpack/qpack_progressive_decoder.cc
+++ b/quic/core/qpack/qpack_progressive_decoder.cc
@@ -12,20 +12,27 @@
#include "quic/core/qpack/qpack_index_conversions.h"
#include "quic/core/qpack/qpack_instructions.h"
#include "quic/core/qpack/qpack_required_insert_count.h"
+#include "quic/platform/api/quic_flag_utils.h"
+#include "quic/platform/api/quic_flags.h"
#include "quic/platform/api/quic_logging.h"
namespace quic {
+namespace {
+
+// The value argument passed to OnHeaderDecoded() is from an entry in the static
+// table.
+constexpr bool kValueFromStaticTable = true;
+
+} // anonymous namespace
+
QpackProgressiveDecoder::QpackProgressiveDecoder(
- QuicStreamId stream_id,
- BlockedStreamLimitEnforcer* enforcer,
- DecodingCompletedVisitor* visitor,
- QpackDecoderHeaderTable* header_table,
+ QuicStreamId stream_id, BlockedStreamLimitEnforcer* enforcer,
+ DecodingCompletedVisitor* visitor, QpackDecoderHeaderTable* header_table,
HeadersHandlerInterface* handler)
: stream_id_(stream_id),
- prefix_decoder_(
- std::make_unique<QpackInstructionDecoder>(QpackPrefixLanguage(),
- this)),
+ prefix_decoder_(std::make_unique<QpackInstructionDecoder>(
+ QpackPrefixLanguage(), this)),
instruction_decoder_(QpackRequestStreamLanguage(), this),
enforcer_(enforcer),
visitor_(visitor),
@@ -38,7 +45,13 @@
blocked_(false),
decoding_(true),
error_detected_(false),
- cancelled_(false) {}
+ cancelled_(false),
+ reject_invalid_chars_in_field_value_(
+ GetQuicReloadableFlag(quic_reject_invalid_chars_in_field_value)) {
+ if (reject_invalid_chars_in_field_value_) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_reject_invalid_chars_in_field_value);
+ }
+}
QpackProgressiveDecoder::~QpackProgressiveDecoder() {
if (blocked_ && !cancelled_) {
@@ -89,14 +102,6 @@
}
}
-void QpackProgressiveDecoder::OnError(absl::string_view error_message) {
- QUICHE_DCHECK(!error_detected_);
-
- error_detected_ = true;
- // Might destroy |this|.
- handler_->OnDecodingErrorDetected(error_message);
-}
-
bool QpackProgressiveDecoder::OnInstructionDecoded(
const QpackInstruction* instruction) {
if (instruction == QpackPrefixInstruction()) {
@@ -126,10 +131,9 @@
void QpackProgressiveDecoder::OnInstructionDecodingError(
QpackInstructionDecoder::ErrorCode /* error_code */,
absl::string_view error_message) {
- // Ignore |error_code|, because header block decoding errors trigger a
- // RESET_STREAM frame which cannot carry an error code more granular than
- // QPACK_DECOMPRESSION_FAILED.
- OnError(error_message);
+ // Ignore |error_code| and always use QUIC_QPACK_DECOMPRESSION_FAILED to avoid
+ // having to define a new QuicErrorCode for every instruction decoder error.
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED, error_message);
}
void QpackProgressiveDecoder::OnInsertCountReachedThreshold() {
@@ -164,12 +168,13 @@
uint64_t absolute_index;
if (!QpackRequestStreamRelativeIndexToAbsoluteIndex(
instruction_decoder_.varint(), base_, &absolute_index)) {
- OnError("Invalid relative index.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED, "Invalid relative index.");
return false;
}
if (absolute_index >= required_insert_count_) {
- OnError("Absolute Index must be smaller than Required Insert Count.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED,
+ "Absolute Index must be smaller than Required Insert Count.");
return false;
}
@@ -180,36 +185,37 @@
auto entry =
header_table_->LookupEntry(/* is_static = */ false, absolute_index);
if (!entry) {
- OnError("Dynamic table entry already evicted.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED,
+ "Dynamic table entry already evicted.");
return false;
}
header_table_->set_dynamic_table_entry_referenced();
- handler_->OnHeaderDecoded(entry->name(), entry->value());
- return true;
+ return OnHeaderDecoded(!kValueFromStaticTable, entry->name(),
+ entry->value());
}
auto entry = header_table_->LookupEntry(/* is_static = */ true,
instruction_decoder_.varint());
if (!entry) {
- OnError("Static table entry not found.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED, "Static table entry not found.");
return false;
}
- handler_->OnHeaderDecoded(entry->name(), entry->value());
- return true;
+ return OnHeaderDecoded(kValueFromStaticTable, entry->name(), entry->value());
}
bool QpackProgressiveDecoder::DoIndexedHeaderFieldPostBaseInstruction() {
uint64_t absolute_index;
if (!QpackPostBaseIndexToAbsoluteIndex(instruction_decoder_.varint(), base_,
&absolute_index)) {
- OnError("Invalid post-base index.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED, "Invalid post-base index.");
return false;
}
if (absolute_index >= required_insert_count_) {
- OnError("Absolute Index must be smaller than Required Insert Count.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED,
+ "Absolute Index must be smaller than Required Insert Count.");
return false;
}
@@ -220,13 +226,13 @@
auto entry =
header_table_->LookupEntry(/* is_static = */ false, absolute_index);
if (!entry) {
- OnError("Dynamic table entry already evicted.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED,
+ "Dynamic table entry already evicted.");
return false;
}
header_table_->set_dynamic_table_entry_referenced();
- handler_->OnHeaderDecoded(entry->name(), entry->value());
- return true;
+ return OnHeaderDecoded(!kValueFromStaticTable, entry->name(), entry->value());
}
bool QpackProgressiveDecoder::DoLiteralHeaderFieldNameReferenceInstruction() {
@@ -234,12 +240,13 @@
uint64_t absolute_index;
if (!QpackRequestStreamRelativeIndexToAbsoluteIndex(
instruction_decoder_.varint(), base_, &absolute_index)) {
- OnError("Invalid relative index.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED, "Invalid relative index.");
return false;
}
if (absolute_index >= required_insert_count_) {
- OnError("Absolute Index must be smaller than Required Insert Count.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED,
+ "Absolute Index must be smaller than Required Insert Count.");
return false;
}
@@ -250,36 +257,38 @@
auto entry =
header_table_->LookupEntry(/* is_static = */ false, absolute_index);
if (!entry) {
- OnError("Dynamic table entry already evicted.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED,
+ "Dynamic table entry already evicted.");
return false;
}
header_table_->set_dynamic_table_entry_referenced();
- handler_->OnHeaderDecoded(entry->name(), instruction_decoder_.value());
- return true;
+ return OnHeaderDecoded(!kValueFromStaticTable, entry->name(),
+ instruction_decoder_.value());
}
auto entry = header_table_->LookupEntry(/* is_static = */ true,
instruction_decoder_.varint());
if (!entry) {
- OnError("Static table entry not found.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED, "Static table entry not found.");
return false;
}
- handler_->OnHeaderDecoded(entry->name(), instruction_decoder_.value());
- return true;
+ return OnHeaderDecoded(kValueFromStaticTable, entry->name(),
+ instruction_decoder_.value());
}
bool QpackProgressiveDecoder::DoLiteralHeaderFieldPostBaseInstruction() {
uint64_t absolute_index;
if (!QpackPostBaseIndexToAbsoluteIndex(instruction_decoder_.varint(), base_,
&absolute_index)) {
- OnError("Invalid post-base index.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED, "Invalid post-base index.");
return false;
}
if (absolute_index >= required_insert_count_) {
- OnError("Absolute Index must be smaller than Required Insert Count.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED,
+ "Absolute Index must be smaller than Required Insert Count.");
return false;
}
@@ -290,20 +299,19 @@
auto entry =
header_table_->LookupEntry(/* is_static = */ false, absolute_index);
if (!entry) {
- OnError("Dynamic table entry already evicted.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED,
+ "Dynamic table entry already evicted.");
return false;
}
header_table_->set_dynamic_table_entry_referenced();
- handler_->OnHeaderDecoded(entry->name(), instruction_decoder_.value());
- return true;
+ return OnHeaderDecoded(!kValueFromStaticTable, entry->name(),
+ instruction_decoder_.value());
}
bool QpackProgressiveDecoder::DoLiteralHeaderFieldInstruction() {
- handler_->OnHeaderDecoded(instruction_decoder_.name(),
- instruction_decoder_.value());
-
- return true;
+ return OnHeaderDecoded(!kValueFromStaticTable, instruction_decoder_.name(),
+ instruction_decoder_.value());
}
bool QpackProgressiveDecoder::DoPrefixInstruction() {
@@ -312,14 +320,15 @@
if (!QpackDecodeRequiredInsertCount(
prefix_decoder_->varint(), header_table_->max_entries(),
header_table_->inserted_entry_count(), &required_insert_count_)) {
- OnError("Error decoding Required Insert Count.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED,
+ "Error decoding Required Insert Count.");
return false;
}
const bool sign = prefix_decoder_->s_bit();
const uint64_t delta_base = prefix_decoder_->varint2();
if (!DeltaBaseToBase(sign, delta_base, &base_)) {
- OnError("Error calculating Base.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED, "Error calculating Base.");
return false;
}
@@ -327,7 +336,8 @@
if (required_insert_count_ > header_table_->inserted_entry_count()) {
if (!enforcer_->OnStreamBlocked(stream_id_)) {
- OnError("Limit on number of blocked streams exceeded.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED,
+ "Limit on number of blocked streams exceeded.");
return false;
}
blocked_ = true;
@@ -337,6 +347,32 @@
return true;
}
+bool QpackProgressiveDecoder::OnHeaderDecoded(bool value_from_static_table,
+ absl::string_view name,
+ absl::string_view value) {
+ // Skip test for static table entries as they are all known to be valid.
+ if (reject_invalid_chars_in_field_value_ && !value_from_static_table) {
+ // According to Section 10.3 of
+ // https://quicwg.org/base-drafts/draft-ietf-quic-http.html,
+ // "[...] HTTP/3 can transport field values that are not valid. While most
+ // values that can be encoded will not alter field parsing, carriage return
+ // (CR, ASCII 0x0d), line feed (LF, ASCII 0x0a), and the zero character
+ // (NUL, ASCII 0x00) might be exploited by an attacker if they are
+ // translated verbatim. Any request or response that contains a character
+ // not permitted in a field value MUST be treated as malformed [...]"
+ for (const auto c : value) {
+ if (c == '\0' || c == '\n' || c == '\r') {
+ OnError(QUIC_INVALID_CHARACTER_IN_FIELD_VALUE,
+ "Invalid character in field value.");
+ return false;
+ }
+ }
+ }
+
+ handler_->OnHeaderDecoded(name, value);
+ return true;
+}
+
void QpackProgressiveDecoder::FinishDecoding() {
QUICHE_DCHECK(buffer_.empty());
QUICHE_DCHECK(!blocked_);
@@ -347,17 +383,18 @@
}
if (!instruction_decoder_.AtInstructionBoundary()) {
- OnError("Incomplete header block.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED, "Incomplete header block.");
return;
}
if (!prefix_decoded_) {
- OnError("Incomplete header data prefix.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED, "Incomplete header data prefix.");
return;
}
if (required_insert_count_ != required_insert_count_so_far_) {
- OnError("Required Insert Count too large.");
+ OnError(QUIC_QPACK_DECOMPRESSION_FAILED,
+ "Required Insert Count too large.");
return;
}
@@ -365,6 +402,15 @@
handler_->OnDecodingCompleted();
}
+void QpackProgressiveDecoder::OnError(QuicErrorCode error_code,
+ absl::string_view error_message) {
+ QUICHE_DCHECK(!error_detected_);
+
+ error_detected_ = true;
+ // Might destroy |this|.
+ handler_->OnDecodingErrorDetected(error_code, error_message);
+}
+
bool QpackProgressiveDecoder::DeltaBaseToBase(bool sign,
uint64_t delta_base,
uint64_t* base) {
diff --git a/quic/core/qpack/qpack_progressive_decoder.h b/quic/core/qpack/qpack_progressive_decoder.h
index ced6bf0..b077ab1 100644
--- a/quic/core/qpack/qpack_progressive_decoder.h
+++ b/quic/core/qpack/qpack_progressive_decoder.h
@@ -13,6 +13,7 @@
#include "quic/core/qpack/qpack_encoder_stream_receiver.h"
#include "quic/core/qpack/qpack_header_table.h"
#include "quic/core/qpack/qpack_instruction_decoder.h"
+#include "quic/core/quic_error_codes.h"
#include "quic/core/quic_types.h"
#include "quic/platform/api/quic_export.h"
@@ -46,7 +47,8 @@
// Called when a decoding error has occurred. No other methods will be
// called afterwards. Implementations are allowed to destroy
// the QpackProgressiveDecoder instance synchronously.
- virtual void OnDecodingErrorDetected(absl::string_view error_message) = 0;
+ virtual void OnDecodingErrorDetected(QuicErrorCode error_code,
+ absl::string_view error_message) = 0;
};
// Interface for keeping track of blocked streams for the purpose of enforcing
@@ -95,9 +97,6 @@
// through Decode(). No methods must be called afterwards.
void EndHeaderBlock();
- // Called on error.
- void OnError(absl::string_view error_message);
-
// QpackInstructionDecoder::Delegate implementation.
bool OnInstructionDecoded(const QpackInstruction* instruction) override;
void OnInstructionDecodingError(QpackInstructionDecoder::ErrorCode error_code,
@@ -115,9 +114,20 @@
bool DoLiteralHeaderFieldInstruction();
bool DoPrefixInstruction();
+ // Called when an entry is decoded. Performs validation and calls
+ // HeadersHandlerInterface::OnHeaderDecoded() or OnError() as needed. Returns
+ // true if header value is valid, false otherwise. Skips validation if
+ // |value_from_static_table| is true, because static table entries are always
+ // valid.
+ bool OnHeaderDecoded(bool value_from_static_table, absl::string_view name,
+ absl::string_view value);
+
// Called as soon as EndHeaderBlock() is called and decoding is not blocked.
void FinishDecoding();
+ // Called on error.
+ void OnError(QuicErrorCode error_code, absl::string_view error_message);
+
// Calculates Base from |required_insert_count_|, which must be set before
// calling this method, and sign bit and Delta Base in the Header Data Prefix,
// which are passed in as arguments. Returns true on success, false on
@@ -166,6 +176,10 @@
// True if QpackDecoderHeaderTable has been destroyed
// while decoding is still blocked.
bool cancelled_;
+
+ // Latched value of
+ // gfe2_reloadable_flag_quic_reject_invalid_chars_in_field_value
+ const bool reject_invalid_chars_in_field_value_;
};
} // namespace quic
diff --git a/quic/core/quic_error_codes.cc b/quic/core/quic_error_codes.cc
index afd0bfb..162cb04 100644
--- a/quic/core/quic_error_codes.cc
+++ b/quic/core/quic_error_codes.cc
@@ -275,6 +275,8 @@
RETURN_STRING_LITERAL(QUIC_TLS_UNRECOGNIZED_NAME);
RETURN_STRING_LITERAL(QUIC_TLS_CERTIFICATE_REQUIRED);
+ RETURN_STRING_LITERAL(QUIC_INVALID_CHARACTER_IN_FIELD_VALUE);
+
RETURN_STRING_LITERAL(QUIC_LAST_ERROR);
// Intentionally have no default case, so we'll break the build
// if we add errors and don't put them here.
@@ -772,6 +774,8 @@
return {true, static_cast<uint64_t>(CONNECTION_ID_LIMIT_ERROR)};
case QUIC_TOO_MANY_CONNECTION_ID_WAITING_TO_RETIRE:
return {true, static_cast<uint64_t>(INTERNAL_ERROR)};
+ case QUIC_INVALID_CHARACTER_IN_FIELD_VALUE:
+ return {false, static_cast<uint64_t>(QuicHttp3ErrorCode::MESSAGE_ERROR)};
case QUIC_LAST_ERROR:
return {false, static_cast<uint64_t>(QUIC_LAST_ERROR)};
}
diff --git a/quic/core/quic_error_codes.h b/quic/core/quic_error_codes.h
index f43ad0a..860f107 100644
--- a/quic/core/quic_error_codes.h
+++ b/quic/core/quic_error_codes.h
@@ -599,8 +599,11 @@
QUIC_TLS_UNRECOGNIZED_NAME = 201,
QUIC_TLS_CERTIFICATE_REQUIRED = 202,
+ // An HTTP field value containing an invalid character has been received.
+ QUIC_INVALID_CHARACTER_IN_FIELD_VALUE = 206,
+
// No error. Used as bound while iterating.
- QUIC_LAST_ERROR = 206,
+ QUIC_LAST_ERROR = 207,
};
// QuicErrorCodes is encoded as four octets on-the-wire when doing Google QUIC,
// or a varint62 when doing IETF QUIC. Ensure that its value does not exceed
@@ -716,6 +719,7 @@
REQUEST_REJECTED = 0x10B,
REQUEST_CANCELLED = 0x10C,
REQUEST_INCOMPLETE = 0x10D,
+ MESSAGE_ERROR = 0x10E,
CONNECT_ERROR = 0x10F,
VERSION_FALLBACK = 0x110,
};
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index db08117..125cbc0 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -21,6 +21,8 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_bbr2_check_cwnd_limited_before_aggregation_epoch, true)
// If true, GFE will explicitly configure its signature algorithm preference.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_tls_set_signature_algorithm_prefs, true)
+// If true, QPACK decoder rejects CR, LF, and NULL in field (header) values, and causes the stream to be reset with H3_MESSAGE_ERROR.
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_reject_invalid_chars_in_field_value, true)
// If true, QUIC will default enable MTU discovery at server, with a target of 1450 bytes.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_enable_mtu_discovery_at_server, false)
// If true, QuicAlarms that belong to a single QuicConnection will fire under the corresponding QuicConnectionContext.
diff --git a/quic/test_tools/qpack/qpack_decoder_test_utils.cc b/quic/test_tools/qpack/qpack_decoder_test_utils.cc
index ab76c44..37f182a 100644
--- a/quic/test_tools/qpack/qpack_decoder_test_utils.cc
+++ b/quic/test_tools/qpack/qpack_decoder_test_utils.cc
@@ -37,7 +37,7 @@
}
void TestHeadersHandler::OnDecodingErrorDetected(
- absl::string_view error_message) {
+ QuicErrorCode /*error_code*/, absl::string_view error_message) {
ASSERT_FALSE(decoding_completed_);
ASSERT_FALSE(decoding_error_detected_);
diff --git a/quic/test_tools/qpack/qpack_decoder_test_utils.h b/quic/test_tools/qpack/qpack_decoder_test_utils.h
index 2220bd4..b6bff78 100644
--- a/quic/test_tools/qpack/qpack_decoder_test_utils.h
+++ b/quic/test_tools/qpack/qpack_decoder_test_utils.h
@@ -10,6 +10,7 @@
#include "absl/strings/string_view.h"
#include "quic/core/qpack/qpack_decoder.h"
#include "quic/core/qpack/qpack_progressive_decoder.h"
+#include "quic/core/quic_error_codes.h"
#include "quic/platform/api/quic_test.h"
#include "quic/test_tools/qpack/qpack_test_utils.h"
#include "spdy/core/spdy_header_block.h"
@@ -51,7 +52,8 @@
void OnHeaderDecoded(absl::string_view name,
absl::string_view value) override;
void OnDecodingCompleted() override;
- void OnDecodingErrorDetected(absl::string_view error_message) override;
+ void OnDecodingErrorDetected(QuicErrorCode error_code,
+ absl::string_view error_message) override;
// Release decoded header list. Must only be called if decoding is complete
// and no errors have been detected.
@@ -81,9 +83,8 @@
(absl::string_view name, absl::string_view value),
(override));
MOCK_METHOD(void, OnDecodingCompleted, (), (override));
- MOCK_METHOD(void,
- OnDecodingErrorDetected,
- (absl::string_view error_message),
+ MOCK_METHOD(void, OnDecodingErrorDetected,
+ (QuicErrorCode error_code, absl::string_view error_message),
(override));
};
@@ -95,7 +96,8 @@
void OnHeaderDecoded(absl::string_view /*name*/,
absl::string_view /*value*/) override {}
void OnDecodingCompleted() override {}
- void OnDecodingErrorDetected(absl::string_view /*error_message*/) override {}
+ void OnDecodingErrorDetected(QuicErrorCode /*error_code*/,
+ absl::string_view /*error_message*/) override {}
};
void QpackDecode(
