Surface hpack decoder detailed error for header value too long, and put detailed error to quic connection close error details. only change connection close error detail, not protected. PiperOrigin-RevId: 323008871 Change-Id: I6f41118464ae686fbfeb009c8ac13d83b8fff3f0
diff --git a/http2/hpack/decoder/hpack_decoder.cc b/http2/hpack/decoder/hpack_decoder.cc index b879511..6cfd1e0 100644 --- a/http2/hpack/decoder/hpack_decoder.cc +++ b/http2/hpack/decoder/hpack_decoder.cc
@@ -60,7 +60,7 @@ // which finally forwards them to the HpackDecoderListener. DecodeStatus status = block_decoder_.Decode(db); if (status == DecodeStatus::kDecodeError) { - ReportError(block_decoder_.error()); + ReportError(block_decoder_.error(), ""); HTTP2_CODE_COUNT_N(decompress_failure_3, 4, 23); return false; } else if (DetectError()) { @@ -85,7 +85,7 @@ } if (!block_decoder_.before_entry()) { // The HPACK block ended in the middle of an entry. - ReportError(HpackDecodingError::kTruncatedBlock); + ReportError(HpackDecodingError::kTruncatedBlock, ""); HTTP2_CODE_COUNT_N(decompress_failure_3, 7, 23); return false; } @@ -107,6 +107,7 @@ HTTP2_DVLOG(2) << "Error detected in decoder_state_"; HTTP2_CODE_COUNT_N(decompress_failure_3, 10, 23); error_ = decoder_state_.error(); + detailed_error_ = decoder_state_.detailed_error(); } return error_ != HpackDecodingError::kOk; @@ -116,12 +117,14 @@ return Http2EstimateMemoryUsage(entry_buffer_); } -void HpackDecoder::ReportError(HpackDecodingError error) { +void HpackDecoder::ReportError(HpackDecodingError error, + std::string detailed_error) { HTTP2_DVLOG(3) << "HpackDecoder::ReportError is new=" << (error_ == HpackDecodingError::kOk ? "true" : "false") << ", error: " << HpackDecodingErrorToString(error); if (error_ == HpackDecodingError::kOk) { error_ = error; + detailed_error_ = detailed_error; decoder_state_.listener()->OnHeaderErrorDetected( HpackDecodingErrorToString(error)); }
diff --git a/http2/hpack/decoder/hpack_decoder.h b/http2/hpack/decoder/hpack_decoder.h index efe334d..0666c38 100644 --- a/http2/hpack/decoder/hpack_decoder.h +++ b/http2/hpack/decoder/hpack_decoder.h
@@ -103,11 +103,13 @@ // Returns the estimate of dynamically allocated memory in bytes. size_t EstimateMemoryUsage() const; + std::string detailed_error() const { return detailed_error_; } + private: friend class test::HpackDecoderPeer; // Reports an error to the listener IF this is the first error detected. - void ReportError(HpackDecodingError error); + void ReportError(HpackDecodingError error, std::string detailed_error); // The decompressor state, as defined by HPACK (i.e. the static and dynamic // tables). @@ -121,6 +123,7 @@ // Error code if an error has occurred, HpackDecodingError::kOk otherwise. HpackDecodingError error_; + std::string detailed_error_; }; } // namespace http2
diff --git a/http2/hpack/decoder/hpack_decoder_state.cc b/http2/hpack/decoder/hpack_decoder_state.cc index 09b64c0..eba3e66 100644 --- a/http2/hpack/decoder/hpack_decoder_state.cc +++ b/http2/hpack/decoder/hpack_decoder_state.cc
@@ -85,7 +85,7 @@ return; } if (require_dynamic_table_size_update_) { - ReportError(HpackDecodingError::kMissingDynamicTableSizeUpdate); + ReportError(HpackDecodingError::kMissingDynamicTableSizeUpdate, ""); return; } allow_dynamic_table_size_update_ = false; @@ -93,7 +93,7 @@ if (entry != nullptr) { listener_->OnHeader(entry->name, entry->value); } else { - ReportError(HpackDecodingError::kInvalidIndex); + ReportError(HpackDecodingError::kInvalidIndex, ""); } } @@ -108,7 +108,7 @@ return; } if (require_dynamic_table_size_update_) { - ReportError(HpackDecodingError::kMissingDynamicTableSizeUpdate); + ReportError(HpackDecodingError::kMissingDynamicTableSizeUpdate, ""); return; } allow_dynamic_table_size_update_ = false; @@ -120,7 +120,7 @@ decoder_tables_.Insert(entry->name, value); } } else { - ReportError(HpackDecodingError::kInvalidNameIndex); + ReportError(HpackDecodingError::kInvalidNameIndex, ""); } } @@ -134,7 +134,7 @@ return; } if (require_dynamic_table_size_update_) { - ReportError(HpackDecodingError::kMissingDynamicTableSizeUpdate); + ReportError(HpackDecodingError::kMissingDynamicTableSizeUpdate, ""); return; } allow_dynamic_table_size_update_ = false; @@ -159,14 +159,15 @@ if (!allow_dynamic_table_size_update_) { // At most two dynamic table size updates allowed at the start, and not // after a header. - ReportError(HpackDecodingError::kDynamicTableSizeUpdateNotAllowed); + ReportError(HpackDecodingError::kDynamicTableSizeUpdateNotAllowed, ""); return; } if (require_dynamic_table_size_update_) { // The new size must not be greater than the low water mark. if (size_limit > lowest_header_table_size_) { - ReportError(HpackDecodingError:: - kInitialDynamicTableSizeUpdateIsAboveLowWaterMark); + ReportError( + HpackDecodingError::kInitialDynamicTableSizeUpdateIsAboveLowWaterMark, + ""); return; } require_dynamic_table_size_update_ = false; @@ -174,7 +175,8 @@ // The new size must not be greater than the final max header table size // that the peer acknowledged. ReportError( - HpackDecodingError::kDynamicTableSizeUpdateIsAboveAcknowledgedSetting); + HpackDecodingError::kDynamicTableSizeUpdateIsAboveAcknowledgedSetting, + ""); return; } decoder_tables_.DynamicTableSizeUpdate(size_limit); @@ -187,11 +189,12 @@ lowest_header_table_size_ = final_header_table_size_; } -void HpackDecoderState::OnHpackDecodeError(HpackDecodingError error) { +void HpackDecoderState::OnHpackDecodeError(HpackDecodingError error, + std::string detailed_error) { HTTP2_DVLOG(2) << "HpackDecoderState::OnHpackDecodeError " << HpackDecodingErrorToString(error); if (error_ == HpackDecodingError::kOk) { - ReportError(error); + ReportError(error, detailed_error); } } @@ -203,19 +206,21 @@ if (require_dynamic_table_size_update_) { // Apparently the HPACK block was empty, but we needed it to contain at // least 1 dynamic table size update. - ReportError(HpackDecodingError::kMissingDynamicTableSizeUpdate); + ReportError(HpackDecodingError::kMissingDynamicTableSizeUpdate, ""); } else { listener_->OnHeaderListEnd(); } } -void HpackDecoderState::ReportError(HpackDecodingError error) { +void HpackDecoderState::ReportError(HpackDecodingError error, + std::string detailed_error) { HTTP2_DVLOG(2) << "HpackDecoderState::ReportError is new=" << (error_ == HpackDecodingError::kOk ? "true" : "false") << ", error: " << HpackDecodingErrorToString(error); if (error_ == HpackDecodingError::kOk) { listener_->OnHeaderErrorDetected(HpackDecodingErrorToString(error)); error_ = error; + detailed_error_ = detailed_error; } }
diff --git a/http2/hpack/decoder/hpack_decoder_state.h b/http2/hpack/decoder/hpack_decoder_state.h index 0735f6a..3854a6c 100644 --- a/http2/hpack/decoder/hpack_decoder_state.h +++ b/http2/hpack/decoder/hpack_decoder_state.h
@@ -77,7 +77,8 @@ HpackDecoderStringBuffer* name_buffer, HpackDecoderStringBuffer* value_buffer) override; void OnDynamicTableSizeUpdate(size_t size) override; - void OnHpackDecodeError(HpackDecodingError error) override; + void OnHpackDecodeError(HpackDecodingError error, + std::string detailed_error) override; // OnHeaderBlockEnd notifies this object that an entire HPACK block has been // decoded, which might have extended into CONTINUATION blocks. @@ -91,11 +92,13 @@ return decoder_tables_; } + std::string detailed_error() const { return detailed_error_; } + private: friend class test::HpackDecoderStatePeer; // Reports an error to the listener IF this is the first error detected. - void ReportError(HpackDecodingError error); + void ReportError(HpackDecodingError error, std::string detailed_error); // The static and dynamic HPACK tables. HpackDecoderTables decoder_tables_; @@ -123,6 +126,7 @@ // Has an error already been detected and reported to the listener? HpackDecodingError error_; + std::string detailed_error_; }; } // namespace http2
diff --git a/http2/hpack/decoder/hpack_decoder_state_test.cc b/http2/hpack/decoder/hpack_decoder_state_test.cc index 02abb55..41a8453 100644 --- a/http2/hpack/decoder/hpack_decoder_state_test.cc +++ b/http2/hpack/decoder/hpack_decoder_state_test.cc
@@ -455,7 +455,7 @@ decoder_state_.OnLiteralNameAndValue(HpackEntryType::kIndexedLiteralHeader, &name_buffer_, &value_buffer_); decoder_state_.OnHeaderBlockEnd(); - decoder_state_.OnHpackDecodeError(HpackDecodingError::kIndexVarintError); + decoder_state_.OnHpackDecodeError(HpackDecodingError::kIndexVarintError, ""); } // Confirm that required size updates are validated. @@ -524,7 +524,7 @@ SendStartAndVerifyCallback(); EXPECT_CALL(listener_, OnHeaderErrorDetected(Eq("Name Huffman encoding error"))); - decoder_state_.OnHpackDecodeError(HpackDecodingError::kNameHuffmanError); + decoder_state_.OnHpackDecodeError(HpackDecodingError::kNameHuffmanError, ""); // Further decoded entries are ignored. decoder_state_.OnIndexedHeader(1); @@ -536,7 +536,7 @@ decoder_state_.OnLiteralNameAndValue(HpackEntryType::kIndexedLiteralHeader, &name_buffer_, &value_buffer_); decoder_state_.OnHeaderBlockEnd(); - decoder_state_.OnHpackDecodeError(HpackDecodingError::kIndexVarintError); + decoder_state_.OnHpackDecodeError(HpackDecodingError::kIndexVarintError, ""); } } // namespace
diff --git a/http2/hpack/decoder/hpack_whole_entry_buffer.cc b/http2/hpack/decoder/hpack_whole_entry_buffer.cc index da5613e..dc93d1e 100644 --- a/http2/hpack/decoder/hpack_whole_entry_buffer.cc +++ b/http2/hpack/decoder/hpack_whole_entry_buffer.cc
@@ -9,6 +9,7 @@ #include "net/third_party/quiche/src/http2/platform/api/http2_logging.h" #include "net/third_party/quiche/src/http2/platform/api/http2_macros.h" #include "net/third_party/quiche/src/http2/platform/api/http2_string_utils.h" +#include "net/third_party/quiche/src/common/platform/api/quiche_str_cat.h" namespace http2 { @@ -58,7 +59,7 @@ if (len > max_string_size_bytes_) { HTTP2_DVLOG(1) << "Name length (" << len << ") is longer than permitted (" << max_string_size_bytes_ << ")"; - ReportError(HpackDecodingError::kNameTooLong); + ReportError(HpackDecodingError::kNameTooLong, ""); HTTP2_CODE_COUNT_N(decompress_failure_3, 18, 23); return; } @@ -72,7 +73,7 @@ << Http2HexDump(quiche::QuicheStringPiece(data, len)); DCHECK_EQ(maybe_name_index_, 0u); if (!error_detected_ && !name_.OnData(data, len)) { - ReportError(HpackDecodingError::kNameHuffmanError); + ReportError(HpackDecodingError::kNameHuffmanError, ""); HTTP2_CODE_COUNT_N(decompress_failure_3, 19, 23); } } @@ -81,7 +82,7 @@ HTTP2_DVLOG(2) << "HpackWholeEntryBuffer::OnNameEnd"; DCHECK_EQ(maybe_name_index_, 0u); if (!error_detected_ && !name_.OnEnd()) { - ReportError(HpackDecodingError::kNameHuffmanError); + ReportError(HpackDecodingError::kNameHuffmanError, ""); HTTP2_CODE_COUNT_N(decompress_failure_3, 20, 23); } } @@ -91,10 +92,11 @@ << (huffman_encoded ? "true" : "false") << ", len=" << len; if (!error_detected_) { if (len > max_string_size_bytes_) { - HTTP2_DVLOG(1) << "Value length (" << len - << ") is longer than permitted (" << max_string_size_bytes_ - << ")"; - ReportError(HpackDecodingError::kValueTooLong); + std::string detailed_error = quiche::QuicheStrCat( + "Value length (", len, ") of [", name_.str(), + "] is longer than permitted (", max_string_size_bytes_, ")"); + HTTP2_DVLOG(1) << detailed_error; + ReportError(HpackDecodingError::kValueTooLong, detailed_error); HTTP2_CODE_COUNT_N(decompress_failure_3, 21, 23); return; } @@ -107,7 +109,7 @@ << " data:\n" << Http2HexDump(quiche::QuicheStringPiece(data, len)); if (!error_detected_ && !value_.OnData(data, len)) { - ReportError(HpackDecodingError::kValueHuffmanError); + ReportError(HpackDecodingError::kValueHuffmanError, ""); HTTP2_CODE_COUNT_N(decompress_failure_3, 22, 23); } } @@ -118,7 +120,7 @@ return; } if (!value_.OnEnd()) { - ReportError(HpackDecodingError::kValueHuffmanError); + ReportError(HpackDecodingError::kValueHuffmanError, ""); HTTP2_CODE_COUNT_N(decompress_failure_3, 23, 23); return; } @@ -138,12 +140,13 @@ listener_->OnDynamicTableSizeUpdate(size); } -void HpackWholeEntryBuffer::ReportError(HpackDecodingError error) { +void HpackWholeEntryBuffer::ReportError(HpackDecodingError error, + std::string detailed_error) { if (!error_detected_) { HTTP2_DVLOG(1) << "HpackWholeEntryBuffer::ReportError: " << HpackDecodingErrorToString(error); error_detected_ = true; - listener_->OnHpackDecodeError(error); + listener_->OnHpackDecodeError(error, detailed_error); listener_ = HpackWholeEntryNoOpListener::NoOpListener(); } }
diff --git a/http2/hpack/decoder/hpack_whole_entry_buffer.h b/http2/hpack/decoder/hpack_whole_entry_buffer.h index 5c0bfe2..afca3b8 100644 --- a/http2/hpack/decoder/hpack_whole_entry_buffer.h +++ b/http2/hpack/decoder/hpack_whole_entry_buffer.h
@@ -80,7 +80,7 @@ void OnDynamicTableSizeUpdate(size_t size) override; private: - void ReportError(HpackDecodingError error); + void ReportError(HpackDecodingError error, std::string detailed_error); HpackWholeEntryListener* listener_; HpackDecoderStringBuffer name_, value_;
diff --git a/http2/hpack/decoder/hpack_whole_entry_buffer_test.cc b/http2/hpack/decoder/hpack_whole_entry_buffer_test.cc index a48afd7..2396bd6 100644 --- a/http2/hpack/decoder/hpack_whole_entry_buffer_test.cc +++ b/http2/hpack/decoder/hpack_whole_entry_buffer_test.cc
@@ -11,6 +11,7 @@ #include "testing/gtest/include/gtest/gtest.h" #include "net/third_party/quiche/src/http2/platform/api/http2_test_helpers.h" +using ::testing::_; using ::testing::AllOf; using ::testing::InSequence; using ::testing::Property; @@ -26,17 +27,24 @@ public: ~MockHpackWholeEntryListener() override = default; - MOCK_METHOD1(OnIndexedHeader, void(size_t index)); - MOCK_METHOD3(OnNameIndexAndLiteralValue, - void(HpackEntryType entry_type, - size_t name_index, - HpackDecoderStringBuffer* value_buffer)); - MOCK_METHOD3(OnLiteralNameAndValue, - void(HpackEntryType entry_type, - HpackDecoderStringBuffer* name_buffer, - HpackDecoderStringBuffer* value_buffer)); - MOCK_METHOD1(OnDynamicTableSizeUpdate, void(size_t size)); - MOCK_METHOD1(OnHpackDecodeError, void(HpackDecodingError error)); + MOCK_METHOD(void, OnIndexedHeader, (size_t index), (override)); + MOCK_METHOD(void, + OnNameIndexAndLiteralValue, + (HpackEntryType entry_type, + size_t name_index, + HpackDecoderStringBuffer* value_buffer), + (override)); + MOCK_METHOD(void, + OnLiteralNameAndValue, + (HpackEntryType entry_type, + HpackDecoderStringBuffer* name_buffer, + HpackDecoderStringBuffer* value_buffer), + (override)); + MOCK_METHOD(void, OnDynamicTableSizeUpdate, (size_t size), (override)); + MOCK_METHOD(void, + OnHpackDecodeError, + (HpackDecodingError error, std::string detailed_error), + (override)); }; class HpackWholeEntryBufferTest : public ::testing::Test { @@ -154,14 +162,21 @@ // Verify that a name longer than the allowed size generates an error. TEST_F(HpackWholeEntryBufferTest, NameTooLong) { entry_buffer_.OnStartLiteralHeader(HpackEntryType::kIndexedLiteralHeader, 0); - EXPECT_CALL(listener_, OnHpackDecodeError(HpackDecodingError::kNameTooLong)); + EXPECT_CALL(listener_, + OnHpackDecodeError(HpackDecodingError::kNameTooLong, _)); entry_buffer_.OnNameStart(false, kMaxStringSize + 1); } -// Verify that a name longer than the allowed size generates an error. +// Verify that a value longer than the allowed size generates an error. TEST_F(HpackWholeEntryBufferTest, ValueTooLong) { - entry_buffer_.OnStartLiteralHeader(HpackEntryType::kIndexedLiteralHeader, 1); - EXPECT_CALL(listener_, OnHpackDecodeError(HpackDecodingError::kValueTooLong)); + entry_buffer_.OnStartLiteralHeader(HpackEntryType::kIndexedLiteralHeader, 0); + EXPECT_CALL(listener_, + OnHpackDecodeError( + HpackDecodingError::kValueTooLong, + "Value length (21) of [path] is longer than permitted (20)")); + entry_buffer_.OnNameStart(false, 4); + entry_buffer_.OnNameData("path", 4); + entry_buffer_.OnNameEnd(); entry_buffer_.OnValueStart(false, kMaxStringSize + 1); } @@ -175,7 +190,7 @@ entry_buffer_.OnNameData(data, 3); EXPECT_CALL(listener_, - OnHpackDecodeError(HpackDecodingError::kNameHuffmanError)); + OnHpackDecodeError(HpackDecodingError::kNameHuffmanError, _)); entry_buffer_.OnNameData(data, 1); @@ -194,7 +209,7 @@ entry_buffer_.OnValueData(data, 3); EXPECT_CALL(listener_, - OnHpackDecodeError(HpackDecodingError::kValueHuffmanError)); + OnHpackDecodeError(HpackDecodingError::kValueHuffmanError, _)); entry_buffer_.OnValueEnd();
diff --git a/http2/hpack/decoder/hpack_whole_entry_listener.cc b/http2/hpack/decoder/hpack_whole_entry_listener.cc index 02f3a7a..5900b6d 100644 --- a/http2/hpack/decoder/hpack_whole_entry_listener.cc +++ b/http2/hpack/decoder/hpack_whole_entry_listener.cc
@@ -21,7 +21,8 @@ HpackDecoderStringBuffer* /*value_buffer*/) {} void HpackWholeEntryNoOpListener::OnDynamicTableSizeUpdate(size_t /*size*/) {} void HpackWholeEntryNoOpListener::OnHpackDecodeError( - HpackDecodingError /*error*/) {} + HpackDecodingError /*error*/, + std::string /*detailed_error*/) {} // static HpackWholeEntryNoOpListener* HpackWholeEntryNoOpListener::NoOpListener() {
diff --git a/http2/hpack/decoder/hpack_whole_entry_listener.h b/http2/hpack/decoder/hpack_whole_entry_listener.h index ef2f77e..ec2ae52 100644 --- a/http2/hpack/decoder/hpack_whole_entry_listener.h +++ b/http2/hpack/decoder/hpack_whole_entry_listener.h
@@ -51,7 +51,8 @@ virtual void OnDynamicTableSizeUpdate(size_t size) = 0; // OnHpackDecodeError is called if an error is detected while decoding. - virtual void OnHpackDecodeError(HpackDecodingError error) = 0; + virtual void OnHpackDecodeError(HpackDecodingError error, + std::string detailed_error) = 0; }; // A no-op implementation of HpackWholeEntryDecoderListener, useful for ignoring @@ -69,7 +70,8 @@ HpackDecoderStringBuffer* name_buffer, HpackDecoderStringBuffer* value_buffer) override; void OnDynamicTableSizeUpdate(size_t size) override; - void OnHpackDecodeError(HpackDecodingError error) override; + void OnHpackDecodeError(HpackDecodingError error, + std::string detailed_error) override; // Returns a listener that ignores all the calls. static HpackWholeEntryNoOpListener* NoOpListener();
diff --git a/quic/core/http/quic_headers_stream_test.cc b/quic/core/http/quic_headers_stream_test.cc index d9e1c8e..d2736ff 100644 --- a/quic/core/http/quic_headers_stream_test.cc +++ b/quic/core/http/quic_headers_stream_test.cc
@@ -87,7 +87,8 @@ public: MOCK_METHOD(void, OnError, - (http2::Http2DecoderAdapter::SpdyFramerError error), + (http2::Http2DecoderAdapter::SpdyFramerError error, + std::string detailed_error), (override)); MOCK_METHOD(void, OnDataFrameHeader,
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc index b0bf7e9..63b6b87 100644 --- a/quic/core/http/quic_spdy_session.cc +++ b/quic/core/http/quic_spdy_session.cc
@@ -135,7 +135,8 @@ QUIC_INVALID_HEADERS_STREAM_DATA); } - void OnError(Http2DecoderAdapter::SpdyFramerError error) override { + void OnError(Http2DecoderAdapter::SpdyFramerError error, + std::string detailed_error) override { QuicErrorCode code; switch (error) { case Http2DecoderAdapter::SpdyFramerError::SPDY_HPACK_INDEX_VARINT_ERROR: @@ -200,7 +201,7 @@ code = QUIC_INVALID_HEADERS_STREAM_DATA; } CloseConnection(quiche::QuicheStrCat( - "SPDY framing error: ", + "SPDY framing error: ", detailed_error, Http2DecoderAdapter::SpdyFramerErrorToString(error)), code); }
diff --git a/spdy/core/hpack/hpack_decoder_adapter.cc b/spdy/core/hpack/hpack_decoder_adapter.cc index 4c44635..cf9a0e7 100644 --- a/spdy/core/hpack/hpack_decoder_adapter.cc +++ b/spdy/core/hpack/hpack_decoder_adapter.cc
@@ -51,6 +51,7 @@ if (!hpack_decoder_.StartDecodingBlock()) { header_block_started_ = false; error_ = hpack_decoder_.error(); + detailed_error_ = hpack_decoder_.detailed_error(); return false; } } @@ -65,12 +66,14 @@ << max_decode_buffer_size_bytes_ << " < " << headers_data_length; error_ = http2::HpackDecodingError::kFragmentTooLong; + detailed_error_ = ""; return false; } listener_adapter_.AddToTotalHpackBytes(headers_data_length); if (max_header_block_bytes_ != 0 && listener_adapter_.total_hpack_bytes() > max_header_block_bytes_) { error_ = http2::HpackDecodingError::kCompressedHeaderSizeExceedsLimit; + detailed_error_ = ""; return false; } http2::DecodeBuffer db(headers_data, headers_data_length); @@ -78,6 +81,7 @@ DCHECK(!ok || db.Empty()) << "Remaining=" << db.Remaining(); if (!ok) { error_ = hpack_decoder_.error(); + detailed_error_ = hpack_decoder_.detailed_error(); } return ok; } @@ -93,6 +97,7 @@ if (!hpack_decoder_.EndDecodingBlock()) { SPDY_DVLOG(3) << "EndDecodingBlock returned false"; error_ = hpack_decoder_.error(); + detailed_error_ = hpack_decoder_.detailed_error(); return false; } header_block_started_ = false;
diff --git a/spdy/core/hpack/hpack_decoder_adapter.h b/spdy/core/hpack/hpack_decoder_adapter.h index 3ff3b4b..f521ee1 100644 --- a/spdy/core/hpack/hpack_decoder_adapter.h +++ b/spdy/core/hpack/hpack_decoder_adapter.h
@@ -88,6 +88,8 @@ // Error code if an error has occurred, Error::kOk otherwise. http2::HpackDecodingError error() const { return error_; } + std::string detailed_error() const { return detailed_error_; } + private: class QUICHE_EXPORT_PRIVATE ListenerAdapter : public http2::HpackDecoderListener, @@ -164,6 +166,7 @@ // Error code if an error has occurred, Error::kOk otherwise. http2::HpackDecodingError error_; + std::string detailed_error_; }; } // namespace spdy
diff --git a/spdy/core/http2_frame_decoder_adapter.cc b/spdy/core/http2_frame_decoder_adapter.cc index f23b583..8f62a93 100644 --- a/spdy/core/http2_frame_decoder_adapter.cc +++ b/spdy/core/http2_frame_decoder_adapter.cc
@@ -361,7 +361,7 @@ << expected_frame_type_ << " frame, but instead received an unknown frame of type " << header.type; - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_UNEXPECTED_FRAME); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_UNEXPECTED_FRAME, ""); return false; } if (!IsSupportedHttp2FrameType(header.type)) { @@ -378,7 +378,7 @@ // Report an invalid frame error if the stream_id is not valid. SPDY_VLOG(1) << "Unknown control frame type " << header.type << " received on invalid stream " << header.stream_id; - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_CONTROL_FRAME); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_CONTROL_FRAME, ""); return false; } else { SPDY_DVLOG(1) << "Ignoring unknown frame type " << header.type; @@ -390,21 +390,21 @@ if (!IsValidHTTP2FrameStreamId(header.stream_id, frame_type)) { SPDY_VLOG(1) << "The framer received an invalid streamID of " << header.stream_id << " for a frame of type " << header.type; - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_STREAM_ID); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_STREAM_ID, ""); return false; } if (has_expected_frame_type_ && header.type != expected_frame_type_) { SPDY_VLOG(1) << "Expected frame type " << expected_frame_type_ << ", not " << header.type; - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_UNEXPECTED_FRAME); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_UNEXPECTED_FRAME, ""); return false; } if (!has_expected_frame_type_ && header.type == Http2FrameType::CONTINUATION) { SPDY_VLOG(1) << "Got CONTINUATION frame when not expected."; - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_UNEXPECTED_FRAME); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_UNEXPECTED_FRAME, ""); return false; } @@ -412,7 +412,7 @@ // For some reason SpdyFramer still rejects invalid DATA frame flags. uint8_t valid_flags = Http2FrameFlag::PADDED | Http2FrameFlag::END_STREAM; if (header.HasAnyFlags(~valid_flags)) { - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_DATA_FRAME_FLAGS); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_DATA_FRAME_FLAGS, ""); return false; } } @@ -496,8 +496,8 @@ on_hpack_fragment_called_ = true; auto* decoder = GetHpackDecoder(); if (!decoder->HandleControlFrameHeadersData(data, len)) { - SetSpdyErrorAndNotify( - HpackDecodingErrorToSpdyFramerError(decoder->error())); + SetSpdyErrorAndNotify(HpackDecodingErrorToSpdyFramerError(decoder->error()), + decoder->detailed_error()); return; } } @@ -522,7 +522,7 @@ if (IsOkToStartFrame(header) && HasRequiredStreamId(header)) { DCHECK(has_hpack_first_frame_header_); if (header.stream_id != hpack_first_frame_header_.stream_id) { - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_UNEXPECTED_FRAME); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_UNEXPECTED_FRAME, ""); return; } frame_header_ = header; @@ -604,7 +604,7 @@ << "; total_padding_length: " << total_padding_length; if (IsOkToStartFrame(header) && HasRequiredStreamId(header)) { if (promise.promised_stream_id == 0) { - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_CONTROL_FRAME); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_CONTROL_FRAME, ""); return; } frame_header_ = header; @@ -706,7 +706,7 @@ if (!SpdyAltSvcWireFormat::ParseHeaderFieldValue(alt_svc_value_, &altsvc_vector)) { SPDY_DLOG(ERROR) << "SpdyAltSvcWireFormat::ParseHeaderFieldValue failed."; - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_CONTROL_FRAME); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_CONTROL_FRAME, ""); return; } visitor()->OnAltSvc(frame_header_.stream_id, alt_svc_origin_, altsvc_vector); @@ -751,33 +751,34 @@ if (header.type == Http2FrameType::DATA) { if (header.payload_length == 0) { DCHECK_EQ(1u, missing_length); - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_DATA_FRAME_FLAGS); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_DATA_FRAME_FLAGS, ""); return; } visitor()->OnStreamPadding(header.stream_id, 1); } - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_PADDING); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_PADDING, ""); } void Http2DecoderAdapter::OnFrameSizeError(const Http2FrameHeader& header) { SPDY_DVLOG(1) << "OnFrameSizeError: " << header; size_t recv_limit = recv_frame_size_limit_; if (header.payload_length > recv_limit) { - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_OVERSIZED_PAYLOAD); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_OVERSIZED_PAYLOAD, ""); return; } if (header.type != Http2FrameType::DATA && header.payload_length > recv_limit) { - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_CONTROL_PAYLOAD_TOO_LARGE); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_CONTROL_PAYLOAD_TOO_LARGE, ""); return; } switch (header.type) { case Http2FrameType::GOAWAY: case Http2FrameType::ALTSVC: - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_CONTROL_FRAME); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_CONTROL_FRAME, ""); break; default: - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_CONTROL_FRAME_SIZE); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_CONTROL_FRAME_SIZE, + ""); } } @@ -855,7 +856,7 @@ if (status != DecodeStatus::kDecodeDone) { SPDY_BUG << "Expected to be done decoding the frame, not " << status; - SetSpdyErrorAndNotify(SPDY_INTERNAL_FRAMER_ERROR); + SetSpdyErrorAndNotify(SPDY_INTERNAL_FRAMER_ERROR, ""); } else if (spdy_framer_error_ != SPDY_NO_ERROR) { SPDY_BUG << "Expected to have no error, not " << SpdyFramerErrorToString(spdy_framer_error_); @@ -866,7 +867,7 @@ set_spdy_state(SpdyState::SPDY_IGNORE_REMAINING_PAYLOAD); } } else { - SetSpdyErrorAndNotify(SPDY_INVALID_CONTROL_FRAME); + SetSpdyErrorAndNotify(SPDY_INVALID_CONTROL_FRAME, ""); } break; } @@ -904,7 +905,8 @@ spdy_state_ = v; } -void Http2DecoderAdapter::SetSpdyErrorAndNotify(SpdyFramerError error) { +void Http2DecoderAdapter::SetSpdyErrorAndNotify(SpdyFramerError error, + std::string detailed_error) { if (HasError()) { DCHECK_EQ(spdy_state_, SpdyState::SPDY_ERROR); } else { @@ -914,7 +916,7 @@ spdy_framer_error_ = error; set_spdy_state(SpdyState::SPDY_ERROR); frame_decoder_->set_listener(&no_op_listener_); - visitor()->OnError(error); + visitor()->OnError(error, detailed_error); } } @@ -979,7 +981,7 @@ if (has_expected_frame_type_ && header.type != expected_frame_type_) { SPDY_VLOG(1) << "Expected frame type " << expected_frame_type_ << ", not " << header.type; - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_UNEXPECTED_FRAME); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_UNEXPECTED_FRAME, ""); return false; } @@ -996,7 +998,7 @@ return true; } SPDY_VLOG(1) << "Stream Id is required, but zero provided"; - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_STREAM_ID); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_STREAM_ID, ""); return false; } @@ -1014,7 +1016,7 @@ return true; } SPDY_VLOG(1) << "Stream Id was not zero, as required: " << stream_id; - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_STREAM_ID); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_STREAM_ID, ""); return false; } @@ -1053,7 +1055,7 @@ visitor()->OnHeaderFrameStart(stream_id()); if (handler == nullptr) { SPDY_BUG << "visitor_->OnHeaderFrameStart returned nullptr"; - SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INTERNAL_FRAMER_ERROR); + SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INTERNAL_FRAMER_ERROR, ""); return; } GetHpackDecoder()->HandleControlFrameHeadersStart(handler); @@ -1086,7 +1088,7 @@ visitor()->OnHeaderFrameEnd(stream_id()); } else { SetSpdyErrorAndNotify( - HpackDecodingErrorToSpdyFramerError(decoder->error())); + HpackDecodingErrorToSpdyFramerError(decoder->error()), ""); return; } const Http2FrameHeader& first = frame_type() == Http2FrameType::CONTINUATION
diff --git a/spdy/core/http2_frame_decoder_adapter.h b/spdy/core/http2_frame_decoder_adapter.h index 4a67833..1ac20c2 100644 --- a/spdy/core/http2_frame_decoder_adapter.h +++ b/spdy/core/http2_frame_decoder_adapter.h
@@ -227,7 +227,7 @@ void set_spdy_state(SpdyState v); - void SetSpdyErrorAndNotify(SpdyFramerError error); + void SetSpdyErrorAndNotify(SpdyFramerError error, std::string detailed_error); const Http2FrameHeader& frame_header() const; @@ -369,7 +369,8 @@ virtual ~SpdyFramerVisitorInterface() {} // Called if an error is detected in the SpdyFrame protocol. - virtual void OnError(http2::Http2DecoderAdapter::SpdyFramerError error) = 0; + virtual void OnError(http2::Http2DecoderAdapter::SpdyFramerError error, + std::string detailed_error) = 0; // Called when the common header for a frame is received. Validating the // common header occurs in later processing.
diff --git a/spdy/core/mock_spdy_framer_visitor.h b/spdy/core/mock_spdy_framer_visitor.h index 89ed853..e54c869 100644 --- a/spdy/core/mock_spdy_framer_visitor.h +++ b/spdy/core/mock_spdy_framer_visitor.h
@@ -23,54 +23,87 @@ MockSpdyFramerVisitor(); ~MockSpdyFramerVisitor() override; - MOCK_METHOD1(OnError, - void(http2::Http2DecoderAdapter::SpdyFramerError error)); - MOCK_METHOD3(OnDataFrameHeader, - void(SpdyStreamId stream_id, size_t length, bool fin)); - MOCK_METHOD3(OnStreamFrameData, - void(SpdyStreamId stream_id, const char* data, size_t len)); - MOCK_METHOD1(OnStreamEnd, void(SpdyStreamId stream_id)); - MOCK_METHOD2(OnStreamPadLength, void(SpdyStreamId stream_id, size_t value)); - MOCK_METHOD2(OnStreamPadding, void(SpdyStreamId stream_id, size_t len)); - MOCK_METHOD1(OnHeaderFrameStart, - SpdyHeadersHandlerInterface*(SpdyStreamId stream_id)); - MOCK_METHOD1(OnHeaderFrameEnd, void(SpdyStreamId stream_id)); - MOCK_METHOD2(OnRstStream, - void(SpdyStreamId stream_id, SpdyErrorCode error_code)); - MOCK_METHOD0(OnSettings, void()); - MOCK_METHOD2(OnSetting, void(SpdySettingsId id, uint32_t value)); - MOCK_METHOD2(OnPing, void(SpdyPingId unique_id, bool is_ack)); - MOCK_METHOD0(OnSettingsEnd, void()); - MOCK_METHOD2(OnGoAway, - void(SpdyStreamId last_accepted_stream_id, - SpdyErrorCode error_code)); - MOCK_METHOD7(OnHeaders, - void(SpdyStreamId stream_id, - bool has_priority, - int weight, - SpdyStreamId parent_stream_id, - bool exclusive, - bool fin, - bool end)); - MOCK_METHOD2(OnWindowUpdate, - void(SpdyStreamId stream_id, int delta_window_size)); - MOCK_METHOD3(OnPushPromise, - void(SpdyStreamId stream_id, - SpdyStreamId promised_stream_id, - bool end)); - MOCK_METHOD2(OnContinuation, void(SpdyStreamId stream_id, bool end)); - MOCK_METHOD3(OnAltSvc, - void(SpdyStreamId stream_id, - quiche::QuicheStringPiece origin, - const SpdyAltSvcWireFormat::AlternativeServiceVector& - altsvc_vector)); - MOCK_METHOD4(OnPriority, - void(SpdyStreamId stream_id, - SpdyStreamId parent_stream_id, - int weight, - bool exclusive)); - MOCK_METHOD2(OnUnknownFrame, - bool(SpdyStreamId stream_id, uint8_t frame_type)); + MOCK_METHOD(void, + OnError, + (http2::Http2DecoderAdapter::SpdyFramerError error, + std::string detailed_error), + (override)); + MOCK_METHOD(void, + OnDataFrameHeader, + (SpdyStreamId stream_id, size_t length, bool fin), + (override)); + MOCK_METHOD(void, + OnStreamFrameData, + (SpdyStreamId stream_id, const char* data, size_t len), + (override)); + MOCK_METHOD(void, OnStreamEnd, (SpdyStreamId stream_id), (override)); + MOCK_METHOD(void, + OnStreamPadLength, + (SpdyStreamId stream_id, size_t value), + (override)); + MOCK_METHOD(void, + OnStreamPadding, + (SpdyStreamId stream_id, size_t len), + (override)); + MOCK_METHOD(SpdyHeadersHandlerInterface*, + OnHeaderFrameStart, + (SpdyStreamId stream_id), + (override)); + MOCK_METHOD(void, OnHeaderFrameEnd, (SpdyStreamId stream_id), (override)); + MOCK_METHOD(void, + OnRstStream, + (SpdyStreamId stream_id, SpdyErrorCode error_code), + (override)); + MOCK_METHOD(void, OnSettings, (), (override)); + MOCK_METHOD(void, OnSetting, (SpdySettingsId id, uint32_t value), (override)); + MOCK_METHOD(void, OnPing, (SpdyPingId unique_id, bool is_ack), (override)); + MOCK_METHOD(void, OnSettingsEnd, (), (override)); + MOCK_METHOD(void, + OnGoAway, + (SpdyStreamId last_accepted_stream_id, SpdyErrorCode error_code), + (override)); + MOCK_METHOD(void, + OnHeaders, + (SpdyStreamId stream_id, + bool has_priority, + int weight, + SpdyStreamId parent_stream_id, + bool exclusive, + bool fin, + bool end), + (override)); + MOCK_METHOD(void, + OnWindowUpdate, + (SpdyStreamId stream_id, int delta_window_size), + (override)); + MOCK_METHOD(void, + OnPushPromise, + (SpdyStreamId stream_id, + SpdyStreamId promised_stream_id, + bool end), + (override)); + MOCK_METHOD(void, + OnContinuation, + (SpdyStreamId stream_id, bool end), + (override)); + MOCK_METHOD( + void, + OnAltSvc, + (SpdyStreamId stream_id, + quiche::QuicheStringPiece origin, + const SpdyAltSvcWireFormat::AlternativeServiceVector& altsvc_vector), + (override)); + MOCK_METHOD(void, + OnPriority, + (SpdyStreamId stream_id, + SpdyStreamId parent_stream_id, + int weight, + bool exclusive), + (override)); + MOCK_METHOD(bool, + OnUnknownFrame, + (SpdyStreamId stream_id, uint8_t frame_type), + (override)); void DelegateHeaderHandling() { ON_CALL(*this, OnHeaderFrameStart(testing::_))
diff --git a/spdy/core/spdy_deframer_visitor.cc b/spdy/core/spdy_deframer_visitor.cc index 10e5e85..3b651e8 100644 --- a/spdy/core/spdy_deframer_visitor.cc +++ b/spdy/core/spdy_deframer_visitor.cc
@@ -148,7 +148,8 @@ void OnDataFrameHeader(SpdyStreamId stream_id, size_t length, bool fin) override; - void OnError(http2::Http2DecoderAdapter::SpdyFramerError error) override; + void OnError(http2::Http2DecoderAdapter::SpdyFramerError error, + std::string detailed_error) override; void OnGoAway(SpdyStreamId last_accepted_stream_id, SpdyErrorCode error_code) override; bool OnGoAwayFrameData(const char* goaway_data, size_t len) override; @@ -461,7 +462,8 @@ // The SpdyFramer will not process any more data at this point. void SpdyTestDeframerImpl::OnError( - http2::Http2DecoderAdapter::SpdyFramerError error) { + http2::Http2DecoderAdapter::SpdyFramerError error, + std::string /*detailed_error*/) { SPDY_DVLOG(1) << "SpdyFramer detected an error in the stream: " << http2::Http2DecoderAdapter::SpdyFramerErrorToString(error) << " frame_type_: " << Http2FrameTypeToString(frame_type_);
diff --git a/spdy/core/spdy_framer_test.cc b/spdy/core/spdy_framer_test.cc index 7e76a26..e36a17e 100644 --- a/spdy/core/spdy_framer_test.cc +++ b/spdy/core/spdy_framer_test.cc
@@ -266,7 +266,8 @@ header_control_type_(SpdyFrameType::DATA), header_buffer_valid_(false) {} - void OnError(Http2DecoderAdapter::SpdyFramerError error) override { + void OnError(Http2DecoderAdapter::SpdyFramerError error, + std::string /*detailed_error*/) override { SPDY_VLOG(1) << "SpdyFramer Error: " << Http2DecoderAdapter::SpdyFramerErrorToString(error); ++error_count_; @@ -727,7 +728,7 @@ SpdySerializedFrame frame(reinterpret_cast<char*>(kH2FrameData), sizeof(kH2FrameData), false); - EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_OVERSIZED_PAYLOAD)); + EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_OVERSIZED_PAYLOAD, _)); deframer_.ProcessInput(frame.data(), frame.size()); EXPECT_TRUE(deframer_.HasError()); EXPECT_EQ(Http2DecoderAdapter::SPDY_OVERSIZED_PAYLOAD, @@ -762,7 +763,7 @@ testing::InSequence seq; EXPECT_CALL(visitor, OnDataFrameHeader(1, 5, 1)); EXPECT_CALL(visitor, OnStreamPadding(1, 1)); - EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_PADDING)); + EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_PADDING, _)); } EXPECT_GT(frame.size(), deframer_.ProcessInput(frame.data(), frame.size())); EXPECT_TRUE(deframer_.HasError()); @@ -795,7 +796,7 @@ testing::InSequence seq; EXPECT_CALL(visitor, OnDataFrameHeader(1, 5, false)); EXPECT_CALL(visitor, OnStreamPadLength(1, 4)); - EXPECT_CALL(visitor, OnError(_)).Times(0); + EXPECT_CALL(visitor, OnError(_, _)).Times(0); // Note that OnStreamFrameData(1, _, 1)) is never called // since there is no data, only padding EXPECT_CALL(visitor, OnStreamPadding(1, 4)); @@ -833,7 +834,7 @@ EXPECT_CALL(visitor, OnHeaders(1, false, 0, 0, false, false, false)); EXPECT_CALL(visitor, OnHeaderFrameStart(1)).Times(1); - EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_PADDING)); + EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_PADDING, _)); EXPECT_EQ(frame.size(), deframer_.ProcessInput(frame.data(), frame.size())); EXPECT_TRUE(deframer_.HasError()); EXPECT_EQ(Http2DecoderAdapter::SPDY_INVALID_PADDING, @@ -883,7 +884,7 @@ SpdySerializedFrame frame(framer_.SerializeData(data_ir)); // We shouldn't have to read the whole frame before we signal an error. - EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID)); + EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, _)); EXPECT_GT(frame.size(), deframer_.ProcessInput(frame.data(), frame.size())); EXPECT_TRUE(deframer_.HasError()); EXPECT_EQ(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, @@ -905,7 +906,7 @@ SpdyFramerPeer::SerializeHeaders(&framer_, headers, &output_)); // We shouldn't have to read the whole frame before we signal an error. - EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID)); + EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, _)); EXPECT_GT(frame.size(), deframer_.ProcessInput(frame.data(), frame.size())); EXPECT_TRUE(deframer_.HasError()); EXPECT_EQ(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, @@ -932,7 +933,7 @@ } // We shouldn't have to read the whole frame before we signal an error. - EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID)); + EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, _)); EXPECT_GT(frame.size(), deframer_.ProcessInput(frame.data(), frame.size())); EXPECT_TRUE(deframer_.HasError()); EXPECT_EQ(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, @@ -956,7 +957,7 @@ } // We shouldn't have to read the whole frame before we signal an error. - EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID)); + EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, _)); EXPECT_GT(frame.size(), deframer_.ProcessInput(frame.data(), frame.size())); EXPECT_TRUE(deframer_.HasError()); EXPECT_EQ(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, @@ -985,7 +986,7 @@ SpdySerializedFrame frame(kH2FrameData, sizeof(kH2FrameData), false); // We shouldn't have to read the whole frame before we signal an error. - EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID)); + EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, _)); EXPECT_GT(frame.size(), deframer_.ProcessInput(frame.data(), frame.size())); EXPECT_TRUE(deframer_.HasError()); EXPECT_EQ(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, @@ -1015,7 +1016,7 @@ SpdySerializedFrame frame(kH2FrameData, sizeof(kH2FrameData), false); // We shouldn't have to read the whole frame before we signal an error. - EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID)); + EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, _)); EXPECT_GT(frame.size(), deframer_.ProcessInput(frame.data(), frame.size())); EXPECT_TRUE(deframer_.HasError()); EXPECT_EQ(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, @@ -1043,7 +1044,7 @@ } // We shouldn't have to read the whole frame before we signal an error. - EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID)); + EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, _)); EXPECT_GT(frame.size(), deframer_.ProcessInput(frame.data(), frame.size())); EXPECT_TRUE(deframer_.HasError()); EXPECT_EQ(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, @@ -1066,7 +1067,7 @@ &framer_, push_promise, use_output_ ? &output_ : nullptr)); // We shouldn't have to read the whole frame before we signal an error. - EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID)); + EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, _)); EXPECT_GT(frame.size(), deframer_.ProcessInput(frame.data(), frame.size())); EXPECT_TRUE(deframer_.HasError()); EXPECT_EQ(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, @@ -1089,7 +1090,7 @@ &framer_, push_promise, use_output_ ? &output_ : nullptr)); EXPECT_CALL(visitor, - OnError(Http2DecoderAdapter::SPDY_INVALID_CONTROL_FRAME)); + OnError(Http2DecoderAdapter::SPDY_INVALID_CONTROL_FRAME, _)); deframer_.ProcessInput(frame.data(), frame.size()); EXPECT_TRUE(deframer_.HasError()); EXPECT_EQ(Http2DecoderAdapter::SPDY_INVALID_CONTROL_FRAME, @@ -2357,7 +2358,7 @@ SpdySerializedFrame frame(kH2FrameData, sizeof(kH2FrameData), false); // We shouldn't have to read the whole frame before we signal an error. - EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_UNEXPECTED_FRAME)); + EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_UNEXPECTED_FRAME, _)); EXPECT_GT(frame.size(), deframer_.ProcessInput(frame.data(), frame.size())); EXPECT_TRUE(deframer_.HasError()); EXPECT_EQ(Http2DecoderAdapter::SPDY_UNEXPECTED_FRAME, @@ -3832,7 +3833,7 @@ SetFrameFlags(&frame, flags); if (flags & ~valid_data_flags) { - EXPECT_CALL(visitor, OnError(_)); + EXPECT_CALL(visitor, OnError(_, _)); } else { EXPECT_CALL(visitor, OnDataFrameHeader(1, 5, flags & DATA_FLAG_FIN)); if (flags & DATA_FLAG_PADDED) { @@ -3840,7 +3841,7 @@ // (0x68) is too large a padding length for a 5 byte payload. EXPECT_CALL(visitor, OnStreamPadding(_, 1)); // Expect Error since the frame ends prematurely. - EXPECT_CALL(visitor, OnError(_)); + EXPECT_CALL(visitor, OnError(_, _)); } else { EXPECT_CALL(visitor, OnStreamFrameData(_, _, 5)); if (flags & DATA_FLAG_FIN) { @@ -3922,7 +3923,7 @@ SetFrameFlags(&frame, flags); if (flags & SETTINGS_FLAG_ACK) { - EXPECT_CALL(visitor, OnError(_)); + EXPECT_CALL(visitor, OnError(_, _)); } else { EXPECT_CALL(visitor, OnSettings()); EXPECT_CALL(visitor, OnSetting(SETTINGS_INITIAL_WINDOW_SIZE, 16)); @@ -4356,7 +4357,7 @@ deframer_.set_visitor(&visitor); EXPECT_CALL(visitor, - OnError(Http2DecoderAdapter::SPDY_INVALID_CONTROL_FRAME)); + OnError(Http2DecoderAdapter::SPDY_INVALID_CONTROL_FRAME, _)); SpdyAltSvcIR altsvc_ir(kStreamId); altsvc_ir.set_origin("o1");
diff --git a/spdy/core/spdy_no_op_visitor.h b/spdy/core/spdy_no_op_visitor.h index a6f4438..93a3235 100644 --- a/spdy/core/spdy_no_op_visitor.h +++ b/spdy/core/spdy_no_op_visitor.h
@@ -26,8 +26,8 @@ ~SpdyNoOpVisitor() override; // SpdyFramerVisitorInterface methods: - void OnError(http2::Http2DecoderAdapter::SpdyFramerError /*error*/) override { - } + void OnError(http2::Http2DecoderAdapter::SpdyFramerError /*error*/, + std::string /*detailed_error*/) override {} SpdyHeadersHandlerInterface* OnHeaderFrameStart( SpdyStreamId stream_id) override; void OnHeaderFrameEnd(SpdyStreamId /*stream_id*/) override {}