Skip to content

Remove C++ exceptions from SystemTimeFormatter and derived classes #45372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yanavlasov wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Remove C++ exceptions from SystemTimeFormatter and derived classes #45372

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b955be5
Remove C++ exceptions from SystemTimeFormatter and derived classes
yanavlasov Jun 1, 2026
a9ee712
address comments
yanavlasov Jun 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion source/common/formatter/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,21 +109,23 @@ envoy_cc_library(
hdrs = ["stream_info_formatter.h"],
rbe_pool = "6gig",
deps = [
":substitution_format_utility_lib",
"//envoy/api:api_interface",
"//envoy/common:exception_lib",
"//envoy/formatter:substitution_formatter_interface",
"//envoy/runtime:runtime_interface",
"//envoy/stream_info:stream_info_interface",
"//envoy/upstream:upstream_interface",
"//source/common/common:assert_lib",
"//source/common/common:random_generator_lib",
"//source/common/common:utility_lib",
"//source/common/formatter:substitution_format_utility_lib",
"//source/common/grpc:common_lib",
"//source/common/http:utility_lib",
"//source/common/json:json_loader_lib",
"//source/common/json:json_utility_lib",
"//source/common/protobuf:message_validator_lib",
"//source/common/stream_info:utility_lib",
"@abseil-cpp//absl/status",
"@abseil-cpp//absl/status:statusor",
],
alwayslink = 1, # has factory registration
Expand Down
35 changes: 22 additions & 13 deletions source/common/formatter/stream_info_formatter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "source/common/runtime/runtime_features.h"
#include "source/common/stream_info/utility.h"

#include "absl/status/statusor.h"
#include "absl/strings/str_format.h"
#include "absl/strings/str_replace.h"
#include "re2/re2.h"
Expand Down Expand Up @@ -601,15 +602,22 @@ UpstreamPeerCertVEndFormatter::UpstreamPeerCertVEndFormatter(absl::string_view f
: absl::optional<SystemTime>();
})) {}

absl::Status SystemTimeFormatter::checkConstructPreconditions(absl::string_view format) {
// Validate the input specifier here. The formatted string may be destined for a header, and
// should not contain invalid characters {NUL, LR, CF}.
if (RE2::PartialMatch(format, getSystemTimeFormatNewlinePattern())) {
return absl::InvalidArgumentError(
"Invalid header configuration. Format string contains newline.");
}
return absl::OkStatus();
}

SystemTimeFormatter::SystemTimeFormatter(absl::string_view format, TimeFieldExtractorPtr f,
bool local_time)
: date_formatter_(format, local_time), time_field_extractor_(std::move(f)),
local_time_(local_time) {
// Validate the input specifier here. The formatted string may be destined for a header, and
// should not contain invalid characters {NUL, LR, CF}.
if (re2::RE2::PartialMatch(format, getSystemTimeFormatNewlinePattern())) {
throw EnvoyException("Invalid header configuration. Format string contains newline.");
}
// Sanity checking that pre-constructor validation was not skipped.
ASSERT(checkConstructPreconditions(format).ok());
}

absl::optional<std::string>
Expand Down Expand Up @@ -2414,7 +2422,7 @@ const StreamInfoFormatterProviderLookupTable& getKnownStreamInfoFormatterProvide
{"START_TIME",
{CommandSyntaxChecker::PARAMS_OPTIONAL,
[](absl::string_view format, absl::optional<size_t>) {
return std::make_unique<SystemTimeFormatter>(
return SystemTimeFormatter::make(
format,
std::make_unique<SystemTimeFormatter::TimeFieldExtractor>(
[](const StreamInfo::StreamInfo& stream_info)
Expand All @@ -2425,7 +2433,7 @@ const StreamInfoFormatterProviderLookupTable& getKnownStreamInfoFormatterProvide
{"START_TIME_LOCAL",
{CommandSyntaxChecker::PARAMS_OPTIONAL,
[](absl::string_view format, absl::optional<size_t>) {
return std::make_unique<SystemTimeFormatter>(
return SystemTimeFormatter::make(
format,
std::make_unique<SystemTimeFormatter::TimeFieldExtractor>(
[](const StreamInfo::StreamInfo& stream_info)
Expand All @@ -2437,7 +2445,7 @@ const StreamInfoFormatterProviderLookupTable& getKnownStreamInfoFormatterProvide
{"EMIT_TIME",
{CommandSyntaxChecker::PARAMS_OPTIONAL,
[](absl::string_view format, absl::optional<size_t>) {
return std::make_unique<SystemTimeFormatter>(
return SystemTimeFormatter::make(
format,
std::make_unique<SystemTimeFormatter::TimeFieldExtractor>(
[](const StreamInfo::StreamInfo& stream_info)
Expand All @@ -2448,7 +2456,7 @@ const StreamInfoFormatterProviderLookupTable& getKnownStreamInfoFormatterProvide
{"EMIT_TIME_LOCAL",
{CommandSyntaxChecker::PARAMS_OPTIONAL,
[](absl::string_view format, absl::optional<size_t>) {
return std::make_unique<SystemTimeFormatter>(
return SystemTimeFormatter::make(
format,
std::make_unique<SystemTimeFormatter::TimeFieldExtractor>(
[](const StreamInfo::StreamInfo& stream_info)
Expand Down Expand Up @@ -2507,22 +2515,23 @@ const StreamInfoFormatterProviderLookupTable& getKnownStreamInfoFormatterProvide
{"DOWNSTREAM_PEER_CERT_V_START",
{CommandSyntaxChecker::PARAMS_OPTIONAL,
[](absl::string_view format, absl::optional<size_t>) {
return std::make_unique<DownstreamPeerCertVStartFormatter>(format);
return makeTimeFormatter<DownstreamPeerCertVStartFormatter>(
format);
}}},
{"DOWNSTREAM_PEER_CERT_V_END",
{CommandSyntaxChecker::PARAMS_OPTIONAL,
[](absl::string_view format, absl::optional<size_t>) {
return std::make_unique<DownstreamPeerCertVEndFormatter>(format);
return makeTimeFormatter<DownstreamPeerCertVEndFormatter>(format);
}}},
{"UPSTREAM_PEER_CERT_V_START",
{CommandSyntaxChecker::PARAMS_OPTIONAL,
[](absl::string_view format, absl::optional<size_t>) {
return std::make_unique<UpstreamPeerCertVStartFormatter>(format);
return makeTimeFormatter<UpstreamPeerCertVStartFormatter>(format);
}}},
{"UPSTREAM_PEER_CERT_V_END",
{CommandSyntaxChecker::PARAMS_OPTIONAL,
[](absl::string_view format, absl::optional<size_t>) {
return std::make_unique<UpstreamPeerCertVEndFormatter>(format);
return makeTimeFormatter<UpstreamPeerCertVEndFormatter>(format);
}}},
{"ENVIRONMENT",
{CommandSyntaxChecker::PARAMS_REQUIRED |
Expand Down
63 changes: 57 additions & 6 deletions source/common/formatter/stream_info_formatter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,18 @@
#include <memory>
#include <regex>
#include <string>
#include <type_traits>
#include <vector>

#include "envoy/common/exception.h"
#include "envoy/formatter/substitution_formatter.h"
#include "envoy/stream_info/stream_info.h"

#include "source/common/common/utility.h"
#include "source/common/formatter/substitution_format_utility.h"

#include "absl/container/flat_hash_map.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/types/optional.h"

Expand Down Expand Up @@ -220,7 +223,12 @@ class SystemTimeFormatter : public StreamInfoFormatterProvider {
std::function<absl::optional<SystemTime>(const StreamInfo::StreamInfo& stream_info)>;
using TimeFieldExtractorPtr = std::unique_ptr<TimeFieldExtractor>;

SystemTimeFormatter(absl::string_view format, TimeFieldExtractorPtr f, bool local_time = false);
static absl::StatusOr<std::unique_ptr<SystemTimeFormatter>>
make(absl::string_view format, TimeFieldExtractorPtr&& f, bool local_time = false) {
RETURN_IF_NOT_OK(checkConstructPreconditions(format));
return std::unique_ptr<SystemTimeFormatter>(
new SystemTimeFormatter(format, std::move(f), local_time));
}

// StreamInfoFormatterProvider
// Don't hide the other structure of format and formatValue.
Expand All @@ -229,6 +237,10 @@ class SystemTimeFormatter : public StreamInfoFormatterProvider {
absl::optional<std::string> format(const StreamInfo::StreamInfo&) const override;
Protobuf::Value formatValue(const StreamInfo::StreamInfo&) const override;

protected:
SystemTimeFormatter(absl::string_view format, TimeFieldExtractorPtr f, bool local_time = false);
static absl::Status checkConstructPreconditions(absl::string_view format);

private:
const Envoy::DateFormatter date_formatter_;
const TimeFieldExtractorPtr time_field_extractor_;
Expand All @@ -240,46 +252,85 @@ class SystemTimeFormatter : public StreamInfoFormatterProvider {
* SystemTimeFormatter (FormatterProvider) for request start time from StreamInfo.
*/
class StartTimeFormatter : public SystemTimeFormatter {
public:
protected:
StartTimeFormatter(absl::string_view format);
static absl::Status checkConstructPreconditions(absl::string_view format) {
return SystemTimeFormatter::checkConstructPreconditions(format);
}
template <typename U>
friend absl::StatusOr<std::unique_ptr<U>> makeTimeFormatter(absl::string_view format);
};

/**
* SystemTimeFormatter (FormatterProvider) for downstream cert start time from the StreamInfo's
* ConnectionInfo.
*/
class DownstreamPeerCertVStartFormatter : public SystemTimeFormatter {
public:
protected:
DownstreamPeerCertVStartFormatter(absl::string_view format);
static absl::Status checkConstructPreconditions(absl::string_view format) {
return SystemTimeFormatter::checkConstructPreconditions(format);
}
template <typename U>
friend absl::StatusOr<std::unique_ptr<U>> makeTimeFormatter(absl::string_view format);
};

/**
* SystemTimeFormatter (FormatterProvider) for downstream cert end time from the StreamInfo's
* ConnectionInfo.
*/
class DownstreamPeerCertVEndFormatter : public SystemTimeFormatter {
public:
protected:
DownstreamPeerCertVEndFormatter(absl::string_view format);
static absl::Status checkConstructPreconditions(absl::string_view format) {
return SystemTimeFormatter::checkConstructPreconditions(format);
}
template <typename U>
friend absl::StatusOr<std::unique_ptr<U>> makeTimeFormatter(absl::string_view format);
};

/**
* SystemTimeFormatter (FormatterProvider) for upstream cert start time from the StreamInfo's
* upstreamInfo.
*/
class UpstreamPeerCertVStartFormatter : public SystemTimeFormatter {
public:
protected:
UpstreamPeerCertVStartFormatter(absl::string_view format);
static absl::Status checkConstructPreconditions(absl::string_view format) {
return SystemTimeFormatter::checkConstructPreconditions(format);
}
template <typename U>
friend absl::StatusOr<std::unique_ptr<U>> makeTimeFormatter(absl::string_view format);
};

/**
* SystemTimeFormatter (FormatterProvider) for upstream cert end time from the StreamInfo's
* upstreamInfo.
*/
class UpstreamPeerCertVEndFormatter : public SystemTimeFormatter {
public:
protected:
UpstreamPeerCertVEndFormatter(absl::string_view format);
static absl::Status checkConstructPreconditions(absl::string_view format) {
return SystemTimeFormatter::checkConstructPreconditions(format);
}
template <typename U>
friend absl::StatusOr<std::unique_ptr<U>> makeTimeFormatter(absl::string_view format);
};

/**
* Factory method for creating an object of type derived from SystemTimeFormatter
* The method first checks constructor preconditions are satisfied. If not the method
* return an error.
* Otherwise it returns unique_ptr with an object.
*/
template <typename T>
absl::StatusOr<std::unique_ptr<T>> makeTimeFormatter(absl::string_view format) {
static_assert(std::is_base_of<SystemTimeFormatter, T>::value,
"T must be derived from SystemTimeFormatter");
RETURN_IF_NOT_OK(T::checkConstructPreconditions(format));
Copy link
Copy Markdown
Member

@wbpcode wbpcode Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we need to add new checkConstructPreconditions() to every derived class because seems they are completely same?

Copy link
Copy Markdown
Contributor Author

@yanavlasov yanavlasov Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I have removed them.

return std::unique_ptr<T>(new T(format));
}

/**
* FormatterProvider for environment. If no valid environment value then
*/
Expand Down
Loading
Loading