Skip to content

HTTP cache support for the QUERY method (RFC 10008)#852

Open
hunghhdev wants to merge 1 commit into
apache:masterfrom
hunghhdev:master
Open

HTTP cache support for the QUERY method (RFC 10008)#852
hunghhdev wants to merge 1 commit into
apache:masterfrom
hunghhdev:master

Conversation

@hunghhdev

Copy link
Copy Markdown
Contributor

Follow-up to #840. The caching layer can now store and serve responses to QUERY requests: cache keys incorporate a digest of the request content and content type, and both classic and async interceptors buffer QUERY bodies instead of bypassing the cache. Anything that cannot be buffered safely still falls back to direct execution.

@ok2c

ok2c commented Jul 17, 2026

Copy link
Copy Markdown
Member

@hunghhdev Awesome! Out of curiosity is this effort coordinated with the original author of #840 or is it purely coincidental?

Please note there is #850 that still needs to land that improves the process of cache key generation.

Overall looks very good.

@desiderantes

desiderantes commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

@hunghhdev Awesome! Out of curiosity is this effort coordinated with the original author of #840 or is it purely coincidental?

Please note there is #850 that still needs to land that improves the process of cache key generation.

Overall looks very good.

No, this PR was not coordinated, I just saw this. If you prefer this code instead, I can stop #846.

* by {@link AsyncEntityProducer#releaseResources()} and can still be used to execute
* the request directly.
*/
private static byte[] drainEntityProducer(final AsyncEntityProducer entityProducer) throws IOException {

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.

@hunghhdev When draining the entity producer we should enforce a total cap on body length in order to avoid some "security professional" reporting it as a potential DoS vulnerability. I imagine requests with content body larger than 25K should be considered non-cacheable

}
// QUERY content must be read in full in order to determine the cache key
if (Method.QUERY.isSame(request.getMethod()) && entity.getContentEncoding() == null) {
final byte[] content = EntityUtils.toByteArray(entity);

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.

@hunghhdev Likewise, we should avoid buffering of content body larger than 25K. That also makes it necessary for the content entity to be repeatable.

@ok2c

ok2c commented Jul 18, 2026

Copy link
Copy Markdown
Member

No, this PR was not coordinated, I just saw this. If you prefer this code instead, I can stop #846.

@desiderantes This is the first time we, as a project, have multiple contributions of the same feature. I suggest we proceed with @hunghhdev contribution as it is almost complete and it is a first-time contribution. In the meantime, if you have time and inclination what also needs to be done is to extend HttpCacheEntry and the related factory and serializer to store request body along with the request head.

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.

3 participants