gfe-relnote: Use indexed name if possible when sending Literal Header Field without Indexing instruction. Protected by gfe2_reloadable_flag_spdy_hpack_use_indexed_name. This CL touches HPACK encoding, which is used over HTTP/2 and Google QUIC. If compression is enabled and a pseudo-header other than :authority has a matching name but not matching value in the static table, then before this change, its name was sent as a string literal; after this change, it is sent indexed. Bug: https://crbug.com/983254 PiperOrigin-RevId: 305517407 Change-Id: If6172beacb02efe2465d8753749e6c586688e3c0
diff --git a/spdy/core/hpack/hpack_encoder.cc b/spdy/core/hpack/hpack_encoder.cc index 3835047..e3bf8f6 100644 --- a/spdy/core/hpack/hpack_encoder.cc +++ b/spdy/core/hpack/hpack_encoder.cc
@@ -13,6 +13,7 @@ #include "net/third_party/quiche/src/spdy/core/hpack/hpack_huffman_table.h" #include "net/third_party/quiche/src/spdy/core/hpack/hpack_output_stream.h" #include "net/third_party/quiche/src/spdy/platform/api/spdy_estimate_memory_usage.h" +#include "net/third_party/quiche/src/spdy/platform/api/spdy_flags.h" #include "net/third_party/quiche/src/spdy/platform/api/spdy_logging.h" namespace spdy { @@ -146,10 +147,10 @@ } else if (should_index_(header.first, header.second)) { EmitIndexedLiteral(header); } else { - EmitNonIndexedLiteral(header); + EmitNonIndexedLiteral(header, enable_compression_); } } else { - EmitNonIndexedLiteral(header); + EmitNonIndexedLiteral(header, enable_compression_); } } @@ -170,12 +171,25 @@ header_table_.TryAddEntry(representation.first, representation.second); } -void HpackEncoder::EmitNonIndexedLiteral(const Representation& representation) { +void HpackEncoder::EmitNonIndexedLiteral(const Representation& representation, + bool enable_compression) { SPDY_DVLOG(2) << "Emitting nonindexed literal: (" << representation.first << ", " << representation.second << ")"; output_stream_.AppendPrefix(kLiteralNoIndexOpcode); - output_stream_.AppendUint32(0); - EmitString(representation.first); + if (GetSpdyReloadableFlag(spdy_hpack_use_indexed_name)) { + SPDY_CODE_COUNT(spdy_hpack_use_indexed_name); + const HpackEntry* name_entry = + header_table_.GetByName(representation.first); + if (enable_compression && name_entry != nullptr) { + output_stream_.AppendUint32(header_table_.IndexOf(name_entry)); + } else { + output_stream_.AppendUint32(0); + EmitString(representation.first); + } + } else { + output_stream_.AppendUint32(0); + EmitString(representation.first); + } EmitString(representation.second); } @@ -347,14 +361,14 @@ std::string* output) { SPDY_BUG_IF(!has_next_) << "Encoderator::Next called with nothing left to encode."; - const bool use_compression = encoder_->enable_compression_; + const bool enable_compression = encoder_->enable_compression_; // Encode up to max_encoded_bytes of headers. while (header_it_->HasNext() && encoder_->output_stream_.size() <= max_encoded_bytes) { const Representation header = header_it_->Next(); encoder_->listener_(header.first, header.second); - if (use_compression) { + if (enable_compression) { const HpackEntry* entry = encoder_->header_table_.GetByNameAndValue( header.first, header.second); if (entry != nullptr) { @@ -362,10 +376,10 @@ } else if (encoder_->should_index_(header.first, header.second)) { encoder_->EmitIndexedLiteral(header); } else { - encoder_->EmitNonIndexedLiteral(header); + encoder_->EmitNonIndexedLiteral(header, enable_compression); } } else { - encoder_->EmitNonIndexedLiteral(header); + encoder_->EmitNonIndexedLiteral(header, enable_compression); } }
diff --git a/spdy/core/hpack/hpack_encoder.h b/spdy/core/hpack/hpack_encoder.h index c3d3a19..534b9e6 100644 --- a/spdy/core/hpack/hpack_encoder.h +++ b/spdy/core/hpack/hpack_encoder.h
@@ -122,7 +122,8 @@ // Emits a literal representation (Section 7.2). void EmitIndexedLiteral(const Representation& representation); - void EmitNonIndexedLiteral(const Representation& representation); + void EmitNonIndexedLiteral(const Representation& representation, + bool enable_compression); void EmitLiteral(const Representation& representation); // Emits a Huffman or identity string (whichever is smaller).
diff --git a/spdy/core/hpack/hpack_encoder_test.cc b/spdy/core/hpack/hpack_encoder_test.cc index 2ac0a5f..2b1efe9 100644 --- a/spdy/core/hpack/hpack_encoder_test.cc +++ b/spdy/core/hpack/hpack_encoder_test.cc
@@ -11,6 +11,7 @@ #include "net/third_party/quiche/src/common/platform/api/quiche_test.h" #include "net/third_party/quiche/src/spdy/core/hpack/hpack_huffman_table.h" #include "net/third_party/quiche/src/spdy/core/spdy_simple_arena.h" +#include "net/third_party/quiche/src/spdy/platform/api/spdy_flags.h" namespace spdy { @@ -182,6 +183,12 @@ ExpectString(&expected_, name); ExpectString(&expected_, value); } + void ExpectNonIndexedLiteralWithNameIndex(const HpackEntry* key_entry, + quiche::QuicheStringPiece value) { + expected_.AppendPrefix(kLiteralNoIndexOpcode); + expected_.AppendUint32(IndexOf(key_entry)); + ExpectString(&expected_, value); + } void ExpectString(HpackOutputStream* stream, quiche::QuicheStringPiece str) { const HpackHuffmanTable& huffman_table = peer_.huffman_table(); size_t encoded_size = peer_.compression_enabled() @@ -267,7 +274,12 @@ {"accept", "text/html, text/plain,application/xml"}, {"cookie", "val4"}, {"withnul", quiche::QuicheStringPiece("one\0two", 7)}}; - ExpectNonIndexedLiteral(":path", "/home"); + if (GetSpdyReloadableFlag(spdy_hpack_use_indexed_name)) { + ExpectNonIndexedLiteralWithNameIndex(peer_.table()->GetByName(":path"), + "/home"); + } else { + ExpectNonIndexedLiteral(":path", "/home"); + } ExpectIndexedLiteral(peer_.table()->GetByName("cookie"), "val1"); ExpectIndexedLiteral(peer_.table()->GetByName("cookie"), "val2"); ExpectIndexedLiteral(peer_.table()->GetByName("cookie"), "val3"); @@ -554,7 +566,12 @@ // Headers are indexed in the order in which they were added. // This entry pushes "cookie: a=bb" back to 63. - ExpectNonIndexedLiteral(":path", "/spam/eggs.html"); + if (GetSpdyReloadableFlag(spdy_hpack_use_indexed_name)) { + ExpectNonIndexedLiteralWithNameIndex(peer_.table()->GetByName(":path"), + "/spam/eggs.html"); + } else { + ExpectNonIndexedLiteral(":path", "/spam/eggs.html"); + } ExpectIndexedLiteral(peer_.table()->GetByName(":authority"), "www.example.com"); ExpectIndexedLiteral("-foo", "bar");