headers: consistent validation #34205
Conversation
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
| // Reject key if it is an invalid header string | ||
| return {}; | ||
| if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.consistent_header_validation")) { | ||
| if (!Http::HeaderUtility::headerValueIsValid(str)) { |
There was a problem hiding this comment.
The intention here is checking the header key/name . So I think we should use utility headerNameIsValid instead?
There was a problem hiding this comment.
yeah good catch - I'd been going off the bug which I think referenced the wrong helper. looks like same thing with the formatter.
There was a problem hiding this comment.
Yeah, tracking the code into here , we can see formatter is also using header key/name
| Http::TestRequestHeaderMapImpl header_map{{"referer", "dogs.com"}}; | ||
| Protobuf::Arena arena; | ||
| HeadersWrapper<Http::RequestHeaderMap> headers(arena, &header_map); | ||
| auto header = headers[CelValue::CreateStringView("dogs.com\n")]; |
There was a problem hiding this comment.
We probably should test a invalid header key/name in particular.
There was a problem hiding this comment.
so this does exercise the test code. did you just prefer an invalid string that looks like a key? PTAL
There was a problem hiding this comment.
Yeah, i was preferring a invalid string that looks like a key. Thanks for updates.
| @@ -52,6 +52,10 @@ minor_behavior_changes: | |||
| ``%UPSTREAM_REMOTE_PORT%`` and ``%UPSTREAM_REMOTE_ADDRESS_WITHOUT_PORT%`` access log format specifiers. | |||
| This behavior can be reverted by setting the runtime guard | |||
| ``envoy.reloadable_features.upstream_remote_address_use_connection`` to false. | |||
| - area: http | |||
| change: | | |||
| Changing header validation checks in the substitution format utility and CEL code to do RCF complaint header validation. | |||
There was a problem hiding this comment.
| Changing header validation checks in the substitution format utility and CEL code to do RCF complaint header validation. | |
| Changing header validation checks in the substitution format utility and CEL code to do RFC complaint header validation. |
|
Needs fixing merge conflict. /wait |
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
|
/retest |
|
/wait on CI |
|
/wait on CI |
|
/wait on CI and debug cleanup |
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Risk Level: low
Testing: existing tests
Docs Changes: n/a
Release Notes: inline
[Optional Runtime guard:] yes