http/2: use hpack_table_size to control both encoder and decoder. (en…

…voyproxy#3659)

Previously, hpack_table_size was used to configure maximum table size used by
the local endpoint for HPACK decoding, however, there was no way to configure
table size used for HPACK enoding.

Since this option is mostly used to disable header compression by setting the
size to 0, it means that Envoy only asked the remote endpoint not to compress
headers, but it was still compressing them itself (unless asked not to by the
remote endpoint).

Re-using hpack_table_size instead of adding a new option, since both: encoder
and decoder will usually use the same value anyway.

*Level*: Medium (some broken libraries don't support header table updates)
*Testing*: bazel test //test/...
*Docs Changes*: n/a
*Release Notes*: n/a

Signed-off-by: Piotr Sikora <[email protected]>

docs/root/intro/version_history.rst

@@ -18,16 +18,18 @@ Version history
   <envoy_api_field_route.RouteAction.idle_timeout>`. This defaults to 5 minutes; if you have
   other timeouts (e.g. connection idle timeout, upstream response per-retry) that are longer than
   this in duration, you may want to consider setting a non-default per-stream idle timeout.
+* http: added generic :ref:`Upgrade support 
+  <envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.upgrade_configs>`.
 * http: better handling of HEAD requests. Now sending transfer-encoding: chunked rather than content-length: 0.
 * http: response filters not applied to early error paths such as http_parser generated 400s.
-* proxy_protocol: added support for HAProxy Proxy Protocol v2 (AF_INET/AF_INET6 only).
-* http: added generic +:ref:`Upgrade support 
-  <envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.upgrade_configs>`
+* http: :ref:`hpack_table_size <envoy_api_field_core.Http2ProtocolOptions.hpack_table_size>` now controls
+  dynamic table size of both: encoder and decoder.
 * listeners: added the ability to match :ref:`FilterChain <envoy_api_msg_listener.FilterChain>` using
   :ref:`destination_port <envoy_api_field_listener.FilterChainMatch.destination_port>` and
   :ref:`prefix_ranges <envoy_api_field_listener.FilterChainMatch.prefix_ranges>`.
 * lua: added :ref:`connection() <config_http_filters_lua_connection_wrapper>` wrapper and *ssl()* API.
 * lua: added :ref:`requestInfo() <config_http_filters_lua_request_info_wrapper>` wrapper and *protocol()* API.
+* proxy_protocol: added support for HAProxy Proxy Protocol v2 (AF_INET/AF_INET6 only).
 * ratelimit: added support for :repo:`api/envoy/service/ratelimit/v2/rls.proto`.
   Lyft's reference implementation of the `ratelimit <https://github.com/lyft/ratelimit>`_ service also supports the data-plane-api proto as of v1.1.0.
   Envoy can use either proto to send client requests to a ratelimit server with the use of the

source/common/http/http2/codec_impl.cc

@@ -37,8 +37,6 @@ bool Utility::reconstituteCrumbledCookies(const HeaderString& key, const HeaderS
 }
 
 ConnectionImpl::Http2Callbacks ConnectionImpl::http2_callbacks_;
-ConnectionImpl::Http2Options ConnectionImpl::http2_options_;
-ConnectionImpl::ClientHttp2Options ConnectionImpl::client_http2_options_;
 
 /**
  * Helper to remove const during a cast. nghttp2 takes non-const pointers for headers even though
@@ -724,19 +722,24 @@ ConnectionImpl::Http2Callbacks::Http2Callbacks() {
 
 ConnectionImpl::Http2Callbacks::~Http2Callbacks() { nghttp2_session_callbacks_del(callbacks_); }
 
-ConnectionImpl::Http2Options::Http2Options() {
+ConnectionImpl::Http2Options::Http2Options(const Http2Settings& http2_settings) {
   nghttp2_option_new(&options_);
   // Currently we do not do anything with stream priority. Setting the following option prevents
   // nghttp2 from keeping around closed streams for use during stream priority dependency graph
   // calculations. This saves a tremendous amount of memory in cases where there are a large number
   // of kept alive HTTP/2 connections.
   nghttp2_option_set_no_closed_streams(options_, 1);
   nghttp2_option_set_no_auto_window_update(options_, 1);
+
+  if (http2_settings.hpack_table_size_ != NGHTTP2_DEFAULT_HEADER_TABLE_SIZE) {
+    nghttp2_option_set_max_deflate_dynamic_table_size(options_, http2_settings.hpack_table_size_);
+  }
 }
 
 ConnectionImpl::Http2Options::~Http2Options() { nghttp2_option_del(options_); }
 
-ConnectionImpl::ClientHttp2Options::ClientHttp2Options() : Http2Options() {
+ConnectionImpl::ClientHttp2Options::ClientHttp2Options(const Http2Settings& http2_settings)
+    : Http2Options(http2_settings) {
   // Temporarily disable initial max streams limit/protection, since we might want to create
   // more than 100 streams before receiving the HTTP/2 SETTINGS frame from the server.
   //
@@ -749,8 +752,9 @@ ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection,
                                            Http::ConnectionCallbacks& callbacks,
                                            Stats::Scope& stats, const Http2Settings& http2_settings)
     : ConnectionImpl(connection, stats, http2_settings), callbacks_(callbacks) {
+  ClientHttp2Options client_http2_options(http2_settings);
   nghttp2_session_client_new2(&session_, http2_callbacks_.callbacks(), base(),
-                              client_http2_options_.options());
+                              client_http2_options.options());
   sendSettings(http2_settings, true);
 }
 
@@ -794,8 +798,9 @@ ServerConnectionImpl::ServerConnectionImpl(Network::Connection& connection,
                                            Http::ServerConnectionCallbacks& callbacks,
                                            Stats::Scope& scope, const Http2Settings& http2_settings)
     : ConnectionImpl(connection, scope, http2_settings), callbacks_(callbacks) {
+  Http2Options http2_options(http2_settings);
   nghttp2_session_server_new2(&session_, http2_callbacks_.callbacks(), base(),
-                              http2_options_.options());
+                              http2_options.options());
   sendSettings(http2_settings, false);
 }
 

source/common/http/http2/codec_impl.h

@@ -118,7 +118,7 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
    */
   class Http2Options {
   public:
-    Http2Options();
+    Http2Options(const Http2Settings& http2_settings);
     ~Http2Options();
 
     const nghttp2_option* options() { return options_; }
@@ -129,7 +129,7 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
 
   class ClientHttp2Options : public Http2Options {
   public:
-    ClientHttp2Options();
+    ClientHttp2Options(const Http2Settings& http2_settings);
   };
 
   /**
@@ -243,8 +243,6 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
   void sendSettings(const Http2Settings& http2_settings, bool disable_push);
 
   static Http2Callbacks http2_callbacks_;
-  static Http2Options http2_options_;
-  static ClientHttp2Options client_http2_options_;
 
   std::list<StreamImplPtr> active_streams_;
   nghttp2_session* session_{};

test/common/http/http2/codec_impl_test.cc

@@ -783,6 +783,34 @@ TEST_P(Http2CodecImplTest, TestCodecHeaderLimits) {
   request_encoder_->encodeHeaders(request_headers, false);
 }
 
+TEST_P(Http2CodecImplTest, TestCodecHeaderCompression) {
+  initialize();
+
+  TestHeaderMapImpl request_headers;
+  HttpTestUtility::addDefaultHeaders(request_headers);
+  EXPECT_CALL(request_decoder_, decodeHeaders_(_, true));
+  request_encoder_->encodeHeaders(request_headers, true);
+
+  TestHeaderMapImpl response_headers{{":status", "200"}, {"compression", "test"}};
+  EXPECT_CALL(response_decoder_, decodeHeaders_(_, true));
+  response_encoder_->encodeHeaders(response_headers, true);
+
+  // Sanity check to verify that state of encoders and decoders matches.
+  EXPECT_EQ(nghttp2_session_get_hd_deflate_dynamic_table_size(server_.session()),
+            nghttp2_session_get_hd_inflate_dynamic_table_size(client_.session()));
+  EXPECT_EQ(nghttp2_session_get_hd_deflate_dynamic_table_size(client_.session()),
+            nghttp2_session_get_hd_inflate_dynamic_table_size(server_.session()));
+
+  // Verify that headers are compressed only when both client and server advertise table size > 0:
+  if (client_http2settings_.hpack_table_size_ && server_http2settings_.hpack_table_size_) {
+    EXPECT_NE(0, nghttp2_session_get_hd_deflate_dynamic_table_size(client_.session()));
+    EXPECT_NE(0, nghttp2_session_get_hd_deflate_dynamic_table_size(server_.session()));
+  } else {
+    EXPECT_EQ(0, nghttp2_session_get_hd_deflate_dynamic_table_size(client_.session()));
+    EXPECT_EQ(0, nghttp2_session_get_hd_deflate_dynamic_table_size(server_.session()));
+  }
+}
+
 } // namespace Http2
 } // namespace Http
 } // namespace Envoy