formatter: removing exceptions

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
alyssawilk · Jun 6, 2024 · 20ef982 · 20ef982
1 parent db9d7ea
commit 20ef982
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 14 deletions.
diff --git a/source/common/formatter/http_specific_formatter.cc b/source/common/formatter/http_specific_formatter.cc
@@ -336,7 +336,8 @@ HttpBuiltInCommandParser::getKnownFormatters() {
          [](const std::string& format, absl::optional<size_t>& max_length) {
            std::string main_header, alternative_header;
 
-           SubstitutionFormatUtils::parseSubcommandHeaders(format, main_header, alternative_header);
+           THROW_IF_NOT_OK(SubstitutionFormatUtils::parseSubcommandHeaders(format, main_header,
+                                                                           alternative_header));
 
            return std::make_unique<RequestHeaderFormatter>(main_header, alternative_header,
                                                            max_length);
@@ -346,7 +347,8 @@ HttpBuiltInCommandParser::getKnownFormatters() {
          [](const std::string& format, absl::optional<size_t>& max_length) {
            std::string main_header, alternative_header;
 
-           SubstitutionFormatUtils::parseSubcommandHeaders(format, main_header, alternative_header);
+           THROW_IF_NOT_OK(SubstitutionFormatUtils::parseSubcommandHeaders(format, main_header,
+                                                                           alternative_header));
 
            return std::make_unique<ResponseHeaderFormatter>(main_header, alternative_header,
                                                             max_length);
@@ -356,7 +358,8 @@ HttpBuiltInCommandParser::getKnownFormatters() {
          [](const std::string& format, absl::optional<size_t>& max_length) {
            std::string main_header, alternative_header;
 
-           SubstitutionFormatUtils::parseSubcommandHeaders(format, main_header, alternative_header);
+           THROW_IF_NOT_OK(SubstitutionFormatUtils::parseSubcommandHeaders(format, main_header,
+                                                                           alternative_header));
 
            return std::make_unique<ResponseTrailerFormatter>(main_header, alternative_header,
                                                              max_length);
@@ -405,7 +408,8 @@ HttpBuiltInCommandParser::getKnownFormatters() {
         {CommandSyntaxChecker::PARAMS_REQUIRED | CommandSyntaxChecker::LENGTH_ALLOWED,
          [](const std::string& format, absl::optional<size_t>& max_length) {
            std::string main_header, alternative_header;
-           SubstitutionFormatUtils::parseSubcommandHeaders(format, main_header, alternative_header);
+           THROW_IF_NOT_OK(SubstitutionFormatUtils::parseSubcommandHeaders(format, main_header,
+                                                                           alternative_header));
 
            return std::make_unique<RequestHeaderFormatter>(main_header, alternative_header,
                                                            max_length);

diff --git a/source/common/formatter/substitution_format_utility.cc b/source/common/formatter/substitution_format_utility.cc
@@ -100,15 +100,15 @@ absl::string_view SubstitutionFormatUtils::truncateStringView(absl::string_view
   return str.substr(0, max_length.value());
 }
 
-void SubstitutionFormatUtils::parseSubcommandHeaders(const std::string& subcommand,
-                                                     std::string& main_header,
-                                                     std::string& alternative_header) {
+absl::Status SubstitutionFormatUtils::parseSubcommandHeaders(const std::string& subcommand,
+                                                             std::string& main_header,
+                                                             std::string& alternative_header) {
   // subs is used only to check if there are more than 2 headers separated by '?'.
   std::vector<std::string> subs;
   alternative_header = "";
   parseSubcommand(subcommand, '?', main_header, alternative_header, subs);
   if (!subs.empty()) {
-    throwEnvoyExceptionOrPanic(
+    return absl::InvalidArgumentError(
         // Header format rules support only one alternative header.
         // docs/root/configuration/observability/access_log/access_log.rst#format-rules
         absl::StrCat("More than 1 alternative header specified in token: ", subcommand));
@@ -117,17 +117,18 @@ void SubstitutionFormatUtils::parseSubcommandHeaders(const std::string& subcomma
   if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.consistent_header_validation")) {
     if (!Http::HeaderUtility::headerNameIsValid(absl::AsciiStrToLower(main_header)) ||
         !Http::HeaderUtility::headerNameIsValid(absl::AsciiStrToLower(alternative_header))) {
-      throwEnvoyExceptionOrPanic(
+      return absl::InvalidArgumentError(
           "Invalid header configuration. Format string contains invalid characters.");
     }
   } else {
     // The main and alternative header should not contain invalid characters {NUL, LR, CF}.
     if (!Envoy::Http::validHeaderString(main_header) ||
         !Envoy::Http::validHeaderString(alternative_header)) {
-      throwEnvoyExceptionOrPanic(
+      return absl::InvalidArgumentError(
           "Invalid header configuration. Format string contains null or newline.");
     }
   }
+  return absl::OkStatus();
 }
 
 } // namespace Formatter

diff --git a/source/common/formatter/substitution_format_utility.h b/source/common/formatter/substitution_format_utility.h
@@ -67,8 +67,9 @@ class SubstitutionFormatUtils {
    * See doc:
    * https://envoyproxy.io/docs/envoy/latest/configuration/observability/access_log/access_log#format-rules
    */
-  static void parseSubcommandHeaders(const std::string& subcommand, std::string& main_header,
-                                     std::string& alternative_header);
+  static absl::Status parseSubcommandHeaders(const std::string& subcommand,
+                                             std::string& main_header,
+                                             std::string& alternative_header);
 
   /* Variadic function template to parse the
      subcommand and assign found tokens to sequence of params.

diff --git a/source/extensions/formatter/req_without_query/req_without_query.cc b/source/extensions/formatter/req_without_query/req_without_query.cc
@@ -71,8 +71,8 @@ ReqWithoutQueryCommandParser::parse(const std::string& command, const std::strin
   if (command == "REQ_WITHOUT_QUERY") {
     std::string main_header, alternative_header;
 
-    Envoy::Formatter::SubstitutionFormatUtils::parseSubcommandHeaders(subcommand, main_header,
-                                                                      alternative_header);
+    THROW_IF_NOT_OK(Envoy::Formatter::SubstitutionFormatUtils::parseSubcommandHeaders(
+        subcommand, main_header, alternative_header));
     return std::make_unique<ReqWithoutQuery>(main_header, alternative_header, max_length);
   }