-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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.
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
api: relax inline_string length limitation in DataSource #14461
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,20 +15,27 @@ static constexpr uint32_t RetryCount = 1; | |
|
||
std::string read(const envoy::config::core::v3::DataSource& source, bool allow_empty, | ||
Api::Api& api) { | ||
std::string data; | ||
switch (source.specifier_case()) { | ||
case envoy::config::core::v3::DataSource::SpecifierCase::kFilename: | ||
return api.fileSystem().fileReadToEnd(source.filename()); | ||
data = api.fileSystem().fileReadToEnd(source.filename()); | ||
break; | ||
case envoy::config::core::v3::DataSource::SpecifierCase::kInlineBytes: | ||
return source.inline_bytes(); | ||
data = source.inline_bytes(); | ||
break; | ||
case envoy::config::core::v3::DataSource::SpecifierCase::kInlineString: | ||
return source.inline_string(); | ||
data = source.inline_string(); | ||
break; | ||
default: | ||
if (!allow_empty) { | ||
throw EnvoyException( | ||
fmt::format("Unexpected DataSource::specifier_case(): {}", source.specifier_case())); | ||
} | ||
return ""; | ||
} | ||
if (!allow_empty && data.empty()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this start enforcing allow_empty on the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes and this is intended in that way. I took quick look on existing use and mostly are allowing empty so this shouldn't be a big issue. |
||
throw EnvoyException("DataSource cannot be empty"); | ||
} | ||
return data; | ||
} | ||
|
||
absl::optional<std::string> getPath(const envoy::config::core::v3::DataSource& source) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also relax on
inline_bytes
?