[router] Fix byte size logging in HTTP gRPC upstream logs (#8836)

Refreshes byte size on downstream headers in `router.cc`. Enabling gRPC upstream
logs will write the byte size of these headers to the logs. Without the refresh,
calling `byteSize()` on this header map will result in a bad optional access.

Testing: Added test that configures upstream logs with `envoy.http_grpc_access_log`.
Would otherwise result in a crash due to instance of `absl::bad_optional_access`

Fixes #8828

Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Matt Klein <mklein@lyft.com>

source/extensions/access_loggers/grpc/http_grpc_access_log_impl.cc

@@ -98,7 +98,9 @@ void HttpGrpcAccessLog::emitLog(const Http::HeaderMap& request_headers,
     request_properties->set_original_path(
         std::string(request_headers.EnvoyOriginalPath()->value().getStringView()));
   }
-  request_properties->set_request_headers_bytes(request_headers.byteSize().value());
+  // TODO(asraa): This causes a manual iteration over the request_headers. Instead, refresh the byte
+  // size of this HeaderMap outside the loggers and use byteSize().
+  request_properties->set_request_headers_bytes(request_headers.byteSizeInternal());
   request_properties->set_request_body_bytes(stream_info.bytesReceived());
   if (request_headers.Method() != nullptr) {
     envoy::api::v2::core::RequestMethod method =
@@ -126,6 +128,7 @@ void HttpGrpcAccessLog::emitLog(const Http::HeaderMap& request_headers,
   if (stream_info.responseCodeDetails()) {
     response_properties->set_response_code_details(stream_info.responseCodeDetails().value());
   }
+  ASSERT(response_headers.byteSize().has_value());
   response_properties->set_response_headers_bytes(response_headers.byteSize().value());
   response_properties->set_response_body_bytes(stream_info.bytesSent());
   if (!response_headers_to_log_.empty()) {

test/integration/BUILD

@@ -317,6 +317,7 @@ envoy_cc_test(
     deps = [
         ":http_integration_lib",
         "//source/common/http:header_map_lib",
+        "//source/extensions/access_loggers/grpc:http_config",
         "//source/extensions/filters/http/buffer:config",
         "//source/extensions/filters/http/dynamo:config",
         "//source/extensions/filters/http/health_check:config",

test/integration/http2_upstream_integration_test.cc

@@ -438,4 +438,48 @@ TEST_P(Http2UpstreamIntegrationTest, LargeResponseHeadersRejected) {
   EXPECT_EQ("503", response->headers().Status()->value().getStringView());
 }
 
+// Regression test to make sure that configuring upstream logs over gRPC will not crash Envoy.
+// TODO(asraa): Test output of the upstream logs.
+// See https://github.com/envoyproxy/envoy/issues/8828.
+TEST_P(Http2UpstreamIntegrationTest, ConfigureHttpOverGrpcLogs) {
+  config_helper_.addConfigModifier(
+      [&](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm)
+          -> void {
+        const std::string access_log_name =
+            TestEnvironment::temporaryPath(TestUtility::uniqueFilename());
+        // Configure just enough of an upstream access log to reference the upstream headers.
+        const std::string yaml_string = R"EOF(
+name: envoy.router
+typed_config:
+  "@type": type.googleapis.com/envoy.config.filter.http.router.v2.Router
+  upstream_log:
+    name: envoy.http_grpc_access_log
+    filter:
+      not_health_check_filter: {}
+    typed_config:
+      "@type": type.googleapis.com/envoy.config.accesslog.v2.HttpGrpcAccessLogConfig
+      common_config:
+        log_name: foo
+        grpc_service:
+          envoy_grpc:
+            cluster_name: cluster_0
+  )EOF";
+        // Replace the terminal envoy.router.
+        hcm.clear_http_filters();
+        TestUtility::loadFromYaml(yaml_string, *hcm.add_http_filters());
+      });
+
+  initialize();
+
+  // Send the request.
+  codec_client_ = makeHttpConnection(lookupPort("http"));
+  auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_);
+  waitForNextUpstreamRequest();
+
+  // Send the response headers.
+  upstream_request_->encodeHeaders(default_response_headers_, true);
+  response->waitForEndStream();
+  EXPECT_EQ("200", response->headers().Status()->value().getStringView());
+}
+
 } // namespace Envoy