| Summary: | r->parsed_uri->query isn't updated after execution of mod_rewrite | ||
|---|---|---|---|
| Product: | Apache httpd-2 | Reporter: | Jacques Yelloz <jacques.yelloz> |
| Component: | mod_cache | Assignee: | Apache HTTPD Bugs Mailing List <bugs> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | chip, minfrin, raphael.droz, rpluem |
| Priority: | P4 | Keywords: | FixedInTrunk |
| Version: | 2.5-HEAD | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | All | ||
| Attachments: |
Use the current path and query-string for the entity key
sample test vhost |
||
|
Description
Jacques Yelloz
2003-07-28 15:37:48 UTC
I don't think mod_rewrite (or anything else for that matter...) should need to change the parsed_uri structure. They can just make their changes to the rest of the members of the request_rec struct. mod_cache is influenced by the query-string. There are case where one needs to affect mod_cache caching by altering or even removing the query-string. The most obvious example is to forcefully cache resources like /x.css?ver=2.4 The current state of mod_rewrite/mod_cache "integration" is that mod-rewrite can't strip the query-string in a effective way: mod_cache will always consider the original query-string from key'ing the resource. I don't know about OP use-case, but in mine, the issue is clearly related to mod_cache (and could even be possibly solved by solely touching mod_cache) Switching to mod_cache rather than mod_rewrite since that's where most use-case would be found. Adding one: Trying to unify query-string (strip some keys/values, reorder, ...) in order to increase cache efficiency, is a no-go if mod_cache can't use the rewritten query-string. I didn't tested to actually revert this commit https://github.com/apache/httpd/commit/66cb25a80d251c86ece768100fd975460a3cae37 but it seems to be the root of this issue. Back in 2006, the point was to avoid resources to be stored under a key, where they cannot be fetched again in the quick handler. For example? I don't think it's a mod_cache issue. If people mess-up with URL, rewrite-rules, ... they should know what they're after. Impossibility to rewrite the query-string *before* it's used as a cache-key is a much more annoying side effect. Created attachment 34120 [details]
Use the current path and query-string for the entity key
In latest versions, the key for the current entity (request) is computed early in the (quick)handler, and reused later when referring to the entity so that rewrites on the path or the query-string don't break things.
The possible issue is that this key is computed based on the original query-string (the path is the one when the handler is called already).
This works when mod_cache is configured with "CacheQuickHandler on" (the default) since no rewrite should happen before any quick_handler, but not when CacheQuickHandler is off (and the admin wants its changes to be taken into account for caching).
This patch tries to use r->uri and r->args where appropriate (instead of immutable r->parsed_uri.path and r->parsed_uri.query) such that the entity key is computed accordingly.
It works with both trunk and 2.4.x.
Could you please try it?
Committed in r1756553 and proposed for backport to 2.4.x. Created attachment 34157 [details]
sample test vhost
Just found the time to test it with the attached configuration, but wasn't able to have the cache key takes the rewrite query string into account.
May I've overlooked something?
Side note: the patch does not seem to affect ap_log_rerror() which still logs ("cache: Caching url: %s", r->unparsed_uri);
Errata, I was not testing the patched/compiled mod_cache* module. Patch works! (as per htcacheclean -A). Thank you very much Yann! (the issue about logging the wrong key still apply though) Actual key logging addressed in r1756631 (added to backport proposal). Fixed in 2.4.25 |