Skip to content
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

http: 100-continue is case insensitive in Expect header #20603

Merged
merged 6 commits into from
Mar 31, 2022

Conversation

lambdai
Copy link
Contributor

@lambdai lambdai commented Mar 31, 2022

Commit Message:
Fixed a bug where 100-continue comparison in the Expect header field was case sensitive.

Signed-off-by: Yuchen Dai silentdai@gmail.com

Additional Description:
Risk Level: LOW
Testing: modified the "100-continue" string literal containing both upper case and lower case
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA] Fix #20602
[Optional Deprecated:]
[Optional API Considerations:]

lambdai added 2 commits March 30, 2022 22:29
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
daixiang0
daixiang0 previously approved these changes Mar 31, 2022
Copy link
Member

@daixiang0 daixiang0 left a comment

Choose a reason for hiding this comment

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

Good catch, lgtm!

adisuissa
adisuissa previously approved these changes Mar 31, 2022
Copy link
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!
/assign @alyssawilk

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
@lambdai lambdai dismissed stale reviews from adisuissa and daixiang0 via e536a14 March 31, 2022 17:39
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Copy link
Contributor

@alyssawilk alyssawilk left a comment

Choose a reason for hiding this comment

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

one little nit but otherwise LGTM, and thanks for the fix! I'll throw it over to Matt for !google review
/wait

@@ -54,7 +54,7 @@ Bug Fixes
* data plane: fixing error handling where writing to a socket failed while under the stack of processing. This should genreally affect HTTP/3. This behavioral change can be reverted by setting ``envoy.reloadable_features.allow_upstream_inline_write`` to false.
* eds: fix the eds cluster update by allowing update on the locality of the cluster endpoints. This behavioral change can be temporarily reverted by setting runtime guard ``envoy.reloadable_features.support_locality_update_on_eds_cluster_endpoints`` to false.
* http: fixed a bug where %RESPONSE_CODE_DETAILS% was not set correctly in :ref:`request_headers_to_add <envoy_v3_api_field_config.route.v3.RouteConfiguration.request_headers_to_add>`.
* http: fixed a bug where ``100-continue`` comparison in the ``Expect`` header field was case sensitive.
* http: fixed a bug where ``100-continue`` comparison in the ``Expect`` request header field was case sensitive. This RFC compliance behavior can be temporarily reverted by setting runtime guard ``envoy.reloadable_features.http_100_continue_case_insensitive`` to false.
Copy link
Contributor

Choose a reason for hiding this comment

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

compliance -> compliant change to

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. Thanks!

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks!

@mattklein123 mattklein123 enabled auto-merge (squash) March 31, 2022 22:21
@mattklein123 mattklein123 merged commit 7915198 into envoyproxy:main Mar 31, 2022
@lambdai lambdai deleted the 100contcase branch April 1, 2022 00:11
ravenblackx pushed a commit to ravenblackx/envoy that referenced this pull request Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

100-continue should be case insensitive per rfc 7231
5 participants