Re: mod_rewrite/proxy UDS issues

[Eric Covener <co...@gmail.com>]

Did this become unnecessary when SetHandler/AddHandler support was
added for proxy:unix:/ ... configuration?

On Fri, Apr 18, 2014 at 12:52 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Fri, Apr 18, 2014 at 5:57 PM, Yann Ylavic <yl...@gmail.com> wrote:
>> Patch inline.
>>
>> Index: modules/proxy/proxy_util.c
>> ===================================================================
>> --- modules/proxy/proxy_util.c    (revision 1588466)
>> +++ modules/proxy/proxy_util.c    (working copy)
>> @@ -1508,32 +1508,111 @@ PROXY_DECLARE(char *) ap_proxy_worker_name(apr_poo
> [...]
>> +PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker_ex(apr_pool_t *p,
>> +                                                     proxy_balancer *balancer,
>> +                                                     proxy_server_conf *conf,
>> +                                                     const char *url, int uds,
>> +                                                     char **to, char **by)
>> +{
> [...]
>> +    if (!uds) {
>> +        /* No real interest in *to or *by being used without uds,
>> +         * don't leave dangling pointers anyway.
>> +         */
>> +        if (to) {
>> +            *to = NULL;
>> +        }
>> +        if (by) {
>> +            *by = NULL;
>> +        }
>> +        url_length = 0;
>> +        url_copy = NULL;
>> +    }
>> +    else {
>> +        url_length = strlen(url);
>
> Not the right place to initialize url_length, will be fixed in next
> patch (if useful).
>
>> +        url_copy = apr_pstrmemdup(p, url, url_length);
>> +        url_copy = ap_proxy_split_url(p, url_copy, by);
>> +        if (to) {
>> +            *to = url_copy;
>> +        }
>> +        url = url_copy;
>> +    }
>> +
>>      c = ap_strchr_c(url, ':');
>>      if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0') {
>>          return NULL;
>>      }
>>
>> -    url_length = strlen(url);
>> -    url_copy = apr_pstrmemdup(p, url, url_length);
>> +    if (!url_copy) {
>> +        url_length = strlen(url);
>> +        url_copy = apr_pstrmemdup(p, url, url_length);
>> +    }
>>
>
> Please read something the following instead :
>
> +    if (!uds) {
> +        /* No real interest in *to or *by being used without uds,
> +         * don't leave dangling pointers anyway.
> +         */
> +        if (to) {
> +            *to = NULL;
> +        }
> +        if (by) {
> +            *by = NULL;
> +        }
> +        url_copy = NULL;
> +    }
> +    else {
> +        url_copy = ap_proxy_split_url(p, apr_pstrdup(p, url), by);
> +        if (to) {
> +            *to = url_copy;
> +        }
> +        url = url_copy;
> +    }
> +
>      c = ap_strchr_c(url, ':');
>      if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0') {
>          return NULL;
>      }
>
>      url_length = strlen(url);
> -    url_copy = apr_pstrmemdup(p, url, url_length);
> +    if (url_copy == NULL) {
> +        url_copy = apr_pstrmemdup(p, url, url_length);
> +    }



-- 
Eric Covener
covener@gmail.com

Re: mod_rewrite/proxy UDS issues

[Yann Ylavic <yl...@gmail.com>]

On Fri, Nov 28, 2014 at 1:56 PM, Eric Covener <co...@gmail.com> wrote:
> Did this become unnecessary when SetHandler/AddHandler support was
> added for proxy:unix:/ ... configuration?

I think both are orthogonals.

One can use <Proxy "unix:/tmp/backend.sock|http://localhost/...> to
declare a worker whose connections will be reusable by :
1.Set/AddHandler "unix:/tmp/backend.sock|http://localhost/..."
2. RewriteRule ... [H=unix:/tmp/backend.sock|http://localhost/...]
(supposedly)
3. RewriteRule ... "http://localhost/.." [P]
But not by :
4. RewriteRule ... "unix:/tmp/backend.sock|http://localhost/..." [P]

Yes 1. and 2. are two nice/efficient "shortcut" alternatives.
But I find 3. vs 4. quite confusing, proxying via RewriteRule is often
used (and even very useful thanks to RewriteCond decision), so we
should either document on that, or fix it.

I also find the idea of having an optional function between mod_proxy
(provider) and mod_rewrite quite useful, since there is no point in
using a [P] or [H=proxy:...] rules without mod_proxy loaded.
The failure to retrieve this proxy optional function could be detected
at init time to raise an error if any [P] or [H=proxy:...] is
configured.

After re-reading this thread, I would now rather propose that this
optional function be downright ap_proxy_declare_worker().
Thus the proxy worker would be automagically declared with the
RewriteRule at init time, and the run time could use the available
proxy_worker struct directly to do its [P] job.
Maybe that's what Jim was thinking about as a "non-obvious but elegant
solution" where "2 workers map onto the same one", back in February
25...

Regards,
Yann.