Filter out unknown fields in ProtoApiScrubber #43222
Open
+69
−7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… for the CheckField() method to 100 to avoid stack overflows. Signed-off-by: nick <[email protected]>
… for the CheckField() method to 100 to avoid stack overflows. Signed-off-by: nick <[email protected]>
…to unknown_field_filter Signed-off-by: nick <[email protected]>
…to unknown_field_filter Signed-off-by: nick <[email protected]>
…to unknown_field_filter Signed-off-by: nick <[email protected]>
|
Hi @nickshokri, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
Signed-off-by: nickshokri <[email protected]>
…to unknown_field_filter Signed-off-by: nick <[email protected]>
…to unknown_field_filter Signed-off-by: nick <[email protected]>
adisuissa
reviewed
Jan 30, 2026
| Fix on-demand TLS selector to enforce session resumption settings. | ||
| - area: ext_authz | ||
| change: | | ||
| Fixed the gRPC ext_authz client to respect ``status_on_error`` configuration when gRPC calls fail. |
Contributor
There was a problem hiding this comment.
I think there's something wrong with the release notes. Please fix.
| if (field->kind() == Protobuf::Field_Kind_TYPE_MESSAGE || | ||
| field->kind() == Protobuf::Field_Kind_TYPE_ENUM) { | ||
| return FieldCheckResults::kPartial; | ||
| if (field_depth < 100) { |
Contributor
There was a problem hiding this comment.
High-level comment: it is not clear to me why the value of 100 is chosen as the maximal value. On a highly busy system, this value may need to be smaller, no?
Would it make sense to have this value configurable?
Signed-off-by: nickshokri <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit Message: Add an unknown field filter in the ProtoApiScrubber's CheckField() method. A max depth cap of 100 was also added to prevent stack overflow.
Additional Description: Note that AI was used for creating the unit tests
Risk Level: Low
Testing: Added unit tests and confirmed they pass
Docs Changes: Added to ProtoApiScrubber documentation the existence of an unknown field filter
Release Notes: Added section under "minor changes"
Platform Specific Features: N/A