You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2008/01/29 21:43:54 UTC

Re: svn commit: r616335 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c


On 01/29/2008 03:26 PM, jim@apache.org wrote:
> Author: jim
> Date: Tue Jan 29 06:26:20 2008
> New Revision: 616335
> 
> URL: http://svn.apache.org/viewvc?rev=616335&view=rev
> Log:
> Now let things like
> 
>    ProxyPassReverse /foo balancer://bar
> 
> work "as expected" :) :)

I guess it may not in the case of members with the schemes
ajp:// or fcgi:// :-).


> 
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/proxy/proxy_util.c
> 

> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=616335&r1=616334&r2=616335&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Tue Jan 29 06:26:20 2008
> @@ -1063,8 +1063,54 @@
>          ent = (struct proxy_alias *)conf->raliases->elts;
>      }
>      for (i = 0; i < conf->raliases->nelts; i++) {
> -        l2 = strlen(ent[i].real);
> -        if (l1 >= l2 && strncasecmp(ent[i].real, url, l2) == 0) {
> +        proxy_server_conf *sconf = (proxy_server_conf *)
> +            ap_get_module_config(r->server->module_config, &proxy_module);
> +        proxy_balancer *balancer;
> +        const char *real;
> +        real = ent[i].real;
> +        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
> +                         "ppr: real: %s", real);
> +        /*
> +         * First check if mapping against a balancer and see
> +         * if we have such a entity. If so, then we need to
> +         * find the particulars of the actual worker which may
> +         * or may not be the right one... basically, we need
> +         * to find which member actually handled this request.
> +         */
> +        if ((strncasecmp(real, "balancer:", 9) == 0) &&
> +            (balancer = ap_proxy_get_balancer(r->pool, sconf, real))) {
> +            int n;
> +            proxy_worker *worker;
> +            worker = (proxy_worker *)balancer->workers->elts;
> +            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
> +                         "ppr: checking balancer: %s",
> +                         balancer->name);
> +            for (n = 0; n < balancer->workers->nelts; n++) {
> +                if (worker->port) {
> +                    u = apr_psprintf(r->pool, "%s://%s:%d/", worker->scheme,
> +                                     worker->hostname, worker->port);
> +                }
> +                else {
> +                    u = apr_psprintf(r->pool, "%s://%s/", worker->scheme,
> +                                     worker->hostname);
> +                }
> +                ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
> +                         "ppr: matching member (%s) and URL (%s)",
> +                         u, url);
> +
> +                l2 = strlen(u);
> +                if (l1 >= l2 && strncasecmp(u, url, l2) == 0) {
> +                    u = apr_pstrcat(r->pool, ent[i].fake, &url[l2], NULL);
> +                    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
> +                         "ppr: matched member (%s)", u);
> +                    return ap_construct_url(r->pool, u, r);
> +                }
> +                worker++;
> +            }

I guess we can shortcut here by


              return url;

It seems unlikely that a backend server redirects to balancer://

> +        }
> +
> +        l2 = strlen(real);
> +        if (l1 >= l2 && strncasecmp(real, url, l2) == 0) {
>              u = apr_pstrcat(r->pool, ent[i].fake, &url[l2], NULL);
>              return ap_construct_url(r->pool, u, r);
>          }


Regards

Rüdiger





Re: svn commit: r616335 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 01/29/2008 10:23 PM, Jim Jagielski wrote:
> 
> On Jan 29, 2008, at 4:09 PM, Ruediger Pluem wrote:
> 
>> Oops. My fault. We need to continue in the loop, but I guess we can do
>> the following to avoid an unneeded strncasecmp
>>
> 
> Huh? If real isn't a balancer, then we drop to the orig
> impl. If it is, then we try to find the right worker
> and, when we find it, we match and return. If we
> are a balancer but don't find a worker, we drop through
> anyway. I guess your point is that you don't like
> the drop through on the found-balancer-but-didn't-find-
> worker case... is that right? The reason I didn't is

Exactly.

> that I didn't want to too much overload the idea
> of special schemes. Treat balancer: special for
> what we know, but even if we don't find a member,
> continue on as we would not matter what the
> scheme :)

Ok, thats fine with me.

Regards

Rüdiger



Re: svn commit: r616335 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
On Jan 29, 2008, at 4:09 PM, Ruediger Pluem wrote:

> Oops. My fault. We need to continue in the loop, but I guess we can do
> the following to avoid an unneeded strncasecmp
>

Huh? If real isn't a balancer, then we drop to the orig
impl. If it is, then we try to find the right worker
and, when we find it, we match and return. If we
are a balancer but don't find a worker, we drop through
anyway. I guess your point is that you don't like
the drop through on the found-balancer-but-didn't-find-
worker case... is that right? The reason I didn't is
that I didn't want to too much overload the idea
of special schemes. Treat balancer: special for
what we know, but even if we don't find a member,
continue on as we would not matter what the
scheme :)

Re: svn commit: r616335 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 01/29/2008 09:59 PM, Jim Jagielski wrote:
> 
> On Jan 29, 2008, at 3:43 PM, Ruediger Pluem wrote:
> 
>>
>>
>> On 01/29/2008 03:26 PM, jim@apache.org wrote:
>>> Author: jim
>>> Date: Tue Jan 29 06:26:20 2008
>>> New Revision: 616335
>>>
>>> URL: http://svn.apache.org/viewvc?rev=616335&view=rev
>>> Log:
>>> Now let things like
>>>
>>>    ProxyPassReverse /foo balancer://bar
>>>
>>> work "as expected" :) :)
>>
>> I guess it may not in the case of members with the schemes
>> ajp:// or fcgi:// :-).
>>
>>
> 
> :)
> 
> Well, if that's how they construct their Location headers :)
> 
>> I guess we can shortcut here by
>>
>>
>>               return url;

Oops. My fault. We need to continue in the loop, but I guess we can do
the following to avoid an unneeded strncasecmp

Index: proxy_util.c
===================================================================
--- proxy_util.c        (Revision 616517)
+++ proxy_util.c        (Arbeitskopie)
@@ -1108,11 +1108,12 @@
                 worker++;
             }
         }
-
-        l2 = strlen(real);
-        if (l1 >= l2 && strncasecmp(real, url, l2) == 0) {
-            u = apr_pstrcat(r->pool, ent[i].fake, &url[l2], NULL);
-            return ap_construct_url(r->pool, u, r);
+        else {
+            l2 = strlen(real);
+            if (l1 >= l2 && strncasecmp(real, url, l2) == 0) {
+                u = apr_pstrcat(r->pool, ent[i].fake, &url[l2], NULL);
+                return ap_construct_url(r->pool, u, r);
+            }
         }
     }

or simply

Index: proxy_util.c
===================================================================
--- proxy_util.c        (Revision 616517)
+++ proxy_util.c        (Arbeitskopie)
@@ -1107,6 +1107,7 @@
                 }
                 worker++;
             }
+            continue;
         }

         l2 = strlen(real);


Regards

Rüdiger

Re: svn commit: r616335 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
On Jan 29, 2008, at 3:43 PM, Ruediger Pluem wrote:

>
>
> On 01/29/2008 03:26 PM, jim@apache.org wrote:
>> Author: jim
>> Date: Tue Jan 29 06:26:20 2008
>> New Revision: 616335
>>
>> URL: http://svn.apache.org/viewvc?rev=616335&view=rev
>> Log:
>> Now let things like
>>
>>    ProxyPassReverse /foo balancer://bar
>>
>> work "as expected" :) :)
>
> I guess it may not in the case of members with the schemes
> ajp:// or fcgi:// :-).
>
>

:)

Well, if that's how they construct their Location headers :)

> I guess we can shortcut here by
>
>
>               return url;
>
> It seems unlikely that a backend server redirects to balancer://
>
>> +        }
>> +
>> +        l2 = strlen(real);
>> +        if (l1 >= l2 && strncasecmp(real, url, l2) == 0) {
>>              u = apr_pstrcat(r->pool, ent[i].fake, &url[l2], NULL);
>>              return ap_construct_url(r->pool, u, r);
>>          }
>
>

Shortcut where?