Change CommandParser::parse to return StatusOr

envoy/formatter/BUILD

@@ -32,6 +32,7 @@ envoy_cc_library(
         "//envoy/server:factory_context_interface",
         "//envoy/stream_info:stream_info_interface",
         "//source/common/protobuf",
+        "@abseil-cpp//absl/status:statusor",
         "@abseil-cpp//absl/strings",
         "@abseil-cpp//absl/types:optional",
     ],

envoy/formatter/substitution_formatter.h

@@ -13,6 +13,7 @@
 
 #include "source/common/protobuf/protobuf.h"
 
+#include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
 
@@ -81,10 +82,12 @@ class CommandParser {
    * @param max_length length to which the output produced by FormatterProvider
    *                   should be truncated to (optional).
    *
-   * @return FormattterProviderPtr substitution provider for the parsed command.
+   * @return absl::StatusOr<FormatterProviderPtr> substitution provider for the parsed command or an
+   * error status.
    */
-  virtual FormatterProviderPtr parse(absl::string_view command, absl::string_view command_arg,
-                                     absl::optional<size_t> max_length) const PURE;
+  virtual absl::StatusOr<FormatterProviderPtr> parse(absl::string_view command,
+                                                     absl::string_view command_arg,
+                                                     absl::optional<size_t> max_length) const PURE;
 };
 
 using CommandParserPtr = std::unique_ptr<CommandParser>;

source/common/formatter/coalesce_formatter.cc

@@ -105,7 +105,10 @@ CoalesceFormatter::createFormatterForCommand(absl::string_view command, absl::st
                                              absl::optional<size_t> max_length) {
   // Try built-in command parsers to create the formatter.
   for (const auto& parser : BuiltInCommandParserFactoryHelper::commandParsers()) {
-    auto formatter = parser->parse(command, param, max_length);
+    absl::StatusOr<FormatterProviderPtr> formatter_result =
+        parser->parse(command, param, max_length);
+    RETURN_IF_ERROR(formatter_result.status());
+    FormatterProviderPtr formatter = std::move(formatter_result).value();
     if (formatter != nullptr) {
       return formatter;
     }

source/common/formatter/http_specific_formatter.cc

@@ -618,9 +618,9 @@ BuiltInHttpCommandParser::getKnownFormatters() {
          }}}});
 }
 
-FormatterProviderPtr BuiltInHttpCommandParser::parse(absl::string_view command,
-                                                     absl::string_view subcommand,
-                                                     absl::optional<size_t> max_length) const {
+absl::StatusOr<FormatterProviderPtr>
+BuiltInHttpCommandParser::parse(absl::string_view command, absl::string_view subcommand,
+                                absl::optional<size_t> max_length) const {
   const FormatterProviderLookupTbl& providers = getKnownFormatters();
 
   auto it = providers.find(command);

source/common/formatter/http_specific_formatter.h

@@ -249,8 +249,9 @@ class BuiltInHttpCommandParser : public CommandParser {
   BuiltInHttpCommandParser() = default;
 
   // CommandParser
-  FormatterProviderPtr parse(absl::string_view command, absl::string_view subcommand,
-                             absl::optional<size_t> max_length) const override;
+  absl::StatusOr<FormatterProviderPtr> parse(absl::string_view command,
+                                             absl::string_view subcommand,
+                                             absl::optional<size_t> max_length) const override;
 
 private:
   using FormatterProviderCreateFunc =

source/common/formatter/stream_info_formatter.cc

@@ -2600,8 +2600,9 @@ class BuiltInStreamInfoCommandParser : public CommandParser {
   BuiltInStreamInfoCommandParser() = default;
 
   // CommandParser
-  FormatterProviderPtr parse(absl::string_view command, absl::string_view sub_command,
-                             absl::optional<size_t> max_length) const override {
+  absl::StatusOr<FormatterProviderPtr> parse(absl::string_view command,
+                                             absl::string_view sub_command,
+                                             absl::optional<size_t> max_length) const override {
 
     auto it = getKnownStreamInfoFormatterProviders().find(command);
 

source/common/formatter/substitution_formatter.cc

@@ -318,7 +318,10 @@ SubstitutionFormatParser::parse(absl::string_view format,
     // First try the command parsers provided by the user. This allows the user to override
     // built-in command parsers.
     for (const auto& cmd : command_parsers) {
-      auto formatter = cmd->parse(command, command_arg, max_len);
+      absl::StatusOr<FormatterProviderPtr> formatter_result =
+          cmd->parse(command, command_arg, max_len);
+      RETURN_IF_ERROR(formatter_result.status());
+      FormatterProviderPtr formatter = std::move(formatter_result).value();
       if (formatter) {
         formatters.push_back(std::move(formatter));
         added = true;
@@ -329,7 +332,10 @@ SubstitutionFormatParser::parse(absl::string_view format,
     // Next, try the built-in command parsers.
     if (!added) {
       for (const auto& cmd : BuiltInCommandParserFactoryHelper::commandParsers()) {
-        auto formatter = cmd->parse(command, command_arg, max_len);
+        absl::StatusOr<FormatterProviderPtr> formatter_result =
+            cmd->parse(command, command_arg, max_len);
+        RETURN_IF_ERROR(formatter_result.status());
+        FormatterProviderPtr formatter = std::move(formatter_result).value();
         if (formatter) {
           formatters.push_back(std::move(formatter));
           added = true;

source/extensions/filters/http/transform/transform.cc

@@ -58,8 +58,9 @@ class BodyFormatterCommandParser : public Formatter::CommandParser {
 public:
   BodyFormatterCommandParser() = default;
 
-  Formatter::FormatterProviderPtr parse(absl::string_view command, absl::string_view command_arg,
-                                        absl::optional<size_t>) const override {
+  absl::StatusOr<Formatter::FormatterProviderPtr> parse(absl::string_view command,
+                                                        absl::string_view command_arg,
+                                                        absl::optional<size_t>) const override {
 
     if (command == "REQUEST_BODY") {
       return std::make_unique<BodyFormatterProvider>(command_arg, true);

source/extensions/filters/network/generic_proxy/access_log.cc

@@ -53,8 +53,9 @@ class GenericProxyCommandParser : public Formatter::CommandParser {
   using ProviderFuncTable = absl::flat_hash_map<std::string, ProviderFunc>;
 
   // CommandParser
-  FormatterProviderPtr parse(absl::string_view command, absl::string_view command_arg,
-                             absl::optional<size_t> max_length) const override {
+  absl::StatusOr<Formatter::FormatterProviderPtr>
+  parse(absl::string_view command, absl::string_view command_arg,
+        absl::optional<size_t> max_length) const override {
     const auto& provider_func_table = providerFuncTable();
     const auto func_iter = provider_func_table.find(std::string(command));
     if (func_iter == provider_func_table.end()) {

source/extensions/filters/udp/dns_filter/dns_filter_access_log.cc

@@ -83,8 +83,9 @@ class DnsFilterCommandParser : public Formatter::CommandParser {
   using ProviderFuncTable = absl::flat_hash_map<std::string, ProviderFunc>;
 
   // CommandParser
-  Formatter::FormatterProviderPtr parse(absl::string_view command, absl::string_view command_arg,
-                                        absl::optional<size_t> max_length) const override {
+  absl::StatusOr<Formatter::FormatterProviderPtr>
+  parse(absl::string_view command, absl::string_view command_arg,
+        absl::optional<size_t> max_length) const override {
     const auto& provider_table = providerFuncTable();
     const auto func_it = provider_table.find(std::string(command));
     if (func_it == provider_table.end()) {

source/extensions/formatter/cel/cel.cc

@@ -76,7 +76,7 @@ Protobuf::Value CELFormatter::formatValue(const Envoy::Formatter::Context& conte
   }
 }
 
-::Envoy::Formatter::FormatterProviderPtr
+absl::StatusOr<Envoy::Formatter::FormatterProviderPtr>
 CELFormatterCommandParser::parse(absl::string_view command, absl::string_view subcommand,
                                  absl::optional<size_t> max_length) const {
 #if defined(USE_CEL_PARSER)

source/extensions/formatter/cel/cel.h

@@ -33,9 +33,9 @@ class CELFormatter : public ::Envoy::Formatter::FormatterProvider {
 class CELFormatterCommandParser : public ::Envoy::Formatter::CommandParser {
 public:
   CELFormatterCommandParser() = default;
-  ::Envoy::Formatter::FormatterProviderPtr parse(absl::string_view command,
-                                                 absl::string_view subcommand,
-                                                 absl::optional<size_t> max_length) const override;
+  absl::StatusOr<Envoy::Formatter::FormatterProviderPtr>
+  parse(absl::string_view command, absl::string_view subcommand,
+        absl::optional<size_t> max_length) const override;
 };
 
 } // namespace Formatter

source/extensions/formatter/dynamic_modules/formatter.cc

@@ -104,7 +104,7 @@ DynamicModuleFormatterProvider::formatValue(const ::Envoy::Formatter::Context& c
 DynamicModuleCommandParser::DynamicModuleCommandParser(DynamicModuleFormatterConfigSharedPtr config)
     : config_(std::move(config)) {}
 
-::Envoy::Formatter::FormatterProviderPtr
+absl::StatusOr<Envoy::Formatter::FormatterProviderPtr>
 DynamicModuleCommandParser::parse(absl::string_view command, absl::string_view command_arg,
                                   absl::optional<size_t> max_length) const {
   envoy_dynamic_module_type_envoy_buffer command_buf = {.ptr = command.data(),

source/extensions/formatter/dynamic_modules/formatter.h

@@ -107,9 +107,9 @@ class DynamicModuleCommandParser : public ::Envoy::Formatter::CommandParser {
   explicit DynamicModuleCommandParser(DynamicModuleFormatterConfigSharedPtr config);
 
   // Formatter::CommandParser
-  ::Envoy::Formatter::FormatterProviderPtr parse(absl::string_view command,
-                                                 absl::string_view command_arg,
-                                                 absl::optional<size_t> max_length) const override;
+  absl::StatusOr<Envoy::Formatter::FormatterProviderPtr>
+  parse(absl::string_view command, absl::string_view command_arg,
+        absl::optional<size_t> max_length) const override;
 
 private:
   const DynamicModuleFormatterConfigSharedPtr config_;

source/extensions/formatter/file_content/config.cc

@@ -59,9 +59,9 @@ class FileContentCommandParser : public Envoy::Formatter::CommandParser {
   explicit FileContentCommandParser(Server::Configuration::ServerFactoryContext& server_context)
       : server_context_(server_context) {}
 
-  Envoy::Formatter::FormatterProviderPtr parse(absl::string_view command,
-                                               absl::string_view subcommand,
-                                               absl::optional<size_t> max_length) const override {
+  absl::StatusOr<Envoy::Formatter::FormatterProviderPtr>
+  parse(absl::string_view command, absl::string_view subcommand,
+        absl::optional<size_t> max_length) const override {
     if (command != FileContentCommand) {
       return nullptr;
     }

source/extensions/formatter/generic_secret/config.cc

@@ -68,9 +68,9 @@ class GenericSecretCommandParser : public Envoy::Formatter::CommandParser {
 
   explicit GenericSecretCommandParser(ProviderMap providers) : providers_(std::move(providers)) {}
 
-  Envoy::Formatter::FormatterProviderPtr parse(absl::string_view command,
-                                               absl::string_view subcommand,
-                                               absl::optional<size_t> max_length) const override {
+  absl::StatusOr<Envoy::Formatter::FormatterProviderPtr>
+  parse(absl::string_view command, absl::string_view subcommand,
+        absl::optional<size_t> max_length) const override {
     if (command != SecretCommand) {
       return nullptr;
     }

source/extensions/formatter/metadata/metadata.cc

@@ -136,7 +136,7 @@ const auto& formatterProviderFuncTable() {
       });
 }
 
-::Envoy::Formatter::FormatterProviderPtr
+absl::StatusOr<Envoy::Formatter::FormatterProviderPtr>
 MetadataFormatterCommandParser::parse(absl::string_view command, absl::string_view subcommand,
                                       absl::optional<size_t> max_length) const {
   if (command == "METADATA") {

source/extensions/formatter/metadata/metadata.h

@@ -16,9 +16,9 @@ namespace Formatter {
 class MetadataFormatterCommandParser : public ::Envoy::Formatter::CommandParser {
 public:
   MetadataFormatterCommandParser() = default;
-  ::Envoy::Formatter::FormatterProviderPtr parse(absl::string_view command,
-                                                 absl::string_view subcommand,
-                                                 absl::optional<size_t> max_length) const override;
+  absl::StatusOr<Envoy::Formatter::FormatterProviderPtr>
+  parse(absl::string_view command, absl::string_view subcommand,
+        absl::optional<size_t> max_length) const override;
 };
 
 } // namespace Formatter

source/extensions/formatter/req_without_query/req_without_query.cc

@@ -66,7 +66,7 @@ const Http::HeaderEntry* ReqWithoutQuery::findHeader(OptRef<const Http::HeaderMa
   return header.empty() ? nullptr : header[0];
 }
 
-::Envoy::Formatter::FormatterProviderPtr
+absl::StatusOr<Envoy::Formatter::FormatterProviderPtr>
 ReqWithoutQueryCommandParser::parse(absl::string_view command, absl::string_view subcommand,
                                     absl::optional<size_t> max_length) const {
   if (command == "REQ_WITHOUT_QUERY") {

source/extensions/formatter/req_without_query/req_without_query.h

@@ -33,9 +33,9 @@ class ReqWithoutQuery : public ::Envoy::Formatter::FormatterProvider {
 class ReqWithoutQueryCommandParser : public ::Envoy::Formatter::CommandParser {
 public:
   ReqWithoutQueryCommandParser() = default;
-  ::Envoy::Formatter::FormatterProviderPtr parse(absl::string_view command,
-                                                 absl::string_view subcommand,
-                                                 absl::optional<size_t> max_length) const override;
+  absl::StatusOr<Envoy::Formatter::FormatterProviderPtr>
+  parse(absl::string_view command, absl::string_view subcommand,
+        absl::optional<size_t> max_length) const override;
 };
 
 } // namespace Formatter

source/extensions/formatter/xfcc_value/xfcc_value.cc

@@ -215,7 +215,7 @@ class XfccValueFormatterProvider : public ::Envoy::Formatter::FormatterProvider,
   Http::LowerCaseString key_;
 };
 
-Envoy::Formatter::FormatterProviderPtr
+absl::StatusOr<Envoy::Formatter::FormatterProviderPtr>
 XfccValueFormatterCommandParser::parse(absl::string_view command, absl::string_view subcommand,
                                        absl::optional<size_t>) const {
   // Implementation for parsing the XFCC_VALUE() command.

source/extensions/formatter/xfcc_value/xfcc_value.h

@@ -16,9 +16,9 @@ namespace Formatter {
 class XfccValueFormatterCommandParser : public ::Envoy::Formatter::CommandParser {
 public:
   XfccValueFormatterCommandParser() = default;
-  Envoy::Formatter::FormatterProviderPtr parse(absl::string_view command,
-                                               absl::string_view subcommand,
-                                               absl::optional<size_t> max_length) const override;
+  absl::StatusOr<Envoy::Formatter::FormatterProviderPtr>
+  parse(absl::string_view command, absl::string_view subcommand,
+        absl::optional<size_t> max_length) const override;
 };
 
 } // namespace Formatter

test/common/formatter/BUILD

@@ -87,6 +87,7 @@ envoy_cc_test(
         "//test/mocks/server:factory_context_mocks",
         "//test/mocks/stream_info:stream_info_mocks",
         "//test/test_common:registry_lib",
+        "//test/test_common:status_utility_lib",
         "//test/test_common:test_runtime_lib",
         "//test/test_common:utility_lib",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",

test/common/formatter/command_extension.cc

@@ -15,8 +15,9 @@ Protobuf::Value TestFormatter::formatValue(const Context& context,
   return ValueUtil::stringValue(format(context, stream_info).value());
 }
 
-FormatterProviderPtr TestCommandParser::parse(absl::string_view command, absl::string_view,
-                                              absl::optional<size_t>) const {
+absl::StatusOr<FormatterProviderPtr> TestCommandParser::parse(absl::string_view command,
+                                                              absl::string_view,
+                                                              absl::optional<size_t>) const {
   if (command == "COMMAND_EXTENSION") {
     return std::make_unique<TestFormatter>();
   }
@@ -51,8 +52,9 @@ Protobuf::Value AdditionalFormatter::formatValue(const Context& context,
   return ValueUtil::stringValue(format(context, stream_info).value());
 }
 
-FormatterProviderPtr AdditionalCommandParser::parse(absl::string_view command, absl::string_view,
-                                                    absl::optional<size_t>) const {
+absl::StatusOr<FormatterProviderPtr> AdditionalCommandParser::parse(absl::string_view command,
+                                                                    absl::string_view,
+                                                                    absl::optional<size_t>) const {
   if (command == "ADDITIONAL_EXTENSION") {
     return std::make_unique<AdditionalFormatter>();
   }

test/common/formatter/command_extension.h

@@ -22,8 +22,9 @@ class TestFormatter : public FormatterProvider {
 
 class TestCommandParser : public CommandParser {
 public:
-  FormatterProviderPtr parse(absl::string_view command, absl::string_view subcommand,
-                             absl::optional<size_t> max_length) const override;
+  absl::StatusOr<FormatterProviderPtr> parse(absl::string_view command,
+                                             absl::string_view subcommand,
+                                             absl::optional<size_t> max_length) const override;
 };
 
 class TestCommandFactory : public CommandParserFactory {
@@ -48,8 +49,9 @@ class AdditionalFormatter : public FormatterProvider {
 
 class AdditionalCommandParser : public CommandParser {
 public:
-  FormatterProviderPtr parse(absl::string_view command, absl::string_view subcommand,
-                             absl::optional<size_t> max_length) const override;
+  absl::StatusOr<FormatterProviderPtr> parse(absl::string_view command,
+                                             absl::string_view subcommand,
+                                             absl::optional<size_t> max_length) const override;
 };
 
 class AdditionalCommandFactory : public CommandParserFactory {

test/common/formatter/substitution_format_string_test.cc

@@ -7,17 +7,20 @@
 #include "test/mocks/server/factory_context.h"
 #include "test/mocks/stream_info/mocks.h"
 #include "test/test_common/registry.h"
+#include "test/test_common/status_utility.h"
 #include "test/test_common/test_runtime.h"
 #include "test/test_common/utility.h"
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
-using testing::Return;
-
 namespace Envoy {
 namespace Formatter {
 
+using ::Envoy::StatusHelpers::IsOkAndHolds;
+using ::testing::NotNull;
+using ::testing::Return;
+
 class SubstitutionFormatStringUtilsTest : public ::testing::Test {
 public:
   SubstitutionFormatStringUtilsTest() {
@@ -254,8 +257,8 @@ TEST_F(SubstitutionFormatStringUtilsTest, TestParseFormattersWithSingleExtension
 
   absl::optional<size_t> max_length = {};
   ASSERT_TRUE(commands[0] != nullptr);
-  auto provider = commands[0]->parse("COMMAND_EXTENSION", "", max_length);
-  ASSERT_TRUE(provider != nullptr);
+  auto status_or_provider = commands[0]->parse("COMMAND_EXTENSION", "", max_length);
+  ASSERT_THAT(status_or_provider, IsOkAndHolds(NotNull()));
 }
 
 TEST_F(SubstitutionFormatStringUtilsTest, TestParseFormattersWithMultipleExtensions) {
@@ -293,11 +296,12 @@ TEST_F(SubstitutionFormatStringUtilsTest, TestParseFormattersWithMultipleExtensi
 
   absl::optional<size_t> max_length = {};
   ASSERT_TRUE(commands[0] != nullptr);
-  auto test_command_provider = commands[0]->parse("COMMAND_EXTENSION", "", max_length);
-  ASSERT_TRUE(test_command_provider != nullptr);
+  auto test_command_provider_or_status = commands[0]->parse("COMMAND_EXTENSION", "", max_length);
+  ASSERT_THAT(test_command_provider_or_status, IsOkAndHolds(NotNull()));
   ASSERT_TRUE(commands[1] != nullptr);
-  auto additional_command_provider = commands[1]->parse("ADDITIONAL_EXTENSION", "", max_length);
-  ASSERT_TRUE(additional_command_provider != nullptr);
+  auto additional_command_provider_or_status =
+      commands[1]->parse("ADDITIONAL_EXTENSION", "", max_length);
+  ASSERT_THAT(additional_command_provider_or_status, IsOkAndHolds(NotNull()));
 }
 
 } // namespace Formatter

test/common/formatter/substitution_formatter_test.cc

@@ -57,7 +57,7 @@ class StreamInfoFormatter : public FormatterProvider {
                       absl::optional<size_t> max_length = absl::nullopt) {
     DefaultBuiltInStreamInfoCommandParserFactory factory;
     auto parser = factory.createCommandParser();
-    formatter_ = parser->parse(command, sub_command, max_length);
+    formatter_ = parser->parse(command, sub_command, max_length).value();
     if (formatter_ == nullptr) {
       throwEnvoyExceptionOrPanic(fmt::format("Not supported command in StreamInfo: {}", command));
     }