Skip to content

decompressor: fixing a bug for requests with added trailers#18055

Merged
mattklein123 merged 2 commits intoenvoyproxy:mainfrom
alyssawilk:pinterest
Sep 10, 2021
Merged

decompressor: fixing a bug for requests with added trailers#18055
mattklein123 merged 2 commits intoenvoyproxy:mainfrom
alyssawilk:pinterest

Conversation

@alyssawilk
Copy link
Copy Markdown
Contributor

Risk Level: Medium: filter chain change but only (?) for added trailers
Testing: new integration test
Docs Changes: n/a
Release Notes: inline
Runtime guard: envoy.reloadable_features.fix_added_trailers

h/t to @rgs1 for the bug report, and writing the integration test!

Co-authored-by: @rgs1

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Copy link
Copy Markdown
Member

@rgs1 rgs1 left a comment

Choose a reason for hiding this comment

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

Thanks! Adding it to our build now.

@rgs1
Copy link
Copy Markdown
Member

rgs1 commented Sep 9, 2021

Oops:

RSTChecker NOTICE [current_version] Running check
RSTChecker ERROR [current_version] (docs/root/version_history/current.rst:76) Backticks should come in pairs (except for links and refs): * compressor: fix a bug where if trailers were added and a subsequent filter paused the filter chain, the request could be stalled. This behavior can be reverted by setting `envoy.reloadable_features.fix_added_trailers` to false.
RSTChecker ERROR [current_version] (docs/root/version_history/current.rst:77) Version history not in alphabetical order (compressor vs aws request signer): please check placement of line


* access log: fix ``%UPSTREAM_CLUSTER%`` when used in http upstream access logs. Previously, it was always logging as an unset value.
* access log: fix ``%UPSTREAM_CLUSTER%`` when used in http upstream access logs. Previously, it was always logging as an unset value.
* compressor: fix a bug where if trailers were added and a subsequent filter paused the filter chain, the request could be stalled. This behavior can be reverted by setting `envoy.reloadable_features.fix_added_trailers` to false.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

sort and you need two back ticks these days (see UPSTREAM_CLUSTER above)

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Copy link
Copy Markdown
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.

Sweet, thanks for the quick turnaround!

Copy link
Copy Markdown
Contributor

@snowp snowp 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 merged commit b37b726 into envoyproxy:main Sep 10, 2021
tyxia pushed a commit to tyxia/envoy that referenced this pull request Sep 21, 2021
…xy#18055)

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

Signed-off-by: alyssawilk <alyssar@chromium.org>
@alyssawilk alyssawilk deleted the pinterest branch February 28, 2022 21:25
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.

4 participants