Hi Jan, sorry for the late.
On Tue, Apr 22, 2014 at 3:39 PM, Jan Kaluža <[email protected]> wrote: > Hi again, > > the patch has been here for some time already. I hesitate to commit it to > trunk without any review, because it changes the core code in mod_proxy and > I'm afraid that there could exist more corner-cases I'm not aware of. > > On the other side (and to motivate someone with deeper knowledge of > mod_proxy :) ), it would be great to have UDS support also for > ProxyPassMatch and fix some old bugs related to setting options together > with ProxyPassMatch (like PR 43513 and PR 54973). > > So, anyone who would have time to review this patch, please? :) I slightly modified your patch with the following changes : 1. Use \-escaping instead of %-escaping for wildcard_match so to consume less space and unescape quickly. 2. Fix the recursion in ap_proxy_wildcard_match(), which was calling the legacy ap_strcmp_match(), and unescape according to 1. 2'. Shouldn't this function be made iterative (it is not final recursive currently)? 3. Always try to get the worker with its de_socketfy()ed name, and only if not found use ap_proxy_define[_wildcard]_worker() with the configured URL (add_pass() and proxysection() were not consistent about ap_proxy_get_worker()'s given name/URL). 4. Don't match both the "normal" name with strcnmp() and the wildcard name with strcmp_match() in ap_proxy_get_worker(), since the worker is either wildcard or not (this enforcement is also added to add_pass() and proxysection() so that it is not possible to declare the same worker name once with a Match, again without, or vice versa). 4'. Does it makes sense to use the longest match_name or should we go for the first matching wildcard worker (if any) wins? 5. Rename ap_proxy_wildcard_match() as ap_proxy_strcmp_ematch() to suggest it could go to server/util.ch (if/when the associated ap_matchexp_escape() and ap_proxy_strcasecmp_ematch() are coded). 6. Move proxy_worker_shared { char wildcard_name[PROXY_WORKER_MAX_NAME_SIZE]; } to proxy_worker { char *match_name; } to save shm space and avoid length issues (I don't see how this could be updated at runtime, for now, balancer-manager can't declare Match workers). 6. Keep ap_proxy_escape_wildcard_url() static (private) since it is not used outside proxy_util (now). This is not very tested (yet), I'll come back if I detect issues, but first I'd like your feedbacks on this modifications... Patch attached. Regards, Yann.
trunk-mod_proxy-Match.patch
Description: application/download
