fix(proxy-cache): follow request Cache-Control: no-cache header field
#14139
Conversation
github-actions
bot
added
plugins/proxy-cache
cherry-pick kong-ee
oowl
changed the title
cache-control header no-cache field base on rfc7234
There was a problem hiding this comment.
This fix would change the cached object format instead of only refreshing it. We keep the request body in the cache and the new code is returning before storing it.
My suggestion is to skip the fetch() call when Cache-Control: no-cache and setting the correct cache status in the signal_cache_req() call.
| set_header(conf, "X-Cache-Key", cache_key) | ||
| if conf.cache_control then | ||
| if cc["no-cache"] then | ||
| return signal_cache_req(ctx, conf, cache_key, "Bypass") |
There was a problem hiding this comment.
If we return here, we're not caching the request body.
There was a problem hiding this comment.
@locao ,
Just out of curiosity, why are we caching the request body? Used somewhere else?
There was a problem hiding this comment.
@khaled4vokalz yes, it is not used directly by this plugin, but exposed in the request ctx variable so logging plugins can use it to log full request details.
There was a problem hiding this comment.
I understand sharing it via the ctx but as we're using ngx.shared memory space to cache things, caching some object which is not used for anything seems a bit ambitious to me. There can be large request bodies from clients. That's the reason I asked. I was wondering if it'll be a good idea to remove the request_body from the cached object to save some space.
One more thing here is that the ctx we're using in the plugin itself is the plugin context that's not shared between plugins for use, rather it's only available in this plugin's lifecycle.
There was a problem hiding this comment.
I was wondering if it'll be a good idea to remove the request_body from the cached object to save some space.
Removing it now is not expected by custom plugins which can use this field. It can go from just changing the behavior to break someone's logging process completely.
But if you're using Kong Enterprise Edition, you can select redis as a caching strategy :)
the ctx we're using in the plugin itself is the plugin context
Actually, we copy the cached data to the shared context when cache is hit.
locao
changed the title
cache-control header no-cache field base on rfc7234Cache-Control: no-cache header
locao
changed the title
Cache-Control: no-cache headerCache-Control: no-cache header field
Summary
Previously, when the header field
Cache-Controlcontained the directiveno-cache, the proxy-cache plugin was not caching at all, which is not the behavior described by RFC-7234.Checklist
changelog/unreleased/kongorskip-changeloglabel added on PR if changelog is unnecessary. README.mdIssue reference
Fix #14120
KAG-6165