router: support downstream cert formatting in the header formatter

docs/root/configuration/http/http_conn_man/headers.rst

@@ -605,12 +605,18 @@ Supported variable names are:
   TCP
     The validity start date of the client certificate used to establish the downstream TLS connection.
 
+  DOWNSTREAM_PEER_CERT_V_START can be customized with specifiers as specified in
+  :ref:`access log format rules<config_access_log_format_downstream_peer_cert_v_start>`.
+
 %DOWNSTREAM_PEER_CERT_V_END%
   HTTP
     The validity end date of the client certificate used to establish the downstream TLS connection.
   TCP
     The validity end date of the client certificate used to establish the downstream TLS connection.
 
+  DOWNSTREAM_PEER_CERT_V_END can be customized with specifiers as specified in
+  :ref:`access log format rules<config_access_log_format_downstream_peer_cert_v_end>`.
+
 %HOSTNAME%
     The system hostname.
 

docs/root/version_history/current.rst

@@ -35,6 +35,7 @@ Minor Behavior Changes
   `envoy.reloadable_features.remove_forked_chromium_url`.
 * oauth filter: added the optional parameter :ref:`auth_scopes <envoy_v3_api_field_extensions.filters.http.oauth2.v3alpha.OAuth2Config.auth_scopes>` with default value of 'user' if not provided. Enables this value to be overridden in the Authorization request to the OAuth provider.
 * perf: allow reading more bytes per operation from raw sockets to improve performance.
+* router: extended custom date formatting to DOWNSTREAM_PEER_CERT_V_START and DOWNSTREAM_PEER_CERT_V_END when using :ref:`custom request/response header formats <config_http_conn_man_headers_custom_request_headers>`.
 * tcp: setting NODELAY in the base connection class. This should have no effect for TCP or HTTP proxying, but may improve throughput in other areas. This behavior can be temporarily reverted by setting `envoy.reloadable_features.always_nodelay` to false.
 * upstream: host weight changes now cause a full load balancer rebuild as opposed to happening
   atomically inline. This change has been made to support load balancer pre-computation of data

source/common/router/header_formatter.cc

@@ -43,6 +43,25 @@ std::string formatPerRequestStateParseException(absl::string_view params) {
                       params);
 }
 
+// Parses a substitution format field and returns a function that formats it.
+std::function<std::string(const Envoy::StreamInfo::StreamInfo&)>
+parseSubstitutionFormatField(absl::string_view field_name,
+                             StreamInfoHeaderFormatter::FormatterPtrMap& formatter_map) {
+  const std::string pattern = fmt::format("%{}%", field_name);
+  if (formatter_map.find(pattern) == formatter_map.end()) {
+    formatter_map.emplace(
+        std::make_pair(pattern, Formatter::FormatterPtr(new Formatter::FormatterImpl(
+                                    pattern, /*omit_empty_values=*/true))));
+  }
+  return [&formatter_map, pattern](const Envoy::StreamInfo::StreamInfo& stream_info) {
+    const auto& formatter = formatter_map.at(pattern);
+    return formatter->format(*Http::StaticEmptyHeaders::get().request_headers,
+                             *Http::StaticEmptyHeaders::get().response_headers,
+                             *Http::StaticEmptyHeaders::get().response_trailers, stream_info,
+                             absl::string_view());
+  };
+}
+
 // Parses the parameters for UPSTREAM_METADATA and returns a function suitable for accessing the
 // specified metadata from an StreamInfo::StreamInfo. Expects a string formatted as:
 //   (["a", "b", "c"])
@@ -210,21 +229,6 @@ StreamInfoHeaderFormatter::FieldExtractor sslConnectionInfoStringHeaderExtractor
   };
 }
 
-// Helper that handles the case when the desired time field is empty.
-StreamInfoHeaderFormatter::FieldExtractor sslConnectionInfoStringTimeHeaderExtractor(
-    std::function<absl::optional<SystemTime>(const Ssl::ConnectionInfo& connection_info)>
-        time_extractor) {
-  return sslConnectionInfoStringHeaderExtractor(
-      [time_extractor](const Ssl::ConnectionInfo& connection_info) {
-        absl::optional<SystemTime> time = time_extractor(connection_info);
-        if (!time.has_value()) {
-          return std::string();
-        }
-
-        return AccessLogDateTimeFormatter::fromTime(time.value());
-      });
-}
-
 } // namespace
 
 StreamInfoHeaderFormatter::StreamInfoHeaderFormatter(absl::string_view field_name, bool append)
@@ -313,16 +317,10 @@ StreamInfoHeaderFormatter::StreamInfoHeaderFormatter(absl::string_view field_nam
         sslConnectionInfoStringHeaderExtractor([](const Ssl::ConnectionInfo& connection_info) {
           return connection_info.urlEncodedPemEncodedPeerCertificate();
         });
-  } else if (field_name == "DOWNSTREAM_PEER_CERT_V_START") {
-    field_extractor_ =
-        sslConnectionInfoStringTimeHeaderExtractor([](const Ssl::ConnectionInfo& connection_info) {
-          return connection_info.validFromPeerCertificate();
-        });
-  } else if (field_name == "DOWNSTREAM_PEER_CERT_V_END") {
-    field_extractor_ =
-        sslConnectionInfoStringTimeHeaderExtractor([](const Ssl::ConnectionInfo& connection_info) {
-          return connection_info.expirationPeerCertificate();
-        });
+  } else if (absl::StartsWith(field_name, "DOWNSTREAM_PEER_CERT_V_START")) {
+    field_extractor_ = parseSubstitutionFormatField(field_name, formatter_map_);
+  } else if (absl::StartsWith(field_name, "DOWNSTREAM_PEER_CERT_V_END")) {
+    field_extractor_ = parseSubstitutionFormatField(field_name, formatter_map_);
   } else if (field_name == "UPSTREAM_REMOTE_ADDRESS") {
     field_extractor_ = [](const Envoy::StreamInfo::StreamInfo& stream_info) -> std::string {
       if (stream_info.upstreamHost()) {
@@ -331,23 +329,7 @@ StreamInfoHeaderFormatter::StreamInfoHeaderFormatter(absl::string_view field_nam
       return "";
     };
   } else if (absl::StartsWith(field_name, "START_TIME")) {
-    const std::string pattern = fmt::format("%{}%", field_name);
-    if (start_time_formatters_.find(pattern) == start_time_formatters_.end()) {
-      start_time_formatters_.emplace(
-          std::make_pair(pattern, Formatter::SubstitutionFormatParser::parse(pattern)));
-    }
-    field_extractor_ = [this, pattern](const Envoy::StreamInfo::StreamInfo& stream_info) {
-      const auto& formatters = start_time_formatters_.at(pattern);
-      std::string formatted;
-      for (const auto& formatter : formatters) {
-        const auto bit = formatter->format(*Http::StaticEmptyHeaders::get().request_headers,
-                                           *Http::StaticEmptyHeaders::get().response_headers,
-                                           *Http::StaticEmptyHeaders::get().response_trailers,
-                                           stream_info, absl::string_view());
-        absl::StrAppend(&formatted, bit.value_or("-"));
-      }
-      return formatted;
-    };
+    field_extractor_ = parseSubstitutionFormatField(field_name, formatter_map_);
   } else if (absl::StartsWith(field_name, "UPSTREAM_METADATA")) {
     field_extractor_ = parseMetadataField(field_name.substr(STATIC_STRLEN("UPSTREAM_METADATA")));
   } else if (absl::StartsWith(field_name, "DYNAMIC_METADATA")) {

source/common/router/header_formatter.h

@@ -42,12 +42,18 @@ class StreamInfoHeaderFormatter : public HeaderFormatter {
   bool append() const override { return append_; }
 
   using FieldExtractor = std::function<std::string(const Envoy::StreamInfo::StreamInfo&)>;
+  using FormatterPtrMap = absl::node_hash_map<std::string, Envoy::Formatter::FormatterPtr>;
 
 private:
   FieldExtractor field_extractor_;
   const bool append_;
-  absl::node_hash_map<std::string, std::vector<Envoy::Formatter::FormatterProviderPtr>>
-      start_time_formatters_;
+
+  // Maps a string format pattern (including field name and any command operators between
+  // parenthesis) to the list of FormatterProviderPtrs that are capable of formatting that pattern.
+  // We use a map here to make sure that we only create a single parser for a given format pattern
+  // even if it appears multiple times in the larger formatting context (e.g. it shows up multiple
+  // times in a format string).
+  FormatterPtrMap formatter_map_;
 };
 
 /**

test/common/router/header_formatter_test.cc

@@ -463,6 +463,18 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerCertVStart) {
   testFormatting(stream_info, "DOWNSTREAM_PEER_CERT_V_START", "2018-12-18T01:50:34.000Z");
 }
 
+TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerCertVStartCustom) {
+  NiceMock<Envoy::StreamInfo::MockStreamInfo> stream_info;
+  auto connection_info = std::make_shared<NiceMock<Ssl::MockConnectionInfo>>();
+  absl::Time abslStartTime =
+      TestUtility::parseTime("Dec 18 01:50:34 2018 GMT", "%b %e %H:%M:%S %Y GMT");
+  SystemTime startTime = absl::ToChronoTime(abslStartTime);
+  ON_CALL(*connection_info, validFromPeerCertificate()).WillByDefault(Return(startTime));
+  EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info));
+  testFormatting(stream_info, "DOWNSTREAM_PEER_CERT_V_START(%b %e %H:%M:%S %Y %Z)",
+                 "Dec 18 01:50:34 2018 UTC");
+}
+
 TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerCertVStartEmpty) {
   NiceMock<Envoy::StreamInfo::MockStreamInfo> stream_info;
   auto connection_info = std::make_shared<NiceMock<Ssl::MockConnectionInfo>>();
@@ -488,6 +500,18 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerCertVEnd) {
   testFormatting(stream_info, "DOWNSTREAM_PEER_CERT_V_END", "2020-12-17T01:50:34.000Z");
 }
 
+TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerCertVEndCustom) {
+  NiceMock<Envoy::StreamInfo::MockStreamInfo> stream_info;
+  auto connection_info = std::make_shared<NiceMock<Ssl::MockConnectionInfo>>();
+  absl::Time abslStartTime =
+      TestUtility::parseTime("Dec 17 01:50:34 2020 GMT", "%b %e %H:%M:%S %Y GMT");
+  SystemTime startTime = absl::ToChronoTime(abslStartTime);
+  ON_CALL(*connection_info, expirationPeerCertificate()).WillByDefault(Return(startTime));
+  EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info));
+  testFormatting(stream_info, "DOWNSTREAM_PEER_CERT_V_END(%b %e %H:%M:%S %Y %Z)",
+                 "Dec 17 01:50:34 2020 UTC");
+}
+
 TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerCertVEndEmpty) {
   NiceMock<Envoy::StreamInfo::MockStreamInfo> stream_info;
   auto connection_info = std::make_shared<NiceMock<Ssl::MockConnectionInfo>>();
@@ -502,6 +526,24 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerCertVEndNoTls)
   testFormatting(stream_info, "DOWNSTREAM_PEER_CERT_V_END", EMPTY_STRING);
 }
 
+TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithStartTime) {
+  NiceMock<Envoy::StreamInfo::MockStreamInfo> stream_info;
+  absl::Time abslStartTime =
+      TestUtility::parseTime("Dec 17 01:50:34 2020 GMT", "%b %e %H:%M:%S %Y GMT");
+  SystemTime startTime = absl::ToChronoTime(abslStartTime);
+  EXPECT_CALL(stream_info, startTime()).WillRepeatedly(Return(startTime));
+  testFormatting(stream_info, "START_TIME", "2020-12-17T01:50:34.000Z");
+}
+
+TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithStartTimeCustom) {
+  NiceMock<Envoy::StreamInfo::MockStreamInfo> stream_info;
+  absl::Time abslStartTime =
+      TestUtility::parseTime("Dec 17 01:50:34 2020 GMT", "%b %e %H:%M:%S %Y GMT");
+  SystemTime startTime = absl::ToChronoTime(abslStartTime);
+  EXPECT_CALL(stream_info, startTime()).WillRepeatedly(Return(startTime));
+  testFormatting(stream_info, "START_TIME(%b %e %H:%M:%S %Y %Z)", "Dec 17 01:50:34 2020 UTC");
+}
+
 TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithUpstreamMetadataVariable) {
   NiceMock<Envoy::StreamInfo::MockStreamInfo> stream_info;
   std::shared_ptr<NiceMock<Envoy::Upstream::MockHostDescription>> host(