Summary: | CacheEnable not caching Forward proxy | ||
---|---|---|---|
Product: | Apache httpd-2 | Reporter: | ryan pendergast <rpender> |
Component: | mod_cache | Assignee: | Apache HTTPD Bugs Mailing List <bugs> |
Status: | RESOLVED FIXED | ||
Severity: | blocker | Keywords: | FixedInTrunk, PatchAvailable |
Priority: | P2 | ||
Version: | 2.2-HEAD | ||
Target Milestone: | --- | ||
Hardware: | All | ||
OS: | All | ||
Attachments: |
(cache_util.c)Fix CacheEnable forward proxy when only protocol specified
unified diff for cache_util.c Fixes some issues in 'CacheEnable' and 'CacheDisable' matching and extends a bit the functionality. |
Description
ryan pendergast
2006-08-02 16:14:58 UTC
Created attachment 18694 [details]
(cache_util.c)Fix CacheEnable forward proxy when only protocol specified
my fix is in uri_meets_conditions(). One line fix.
(In reply to comment #0) I have attached a proposed fix to this problem. Should work for all protocols. Please provide a diff file of your fix for review (see also http://httpd.apache.org/dev/patches.html). Created attachment 18695 [details]
unified diff for cache_util.c
Sorry bout that - 1st time patch submitter. Here is the unified diff file for
the patch.
Please never change the assignment of bugs. (In reply to comment #4) > Created an attachment (id=18695) [edit] > unified diff for cache_util.c > > Sorry bout that - 1st time patch submitter. > Here is the unified diff file for the patch. The patch is not general enough I think. The language in the documentation leads to me think that *any* subprefix matches. That "ht", "http:", "http://", "http://www.", "http://www.apa" all match "http://www.apache.org". So the "strcasecmp(filter.hostname, url.hostname)" should be replaced by "strncasecmp(filter.hostname, url.hostname,strlen(filter.hostname))" or equivalent. Actually this check should be applied to the whole URI, not just the 'hostname' part. (In reply to comment #6) Agreed. I think the documentation could be worded a little better, and even the addition of a CacheEnableMatch could be handy. I think your suggestion is correct for the problem at hand though. Created attachment 18889 [details]
Fixes some issues in 'CacheEnable' and 'CacheDisable' matching and extends a bit the functionality.
(In reply to comment #7 and #8) > Agreed. I think the documentation could be worded a little better, and even the > addition of a CacheEnableMatch could be handy. I think your suggestion is > correct for the problem at hand though. Oh I got impatient with the matching logic and while I have not added 'CacheEnableMath' or 'CacheDisableMatch' I have rewritten and rather generalized the matching process by giving it comprehensible semantics. I found that the matching logic is passed not the URI but four distinct fields for scheme, hostname, port and path, so they are matched separately. Then I decided that the scheme should not be really prefix-matched. Among the changes: * Now a "/...." will not match a proxy URI like "http://...", and viceversa of course. * Prefix matching for hostnames does not make much sense, do I have added minimal suffix matching: "*w.example.com" will match any hostname ending in "w.example.com" and ".example.com" will match any hostname ending in ".example.com". All I have tested it a bit (I have removed the extensive debugging code from the submitted patch) and it is all nicely commented. Documented and fixed on trunk in r821333. |