Latch in SpdyHeadersIR the feature flag that updates SpdyHeadersIR::size().
We observed a b/331280235 coredump with the following Http2WriteQueue member:
...
pending_frames_bytes_ = 18446744073709551614,
...
From this value, we found that different values of
--gfe2_reloadable_flag_http2_add_hpack_overhead_bytes at various size() call
sites could lead to inconsistent size estimate values, which would lead to
issues when attempting to write.
The new regression test would have failed using the initial version of this
flag: http://sponge2/ae76de9d-6a60-4031-bc9b-12a95ec6ae47 (failed).
This CL deprecates --gfe2_reloadable_flag_http2_add_hpack_overhead_bytes and
renames the flag with the new behavivor to latch in SpdyHeadersIR.
This CL also changes some Http2WriteQueue member types from size_t to ssize_t.
Protected by FLAGS_gfe2_reloadable_flag_http2_add_hpack_overhead_bytes2.
PiperOrigin-RevId: 619920782
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h
index 9666f81..81f9da5 100755
--- a/quiche/common/quiche_feature_flags_list.h
+++ b/quiche/common/quiche_feature_flags_list.h
@@ -8,7 +8,7 @@
#if defined(QUICHE_FLAG)
-QUICHE_FLAG(bool, quiche_reloadable_flag_http2_add_hpack_overhead_bytes, false, "If true, HTTP/2 HEADERS frames will use two additional bytes of HPACK overhead per header in their SpdyFrameIR::size() estimate.")
+QUICHE_FLAG(bool, quiche_reloadable_flag_http2_add_hpack_overhead_bytes2, false, "If true, HTTP/2 HEADERS frames will use two additional bytes of HPACK overhead per header in their SpdyHeadersIR::size() estimate. This flag is latched in SpdyHeadersIR to ensure a consistent size() value for a const SpdyHeadersIR regardless of flag state.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_act_upon_invalid_header, true, "If true, reject or send error response code upon receiving invalid request or response headers.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_add_stream_info_to_idle_close_detail, false, "If true, include stream information in idle timeout connection close detail.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_allow_client_enabled_bbr_v2, true, "If true, allow client to enable BBRv2 on server via connection option 'B2ON'.")
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h
index c489dbb..fbce5e8 100644
--- a/quiche/quic/core/quic_flags_list.h
+++ b/quiche/quic/core/quic_flags_list.h
@@ -17,8 +17,8 @@
QUIC_FLAG(quic_reloadable_flag_quic_block_until_settings_received_copt, true)
// If trrue, early return before write control frame in OnCanWrite() if the connection is already closed.
QUIC_FLAG(quic_reloadable_flag_quic_no_write_control_frame_upon_connection_close, true)
-// If true, HTTP/2 HEADERS frames will use two additional bytes of HPACK overhead per header in their SpdyFrameIR::size() estimate.
-QUIC_FLAG(quic_reloadable_flag_http2_add_hpack_overhead_bytes, true)
+// If true, HTTP/2 HEADERS frames will use two additional bytes of HPACK overhead per header in their SpdyHeadersIR::size() estimate. This flag is latched in SpdyHeadersIR to ensure a consistent size() value for a const SpdyHeadersIR regardless of flag state.
+QUIC_FLAG(quic_reloadable_flag_http2_add_hpack_overhead_bytes2, true)
// If true, QUIC server will not respond to gQUIC probing packet(PING + PADDING) but treat it as a regular packet.
QUIC_FLAG(quic_reloadable_flag_quic_ignore_gquic_probing, true)
// If true, QUIC will default enable MTU discovery at server, with a target of 1450 bytes.
diff --git a/quiche/spdy/core/spdy_protocol.cc b/quiche/spdy/core/spdy_protocol.cc
index 3f2d2f7..fc3b971 100644
--- a/quiche/spdy/core/spdy_protocol.cc
+++ b/quiche/spdy/core/spdy_protocol.cc
@@ -16,7 +16,6 @@
#include "absl/strings/string_view.h"
#include "quiche/common/platform/api/quiche_bug_tracker.h"
#include "quiche/common/platform/api/quiche_flag_utils.h"
-#include "quiche/common/platform/api/quiche_flags.h"
#include "quiche/common/platform/api/quiche_logging.h"
#include "quiche/spdy/core/http2_header_block.h"
#include "quiche/spdy/core/spdy_alt_svc_wire_format.h"
@@ -476,10 +475,10 @@
}
// TODO(b/322146543): Remove `hpack_overhead` with deprecation of
- // --gfe2_reloadable_flag_http2_add_hpack_overhead_bytes.
+ // --gfe2_reloadable_flag_http2_add_hpack_overhead_bytes2.
size_t hpack_overhead = kPerHeaderHpackOverheadOld;
- if (GetQuicheReloadableFlag(http2_add_hpack_overhead_bytes)) {
- QUICHE_RELOADABLE_FLAG_COUNT(http2_add_hpack_overhead_bytes);
+ if (add_hpack_overhead_bytes_) {
+ QUICHE_RELOADABLE_FLAG_COUNT(http2_add_hpack_overhead_bytes2);
hpack_overhead = kPerHeaderHpackOverheadNew;
}
// Assume no hpack encoding is applied.
diff --git a/quiche/spdy/core/spdy_protocol.h b/quiche/spdy/core/spdy_protocol.h
index 8501c94..09bc49e 100644
--- a/quiche/spdy/core/spdy_protocol.h
+++ b/quiche/spdy/core/spdy_protocol.h
@@ -22,6 +22,7 @@
#include "absl/strings/string_view.h"
#include "absl/types/variant.h"
#include "quiche/common/platform/api/quiche_export.h"
+#include "quiche/common/platform/api/quiche_flags.h"
#include "quiche/common/platform/api/quiche_logging.h"
#include "quiche/spdy/core/http2_header_block.h"
#include "quiche/spdy/core/spdy_alt_svc_wire_format.h"
@@ -338,13 +339,13 @@
// - 2 bytes for the name length (assuming new name).
// - 3 bytes for the value length.
// TODO(b/322146543): Remove the `New` suffix with deprecation of
-// --gfe2_reloadable_flag_http2_add_hpack_overhead_bytes.
+// --gfe2_reloadable_flag_http2_add_hpack_overhead_bytes2.
inline constexpr size_t kPerHeaderHpackOverheadNew = 6;
// An estimate size of the HPACK overhead for each header field. 1 bytes for
// indexed literal, 1 bytes for key literal and length encoding, and 2 bytes for
// value literal and length encoding.
// TODO(b/322146543): Remove with deprecation of
-// --gfe2_reloadable_flag_http2_add_hpack_overhead_bytes.
+// --gfe2_reloadable_flag_http2_add_hpack_overhead_bytes2.
inline constexpr size_t kPerHeaderHpackOverheadOld = 4;
// Names of pseudo-headers defined for HTTP/2 requests.
@@ -466,7 +467,8 @@
SpdyStreamId stream_id() const { return stream_id_; }
virtual bool fin() const;
// Returns an estimate of the size of the serialized frame, without applying
- // compression. May not be exact.
+ // compression. May not be exact, but implementations should return the same
+ // value for a const frame.
virtual size_t size() const = 0;
// Returns the number of bytes of flow control window that would be consumed
@@ -755,6 +757,8 @@
bool exclusive_ = false;
bool padded_ = false;
int padding_payload_len_ = 0;
+ const bool add_hpack_overhead_bytes_ =
+ GetQuicheReloadableFlag(http2_add_hpack_overhead_bytes2);
};
class QUICHE_EXPORT SpdyWindowUpdateIR : public SpdyFrameIR {
