compressor: add weaken_etag_on_compress to weaken strong ETags when compressing#43755
Conversation
|
Hi @rafaelgaspar, 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. |
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
ba6a487 to
3bcab8f
Compare
|
cc @adisuissa Gentle ping for the review. |
|
@rafaelgaspar It also needs a main merge. /wait |
adisuissa
left a comment
There was a problem hiding this comment.
Thanks! left a couple of high-level API comments.
3bcab8f to
974d00d
Compare
|
@adisuissa both suggestions have been incorporated into the code, let me know if there is anything else. |
|
@adisuissa re-ping for "stalled" bit. |
|
This requires a compressor filter owner review. |
KBaichoo
left a comment
There was a problem hiding this comment.
looks good otherwise, thanks
|
/wait |
Introduces a new configuration option, `weaken_etag_on_compress`, to the compressor filter's response direction configuration. When enabled, strong ETag headers are modified to include a "W/" prefix instead of being removed during compression, allowing for better cache and conditional request handling. Updates to documentation and tests have been made to reflect this new behavior. Signed-off-by: Rafael Antunes <rafael.gaspar@me.com>
Updated the compressor filter to clarify the precedence of the `weaken_etag_on_compress` option when both it and `disable_on_etag_header` are enabled. Compression will now be applied, and the ETag will be weakened, allowing for better cache management. Documentation and tests have been updated to reflect this behavior. Signed-off-by: Rafael Antunes <rafael.gaspar@me.com>
Signed-off-by: Rafael Antunes <rafael.gaspar@me.com>
9f2ffb3 to
4c94362
Compare
Introduced a helper function `isWeakEtag` to streamline the logic for identifying weak ETags in the `sanitizeEtagHeader` and `weakenEtagHeader` methods. Signed-off-by: Rafael Antunes <rafael.gaspar@me.com>
Updated the initialization of the `weaken_etag_on_compress` member variable to directly use the value from the response direction configuration, simplifying the logic and ensuring consistent behavior. Signed-off-by: Rafael Antunes <rafael.gaspar@me.com>
|
@KBaichoo addressed the comments, and did a rebase. Let me know if there is anything else. |
Updated the CompressorFilter to remove strong ETags of length two during compression, aligning their treatment with longer strong ETags. Documentation has been updated to reflect this change, ensuring clarity on the behavior of the `weaken_etag_on_compress` option. Signed-off-by: Rafael Antunes <rafael.gaspar@me.com>
|
@KBaichoo done, please let me know if there is anything else. |
Signed-off-by: Rafael Antunes <rafael.gaspar@me.com>
|
@KBaichoo fixed the formatter issue, could you trigger CI again? |
…ompressing (envoyproxy#43755) **Commit Message:** compressor: add weaken_etag_on_compress to weaken strong ETags when compressing **Additional Description:** Adds a new option ``weaken_etag_on_compress`` to the compressor filter's ``response_direction_config``. When enabled, strong ``ETag`` response headers are weakened by prepending ``W/`` to the value (e.g. ``"abc123"`` becomes ``W/"abc123"``) instead of being removed when compression is applied. Weak ETags (already starting with ``W/``) are left unchanged. This allows caches and conditional requests to keep working while indicating that the representation was modified by compression, matching behavior used in other proxies (e.g. Varnish). The default remains ``false``, preserving existing behavior (strong ETags are removed when compressing). Fixes envoyproxy#35152 Signed-off-by: Rafael Antunes <rafael.gaspar@me.com> Signed-off-by: Xuyang Tao <taoxuy@google.com>
…ompressing (envoyproxy#43755) **Commit Message:** compressor: add weaken_etag_on_compress to weaken strong ETags when compressing **Additional Description:** Adds a new option ``weaken_etag_on_compress`` to the compressor filter's ``response_direction_config``. When enabled, strong ``ETag`` response headers are weakened by prepending ``W/`` to the value (e.g. ``"abc123"`` becomes ``W/"abc123"``) instead of being removed when compression is applied. Weak ETags (already starting with ``W/``) are left unchanged. This allows caches and conditional requests to keep working while indicating that the representation was modified by compression, matching behavior used in other proxies (e.g. Varnish). The default remains ``false``, preserving existing behavior (strong ETags are removed when compressing). Fixes envoyproxy#35152 Signed-off-by: Rafael Antunes <rafael.gaspar@me.com> Signed-off-by: Jonathan Wu <jtwu@google.com>
…ompressing (envoyproxy#43755) **Commit Message:** compressor: add weaken_etag_on_compress to weaken strong ETags when compressing **Additional Description:** Adds a new option ``weaken_etag_on_compress`` to the compressor filter's ``response_direction_config``. When enabled, strong ``ETag`` response headers are weakened by prepending ``W/`` to the value (e.g. ``"abc123"`` becomes ``W/"abc123"``) instead of being removed when compression is applied. Weak ETags (already starting with ``W/``) are left unchanged. This allows caches and conditional requests to keep working while indicating that the representation was modified by compression, matching behavior used in other proxies (e.g. Varnish). The default remains ``false``, preserving existing behavior (strong ETags are removed when compressing). Fixes envoyproxy#35152 Signed-off-by: Rafael Antunes <rafael.gaspar@me.com> Signed-off-by: Nick Shipilov <nick.shipilov.n@gmail.com>
…ompressing (envoyproxy#43755) **Commit Message:** compressor: add weaken_etag_on_compress to weaken strong ETags when compressing **Additional Description:** Adds a new option ``weaken_etag_on_compress`` to the compressor filter's ``response_direction_config``. When enabled, strong ``ETag`` response headers are weakened by prepending ``W/`` to the value (e.g. ``"abc123"`` becomes ``W/"abc123"``) instead of being removed when compression is applied. Weak ETags (already starting with ``W/``) are left unchanged. This allows caches and conditional requests to keep working while indicating that the representation was modified by compression, matching behavior used in other proxies (e.g. Varnish). The default remains ``false``, preserving existing behavior (strong ETags are removed when compressing). Fixes envoyproxy#35152 Signed-off-by: Rafael Antunes <rafael.gaspar@me.com>
Generative AI Disclaimer: This PR was prepared with assistance from generative AI (Cursor). The implementation, tests, and documentation have been fully reviewed by me as the PR author, and I take responsibility for the changes.
Commit Message:
compressor: add weaken_etag_on_compress to weaken strong ETags when compressing
Additional Description:
Adds a new option
weaken_etag_on_compressto the compressor filter'sresponse_direction_config. When enabled, strongETagresponse headersare weakened by prepending
W/to the value (e.g."abc123"becomesW/"abc123") instead of being removed when compression is applied. WeakETags (already starting with
W/) are left unchanged.This allows caches and conditional requests to keep working while indicating
that the representation was modified by compression, matching behavior used in
other proxies (e.g. Varnish). The default remains
false, preservingexisting behavior (strong ETags are removed when compressing).
Fixes #35152
Risk Level: Low
Testing:
New unit tests in
compressor_filter_test.cc:WeakenEtagOnCompressStrongEtag– strong ETag is weakened when compressingWeakenEtagOnCompressStrongEtagWithQuotes– strong ETag with quotes is weakenedWeakenEtagOnCompressWeakEtagUnchanged– weak ETag is preservedWeakenEtagOnCompressWithStatusHeaderEnabled– weaken behavior with status header enabledDocs Changes: Yes
docs/root/configuration/http/http_filters/compressor_filter.rstwith an "ETag handling" section describingdisable_on_etag_headerandweaken_etag_on_compress, and added a YAML example forweaken_etag_on_compress.EtagNotAllowedstatus andnot_compressed_etagstat descriptions.Release Notes: Yes
new_featuresinchangelogs/current.yamlfor the compressor filter'sweaken_etag_on_compressoption.Platform Specific Features: N/A