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-cache] Don't bypass the cache for if-unmodified-since or if-match #33080

Merged
merged 2 commits into from
Mar 29, 2024

Conversation

ravenblackx
Copy link
Contributor

@ravenblackx ravenblackx commented Mar 22, 2024

Commit Message: [http-cache] Don't bypass the cache for if-unmodified-since or if-match
Additional Description: There's a whole section of the cache filter to do with conditional headers that's unimplemented ("if needed, implement it to this spec!"). Turns out we only need the very easy to implement subset, where the cache is supposed to ignore one of the conditional headers. I threw in the other one that's supposed to be ignored as well.
Risk Level: Very low, it's a WIP filter and an edge case use of it where before the change it does the wrong thing.
Testing: Doesn't even make sense to have a special test for this - there are an infinite number of headers the cache should ignore.
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

Signed-off-by: Raven Black <ravenblack@dropbox.com>
Signed-off-by: Raven Black <ravenblack@dropbox.com>
Copy link
Contributor

@toddmgreer toddmgreer left a comment

Choose a reason for hiding this comment

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

Thank you for this improvement!

@ravenblackx ravenblackx changed the title [http-cache] Don't bypass the catch for if-unmodified-since or if-match [http-cache] Don't bypass the cache for if-unmodified-since or if-match Mar 29, 2024
@ravenblackx ravenblackx enabled auto-merge (squash) March 29, 2024 16:59
@ravenblackx ravenblackx removed the request for review from jmarantz March 29, 2024 16:59
@ravenblackx
Copy link
Contributor Author

+ggreenway for senior stamp

Copy link
Member

@phlax phlax 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 @ravenblackx

@ravenblackx ravenblackx merged commit 8ff28fe into envoyproxy:main Mar 29, 2024
52 checks passed
alyssawilk pushed a commit to alyssawilk/envoy that referenced this pull request Apr 29, 2024
…ch (envoyproxy#33080)

* [cache] Don't bypass the cache for if-unmodified-since or if-match

---------

Signed-off-by: Raven Black <ravenblack@dropbox.com>
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.

5 participants