httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan Kaluža <jkal...@redhat.com>
Subject Re: [PATCH] Fix settings options with ProxyPassMatch
Date Thu, 10 Apr 2014 08:25:10 GMT
Hi,

can you please check the attached patch? I think it should be closer to 
the proper way how to fix this than the previous one.

It works properly for me even for URLs with '*' and '?'.

Regards,
Jan Kaluza

On 03/21/2014 03:27 PM, Yann Ylavic wrote:
> Yes, selecting the right worker is all in ap_proxy_get_worker(), but
> probably also add_pass() and proxysection() would need something like
> ap_proxy_define_wildcard_worker() to register this kind of worker
> (save the original name, ...).
>
> On Fri, Mar 21, 2014 at 3:15 PM, Jim Jagielski <jim@jagunet.com> wrote:
>> Just a thought, but wouldn't the better place to "fix" this
>> be in ap_proxy_get_worker()??
>>
>> On Mar 21, 2014, at 9:13 AM, Yann Ylavic <ylavic.dev@gmail.com> wrote:
>>
>>> oups, sorry for the numbering.
>>>
>>> On Fri, Mar 21, 2014 at 2:11 PM, Yann Ylavic <ylavic.dev@gmail.com> wrote:
>>>> On Wed, Mar 19, 2014 at 9:59 AM, Jan Kaluža <jkaluza@redhat.com> wrote:
>>>>> On 03/18/2014 02:46 PM, Yann Ylavic wrote:
>>>>>>
>>>>>> On Tue, Mar 18, 2014 at 2:38 PM, Yann Ylavic <ylavic.dev@gmail.com>
wrote:
>>>>>>>
>>>>>>> Wouldn't it be possible to define wildcard workers when the URL
is
>>>>>>> known to be a regexp substitution?
>>>>>>> For these workers' URLs, the dollars (plus the following digit)
could
>>>>>>> be replaced by a wildcard (ie. *) and ap_proxy_get_worker() could
then
>>>>>>> use ap_strcasecmp_match() against the requested URL.
>>>>>>
>>>>>>
>>>>>> I meant ap_strcmp_match(), this has to be case sensitive...
>>>>>>
>>>>>
>>>>> I've implemented your idea. Can you check the attached patch please?
It
>>>>> fixes the original PR and also ProxyPassMatch with UDS for me.
>>>>>
>>>>> If it's OK, I will commit it.
>>>>>
>>>>
>>>> I took a deeper look into this but I'm afraid it is a more complex
>>>> story actually...
>>>>
>>>> 1. Workers can be defined in <Proxy[Match] ...> sections too.
>>>> The same should probably be done in proxysection().
>>>>
>>>> 2. Both '*' and '?' are legitimate URL chars.
>>>> We need a way to escape the original ones in the configured worker's
>>>> URL, but ap_strcmp_match doesn't handle (un)escaping.
>>>> So maybe new ap_matchexp_[un]escape() and ap_str[case]cmp_ematch()
>>>> functions should be implemented.
>>>>
>>>> 5. When no $ is used in the worker's URL, an implicit $1 is appended
>>>> by proxy_trans().
>>>> Hence
>>>>    ProxyPassMatch /some/path/with(/capture) http://some.host
>>>> is equivalent to
>>>>    ProxyPassMatch /some/path/with(/capture) http://some.host$1
>>>> So an implicit '*' should be appended to the worker's wildcard URL in
>>>> the first case (maybe only when the match has a capture but this is
>>>> just an optimization, * would match empty).
>>>>
>>>> 3. I think we should mark the worker as wildcard when it is defined by
>>>> ProxyPassMatch or a <ProxyMatch> section.
>>>> So that ap_proxy_get_worker() won't use the costly ap_strcmp_match()
>>>> for "normal" workers, and won't either trigger false positives due to
>>>> '*' or '?' in the configured URL (which is not escaped).
>>>>
>>>> 4. What about the same URL used both with ProxyPass and
>>>> ProxyPassMatch, same worker or different ones?
>>>> IMHO, they should be different workers, and by rewritting the URL to a
>>>> wildcard one as done in the patch they will be, but we lose the
>>>> configured name, and eg. the balancer-manager's param "w" would now
>>>> work only with the rewritten name (which is, at least, non intuitive).
>>>> A new balancer-manager's param could be added to access wildcard
>>>> workers (eg. "ww"), since they are different they may be accessed
>>>> separately, hence I think we need to save the original URL for such
>>>> case(s).
>>>> So maybe for point 2.'s mark we could use the configured (original)
>>>> URL as a new member of the proxy_worker struct (I don't see any reason
>>>> for it to be shared in proxy_worker_shared but one may object...).
>>>> This new field would be NULL for "normal" workers.
>>>>
>>>> 5. What about wildcard balancers (ProxyPassMatch /foo(/.*)
>>>> balancer://mycluster/bar$1)?
>>>> Since the balancers are registered/selected solely by their names
>>>> (path and everything after is ignored), and then their workers are
>>>> selected according to the balancer's method (the path still does not
>>>> matter), I think there is no matching issue here (the requested path,
>>>> eventually rewritten by proxy_trans(), will finally be appended to the
>>>> worker's URL as is).
>>>> Hence the same balancer's URL used both in ProxyPass and
>>>> ProxyPassMatch can refer to the same balancer (IMHO).
>>>> Nothing to be done therefore, but I may be missing something...
>>>>
>>>> 6. ProxyPassReverseMatch would be welcome too, but that's probably
>>>> another story...
>>>>
>>>> Regards,
>>>> Yann.
>>>
>>


Mime
View raw message