Removes support for detailed HPACK error messages. These messages are not used by either Chromium or GFE when speaking HTTP/2, and only referenced in GFE for gQUIC, which is deprecated. Protected by removes a verbose human-readable error message, not protected. PiperOrigin-RevId: 596108431
diff --git a/quiche/http2/hpack/decoder/hpack_decoder.cc b/quiche/http2/hpack/decoder/hpack_decoder.cc index 935cd7f..310e8a7 100644 --- a/quiche/http2/hpack/decoder/hpack_decoder.cc +++ b/quiche/http2/hpack/decoder/hpack_decoder.cc
@@ -54,7 +54,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()); QUICHE_CODE_COUNT_N(decompress_failure_3, 4, 23); return false; } else if (DetectError()) { @@ -80,7 +80,7 @@ } if (!block_decoder_.before_entry()) { // The HPACK block ended in the middle of an entry. - ReportError(HpackDecodingError::kTruncatedBlock, ""); + ReportError(HpackDecodingError::kTruncatedBlock); QUICHE_CODE_COUNT_N(decompress_failure_3, 7, 23); return false; } @@ -102,20 +102,17 @@ QUICHE_DVLOG(2) << "Error detected in decoder_state_"; QUICHE_CODE_COUNT_N(decompress_failure_3, 10, 23); error_ = decoder_state_.error(); - detailed_error_ = decoder_state_.detailed_error(); } return error_ != HpackDecodingError::kOk; } -void HpackDecoder::ReportError(HpackDecodingError error, - std::string detailed_error) { +void HpackDecoder::ReportError(HpackDecodingError error) { QUICHE_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/quiche/http2/hpack/decoder/hpack_decoder.h b/quiche/http2/hpack/decoder/hpack_decoder.h index 9e4b68b..d231f85 100644 --- a/quiche/http2/hpack/decoder/hpack_decoder.h +++ b/quiche/http2/hpack/decoder/hpack_decoder.h
@@ -104,13 +104,11 @@ // Error code if an error has occurred, HpackDecodingError::kOk otherwise. HpackDecodingError error() const { return error_; } - 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, std::string detailed_error); + void ReportError(HpackDecodingError error); // The decompressor state, as defined by HPACK (i.e. the static and dynamic // tables). @@ -124,7 +122,6 @@ // Error code if an error has occurred, HpackDecodingError::kOk otherwise. HpackDecodingError error_; - std::string detailed_error_; }; } // namespace http2
diff --git a/quiche/http2/hpack/decoder/hpack_decoder_state.cc b/quiche/http2/hpack/decoder/hpack_decoder_state.cc index e34be72..459a475 100644 --- a/quiche/http2/hpack/decoder/hpack_decoder_state.cc +++ b/quiche/http2/hpack/decoder/hpack_decoder_state.cc
@@ -83,7 +83,7 @@ return; } if (require_dynamic_table_size_update_) { - ReportError(HpackDecodingError::kMissingDynamicTableSizeUpdate, ""); + ReportError(HpackDecodingError::kMissingDynamicTableSizeUpdate); return; } allow_dynamic_table_size_update_ = false; @@ -91,7 +91,7 @@ if (entry != nullptr) { listener_->OnHeader(entry->name, entry->value); } else { - ReportError(HpackDecodingError::kInvalidIndex, ""); + ReportError(HpackDecodingError::kInvalidIndex); } } @@ -105,7 +105,7 @@ return; } if (require_dynamic_table_size_update_) { - ReportError(HpackDecodingError::kMissingDynamicTableSizeUpdate, ""); + ReportError(HpackDecodingError::kMissingDynamicTableSizeUpdate); return; } allow_dynamic_table_size_update_ = false; @@ -117,7 +117,7 @@ decoder_tables_.Insert(entry->name, std::move(value)); } } else { - ReportError(HpackDecodingError::kInvalidNameIndex, ""); + ReportError(HpackDecodingError::kInvalidNameIndex); } } @@ -130,7 +130,7 @@ return; } if (require_dynamic_table_size_update_) { - ReportError(HpackDecodingError::kMissingDynamicTableSizeUpdate, ""); + ReportError(HpackDecodingError::kMissingDynamicTableSizeUpdate); return; } allow_dynamic_table_size_update_ = false; @@ -155,15 +155,14 @@ 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; @@ -171,8 +170,7 @@ // 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); @@ -185,12 +183,11 @@ lowest_header_table_size_ = final_header_table_size_; } -void HpackDecoderState::OnHpackDecodeError(HpackDecodingError error, - std::string detailed_error) { +void HpackDecoderState::OnHpackDecodeError(HpackDecodingError error) { QUICHE_DVLOG(2) << "HpackDecoderState::OnHpackDecodeError " << HpackDecodingErrorToString(error); if (error_ == HpackDecodingError::kOk) { - ReportError(error, detailed_error); + ReportError(error); } } @@ -202,21 +199,19 @@ 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, - std::string detailed_error) { +void HpackDecoderState::ReportError(HpackDecodingError error) { QUICHE_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/quiche/http2/hpack/decoder/hpack_decoder_state.h b/quiche/http2/hpack/decoder/hpack_decoder_state.h index 4198ef4..32b5c2b 100644 --- a/quiche/http2/hpack/decoder/hpack_decoder_state.h +++ b/quiche/http2/hpack/decoder/hpack_decoder_state.h
@@ -76,8 +76,7 @@ HpackDecoderStringBuffer* name_buffer, HpackDecoderStringBuffer* value_buffer) override; void OnDynamicTableSizeUpdate(size_t size) override; - void OnHpackDecodeError(HpackDecodingError error, - std::string detailed_error) override; + void OnHpackDecodeError(HpackDecodingError error) override; // OnHeaderBlockEnd notifies this object that an entire HPACK block has been // decoded, which might have extended into CONTINUATION blocks. @@ -95,13 +94,11 @@ 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, std::string detailed_error); + void ReportError(HpackDecodingError error); // The static and dynamic HPACK tables. HpackDecoderTables decoder_tables_; @@ -129,7 +126,6 @@ // Has an error already been detected and reported to the listener? HpackDecodingError error_; - std::string detailed_error_; }; } // namespace http2
diff --git a/quiche/http2/hpack/decoder/hpack_decoder_state_test.cc b/quiche/http2/hpack/decoder/hpack_decoder_state_test.cc index a5354e7..12df0be 100644 --- a/quiche/http2/hpack/decoder/hpack_decoder_state_test.cc +++ b/quiche/http2/hpack/decoder/hpack_decoder_state_test.cc
@@ -448,7 +448,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. @@ -517,7 +517,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); @@ -529,7 +529,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/quiche/http2/hpack/decoder/hpack_whole_entry_buffer.cc b/quiche/http2/hpack/decoder/hpack_whole_entry_buffer.cc index ab6128e..04c8aad 100644 --- a/quiche/http2/hpack/decoder/hpack_whole_entry_buffer.cc +++ b/quiche/http2/hpack/decoder/hpack_whole_entry_buffer.cc
@@ -144,7 +144,7 @@ QUICHE_DVLOG(1) << "HpackWholeEntryBuffer::ReportError: " << HpackDecodingErrorToString(error); error_detected_ = true; - listener_->OnHpackDecodeError(error, ""); + listener_->OnHpackDecodeError(error); listener_ = HpackWholeEntryNoOpListener::NoOpListener(); } }
diff --git a/quiche/http2/hpack/decoder/hpack_whole_entry_buffer_test.cc b/quiche/http2/hpack/decoder/hpack_whole_entry_buffer_test.cc index 03ad77c..b1d3907 100644 --- a/quiche/http2/hpack/decoder/hpack_whole_entry_buffer_test.cc +++ b/quiche/http2/hpack/decoder/hpack_whole_entry_buffer_test.cc
@@ -35,9 +35,7 @@ HpackDecoderStringBuffer* value_buffer), (override)); MOCK_METHOD(void, OnDynamicTableSizeUpdate, (size_t size), (override)); - MOCK_METHOD(void, OnHpackDecodeError, - (HpackDecodingError error, std::string detailed_error), - (override)); + MOCK_METHOD(void, OnHpackDecodeError, (HpackDecodingError error), (override)); }; class HpackWholeEntryBufferTest : public quiche::test::QuicheTest { @@ -155,16 +153,14 @@ // 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 value longer than the allowed size generates an error. TEST_F(HpackWholeEntryBufferTest, ValueTooLong) { entry_buffer_.OnStartLiteralHeader(HpackEntryType::kIndexedLiteralHeader, 0); - EXPECT_CALL(listener_, - OnHpackDecodeError(HpackDecodingError::kValueTooLong, "")); + EXPECT_CALL(listener_, OnHpackDecodeError(HpackDecodingError::kValueTooLong)); entry_buffer_.OnNameStart(false, 4); entry_buffer_.OnNameData("path", 4); entry_buffer_.OnNameEnd(); @@ -174,8 +170,7 @@ // Regression test for b/162141899. TEST_F(HpackWholeEntryBufferTest, ValueTooLongWithoutName) { entry_buffer_.OnStartLiteralHeader(HpackEntryType::kIndexedLiteralHeader, 1); - EXPECT_CALL(listener_, - OnHpackDecodeError(HpackDecodingError::kValueTooLong, "")); + EXPECT_CALL(listener_, OnHpackDecodeError(HpackDecodingError::kValueTooLong)); entry_buffer_.OnValueStart(false, kMaxStringSize + 1); } @@ -189,7 +184,7 @@ entry_buffer_.OnNameData(data, 3); EXPECT_CALL(listener_, - OnHpackDecodeError(HpackDecodingError::kNameHuffmanError, _)); + OnHpackDecodeError(HpackDecodingError::kNameHuffmanError)); entry_buffer_.OnNameData(data, 1); @@ -208,7 +203,7 @@ entry_buffer_.OnValueData(data, 3); EXPECT_CALL(listener_, - OnHpackDecodeError(HpackDecodingError::kValueHuffmanError, _)); + OnHpackDecodeError(HpackDecodingError::kValueHuffmanError)); entry_buffer_.OnValueEnd();
diff --git a/quiche/http2/hpack/decoder/hpack_whole_entry_listener.cc b/quiche/http2/hpack/decoder/hpack_whole_entry_listener.cc index d6fe82b..ee2dc91 100644 --- a/quiche/http2/hpack/decoder/hpack_whole_entry_listener.cc +++ b/quiche/http2/hpack/decoder/hpack_whole_entry_listener.cc
@@ -19,7 +19,7 @@ HpackDecoderStringBuffer* /*value_buffer*/) {} void HpackWholeEntryNoOpListener::OnDynamicTableSizeUpdate(size_t /*size*/) {} void HpackWholeEntryNoOpListener::OnHpackDecodeError( - HpackDecodingError /*error*/, std::string /*detailed_error*/) {} + HpackDecodingError /*error*/) {} // static HpackWholeEntryNoOpListener* HpackWholeEntryNoOpListener::NoOpListener() {
diff --git a/quiche/http2/hpack/decoder/hpack_whole_entry_listener.h b/quiche/http2/hpack/decoder/hpack_whole_entry_listener.h index 54e4506..12e10fe 100644 --- a/quiche/http2/hpack/decoder/hpack_whole_entry_listener.h +++ b/quiche/http2/hpack/decoder/hpack_whole_entry_listener.h
@@ -49,8 +49,7 @@ virtual void OnDynamicTableSizeUpdate(size_t size) = 0; // OnHpackDecodeError is called if an error is detected while decoding. - virtual void OnHpackDecodeError(HpackDecodingError error, - std::string detailed_error) = 0; + virtual void OnHpackDecodeError(HpackDecodingError error) = 0; }; // A no-op implementation of HpackWholeEntryDecoderListener, useful for ignoring @@ -68,8 +67,7 @@ HpackDecoderStringBuffer* name_buffer, HpackDecoderStringBuffer* value_buffer) override; void OnDynamicTableSizeUpdate(size_t size) override; - void OnHpackDecodeError(HpackDecodingError error, - std::string detailed_error) override; + void OnHpackDecodeError(HpackDecodingError error) override; // Returns a listener that ignores all the calls. static HpackWholeEntryNoOpListener* NoOpListener();
diff --git a/quiche/spdy/core/hpack/hpack_decoder_adapter.cc b/quiche/spdy/core/hpack/hpack_decoder_adapter.cc index 6e7d2c5..b982778 100644 --- a/quiche/spdy/core/hpack/hpack_decoder_adapter.cc +++ b/quiche/spdy/core/hpack/hpack_decoder_adapter.cc
@@ -55,7 +55,6 @@ if (!hpack_decoder_.StartDecodingBlock()) { header_block_started_ = false; error_ = hpack_decoder_.error(); - detailed_error_ = hpack_decoder_.detailed_error(); return false; } } @@ -70,14 +69,12 @@ << 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); @@ -85,7 +82,6 @@ QUICHE_DCHECK(!ok || db.Empty()) << "Remaining=" << db.Remaining(); if (!ok) { error_ = hpack_decoder_.error(); - detailed_error_ = hpack_decoder_.detailed_error(); } return ok; } @@ -97,7 +93,6 @@ if (!hpack_decoder_.EndDecodingBlock()) { QUICHE_DVLOG(3) << "EndDecodingBlock returned false"; error_ = hpack_decoder_.error(); - detailed_error_ = hpack_decoder_.detailed_error(); return false; } header_block_started_ = false;
diff --git a/quiche/spdy/core/hpack/hpack_decoder_adapter.h b/quiche/spdy/core/hpack/hpack_decoder_adapter.h index ce3220e..d3d3dec 100644 --- a/quiche/spdy/core/hpack/hpack_decoder_adapter.h +++ b/quiche/spdy/core/hpack/hpack_decoder_adapter.h
@@ -80,8 +80,6 @@ // 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 ListenerAdapter : public http2::HpackDecoderListener { public: @@ -137,7 +135,6 @@ // Error code if an error has occurred, Error::kOk otherwise. http2::HpackDecodingError error_; - std::string detailed_error_; }; } // namespace spdy
diff --git a/quiche/spdy/core/http2_frame_decoder_adapter.cc b/quiche/spdy/core/http2_frame_decoder_adapter.cc index 58cf162..3896ebd 100644 --- a/quiche/spdy/core/http2_frame_decoder_adapter.cc +++ b/quiche/spdy/core/http2_frame_decoder_adapter.cc
@@ -475,7 +475,7 @@ auto& decoder = GetHpackDecoder(); if (!decoder.HandleControlFrameHeadersData(data, len)) { SetSpdyErrorAndNotify(HpackDecodingErrorToSpdyFramerError(decoder.error()), - decoder.detailed_error()); + ""); return; } }