-
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
api: relax inline_string length limitation in DataSource #14461
Conversation
Signed-off-by: Lizan Zhou <[email protected]>
} | ||
if (!allow_empty && data.empty()) { |
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.
Will this start enforcing allow_empty on the kFilename
case where it didn't previously?
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.
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.
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.
Looks good
@@ -334,7 +334,7 @@ message DataSource { | |||
bytes inline_bytes = 2 [(validate.rules).bytes = {min_len: 1}]; | |||
|
|||
// String inlined in the configuration. | |||
string inline_string = 3 [(validate.rules).string = {min_len: 1}]; | |||
string inline_string = 3; |
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
?
Signed-off-by: Lizan Zhou <[email protected]>
* master: (30 commits) Deflaked: Guarddog_impl_test (envoyproxy#14475) [fuzz] add fuzz tests for hpack encoding and decoding (envoyproxy#13315) [filters] Prevent a filter from sending local reply and continue (envoyproxy#14416) oauth2: improving coverage (envoyproxy#14479) owners: Change dio email address (envoyproxy#14498) macos build: Fix ninja install (envoyproxy#14495) http: use OptRef helper to reduce some boilerplate (envoyproxy#14361) doc: update test/integration/README.md (envoyproxy#14485) server: wait workers to start before draining parent. (envoyproxy#14319) api: relax inline_string length limitation in DataSource (envoyproxy#14461) oauth: properly stop filter chain when a response was sent (envoyproxy#14476) listener: deprecate use_proxy_proto (envoyproxy#14406) deps: update cel and remove a patch (envoyproxy#14473) preconnect: rename: (envoyproxy#14474) coverage: ratcheting limits (envoyproxy#14472) grpc mux: fix sending node again after stream is reset (envoyproxy#14080) [test] Replace printers_include with printers_lib. (envoyproxy#14442) tcp: nodelay in the new pool (envoyproxy#14453) test: replace mock_methodn macros with mock_method (envoyproxy#14450) tcp: extending tcp integration test (envoyproxy#14451) ... Signed-off-by: Michael Puncel <[email protected]>
As discussed in #14276, relax the min length requirement of
inline_string
in DataSource. It should checked after DataSource since an empty file could pass validation.Risk Level: Low
Testing: integration test
Docs Changes: N/A
Release Notes: N/A
Signed-off-by: Lizan Zhou [email protected]