You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ji...@apache.org on 2005/10/11 23:43:35 UTC
svn commit: r312964 - in /httpd/httpd/branches/2.2.x: CHANGES
modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c
Author: jim
Date: Tue Oct 11 14:43:32 2005
New Revision: 312964
URL: http://svn.apache.org/viewcvs?rev=312964&view=rev
Log:
Merge r312963 from trunk:
mod_proxy_balancer: BalancerManager and proxies correctly handle
member workers with paths. PR36816. [Ruediger Pluem, Jim Jagielski]
Reviewed by: jim, rpluem
Modified:
httpd/httpd/branches/2.2.x/CHANGES
httpd/httpd/branches/2.2.x/modules/proxy/mod_proxy_balancer.c
httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c
Modified: httpd/httpd/branches/2.2.x/CHANGES
URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.2.x/CHANGES?rev=312964&r1=312963&r2=312964&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.2.x/CHANGES [utf-8] Tue Oct 11 14:43:32 2005
@@ -1,6 +1,9 @@
-*- coding: utf-8 -*-
Changes with Apache 2.1.9
+ *) mod_proxy_balancer: BalancerManager and proxies correctly handle
+ member workers with paths. PR36816. [Ruediger Pluem, Jim Jagielski]
+
*) mod_log_config: %{hextid}P will log the thread id in hex with APR
versions 1.2.0 or higher. [Jeff Trawick]
Modified: httpd/httpd/branches/2.2.x/modules/proxy/mod_proxy_balancer.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.2.x/modules/proxy/mod_proxy_balancer.c?rev=312964&r1=312963&r2=312964&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/proxy/mod_proxy_balancer.c (original)
+++ httpd/httpd/branches/2.2.x/modules/proxy/mod_proxy_balancer.c Tue Oct 11 14:43:32 2005
@@ -477,8 +477,12 @@
*val++ = '\0';
if ((tok = ap_strchr(val, '&')))
*tok++ = '\0';
+ /*
+ * Special case: workers are allowed path information
+ */
if ((access_status = ap_unescape_url(val)) != OK)
- return access_status;
+ if (strcmp(args, "w") || (access_status != HTTP_NOT_FOUND))
+ return access_status;
apr_table_setn(params, args, val);
args = tok;
}
@@ -490,13 +494,9 @@
bsel = ap_proxy_get_balancer(r->pool, conf,
apr_pstrcat(r->pool, "balancer://", name, NULL));
if ((name = apr_table_get(params, "w"))) {
- const char *sc = apr_table_get(params, "s");
- char *asname = NULL;
- proxy_worker *ws = NULL;
- if (sc) {
- asname = apr_pstrcat(r->pool, sc, "://", name, NULL);
- ws = ap_proxy_get_worker(r->pool, conf, asname);
- }
+ proxy_worker *ws;
+
+ ws = ap_proxy_get_worker(r->pool, conf, name);
if (ws) {
worker = (proxy_worker *)bsel->workers->elts;
for (n = 0; n < bsel->workers->nelts; n++) {
@@ -613,7 +613,7 @@
balancer->name + sizeof("balancer://") - 1,
"\">", NULL);
ap_rvputs(r, balancer->name, "</a></h3>\n\n", NULL);
- ap_rputs("\n\n<table border=\"0\"><tr>"
+ ap_rputs("\n\n<table border=\"0\" style=\"text-align: left;\"><tr>"
"<th>StickySession</th><th>Timeout</th><th>FailoverAttempts</th><th>Method</th>"
"</tr>\n<tr>", r);
ap_rvputs(r, "<td>", balancer->sticky, NULL);
@@ -622,9 +622,9 @@
ap_rprintf(r, "<td>%d</td>\n", balancer->max_attempts);
ap_rprintf(r, "<td>%s</td>\n",
balancer->lbmethod->name);
- ap_rputs("</table>\n", r);
- ap_rputs("\n\n<table border=\"0\"><tr>"
- "<th>Scheme</th><th>Host</th>"
+ ap_rputs("</table>\n<br />", r);
+ ap_rputs("\n\n<table border=\"0\" style=\"text-align: left;\"><tr>"
+ "<th>Worker URL</th>"
"<th>Route</th><th>RouteRedir</th>"
"<th>Factor</th><th>Status</th>"
"</tr>\n", r);
@@ -632,12 +632,11 @@
worker = (proxy_worker *)balancer->workers->elts;
for (n = 0; n < balancer->workers->nelts; n++) {
- ap_rvputs(r, "<tr>\n<td>", worker->scheme, "</td><td>", NULL);
- ap_rvputs(r, "<a href=\"", r->uri, "?b=",
- balancer->name + sizeof("balancer://") - 1,
- "&s=", worker->scheme, "&w=", worker->hostname,
+ ap_rvputs(r, "<tr>\n<td><a href=\"", r->uri, "?b=",
+ balancer->name + sizeof("balancer://") - 1, "&w=",
+ ap_escape_uri(r->pool, worker->name),
"\">", NULL);
- ap_rvputs(r, worker->hostname, "</a></td>", NULL);
+ ap_rvputs(r, worker->name, "</a></td>", NULL);
ap_rvputs(r, "<td>", worker->s->route, NULL);
ap_rvputs(r, "</td><td>", worker->s->redirect, NULL);
ap_rprintf(r, "</td><td>%d</td><td>", worker->s->lbfactor);
@@ -678,10 +677,8 @@
ap_rputs(" checked", r);
ap_rputs("></td><tr>\n", r);
ap_rputs("<tr><td colspan=2><input type=submit value=\"Submit\"></td></tr>\n", r);
- ap_rvputs(r, "</table>\n<input type=hidden name=\"s\" ", NULL);
- ap_rvputs(r, "value=\"", wsel->scheme, "\">\n", NULL);
- ap_rvputs(r, "<input type=hidden name=\"w\" ", NULL);
- ap_rvputs(r, "value=\"", wsel->hostname, "\">\n", NULL);
+ ap_rvputs(r, "</table>\n<input type=hidden name=\"w\" ", NULL);
+ ap_rvputs(r, "value=\"", ap_escape_uri(r->pool, wsel->name), "\">\n", NULL);
ap_rvputs(r, "<input type=hidden name=\"b\" ", NULL);
ap_rvputs(r, "value=\"", bsel->name + sizeof("balancer://") - 1,
"\">\n</form>\n", NULL);
Modified: httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c?rev=312964&r1=312963&r2=312964&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c Tue Oct 11 14:43:32 2005
@@ -1212,24 +1212,34 @@
const char *url)
{
proxy_worker *worker;
- char *c, *uri = apr_pstrdup(p, url);
+ proxy_worker *max_worker = NULL;
+ int max_match = 0;
+ int url_length;
+ int worker_name_length;
+ char *c;
int i;
- c = strchr(uri, ':');
+ c = strchr(url, ':');
if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0')
return NULL;
- /* remove path from uri */
- if ((c = strchr(c + 3, '/')))
- *c = '\0';
+ url_length = strlen(url);
worker = (proxy_worker *)conf->workers->elts;
+
+ /*
+ * Do a "longest match" on the worker name to find the worker that
+ * fits best to the URL.
+ */
for (i = 0; i < conf->workers->nelts; i++) {
- if (strcasecmp(worker->name, uri) == 0) {
- return worker;
+ if (((worker_name_length = strlen(worker->name)) <= url_length)
+ && (strncasecmp(url, worker->name, worker_name_length) == 0)
+ && (worker_name_length > max_match)) {
+ max_worker = worker;
+ max_match = worker_name_length;
}
worker++;
}
- return NULL;
+ return max_worker;
}
#if APR_HAS_THREADS
Re: svn commit: r312964 - in /httpd/httpd/branches/2.2.x: CHANGES
modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c
Posted by Rüdiger Plüm <r....@gmx.de>.
On 10/12/2005 01:38 PM, Joe Orton wrote:
[..cut..]
>
>
> Yup, if using strchr on a const variable, the correct method to preserve
> const-ness and avoid those warnings is:
>
> const char *foo;
>
> foo = ap_strchr_c(some_const_char_string, 'x');
>
> note that 'c' would need to be const above too.
Thanks for that info. It took me a while to understand the macros
in util_debug.c / httpd.h (small letter macros OtherBill
just mentioned to dislike :-)).
I think I understand now what is going on and that in DEBUG mode
the strict checking whether const or not is enforced by wrapper
functions whereas in the non debug mode it is a simple macro that
maps things directly to the libc functions which do not do this strict
checking sometimes.
If I understand Jim correctly we should use ap_strchr / ap_strchr_c
throughout the code, but it is not done currently. Maybe I have a look
into this on a rainy day and exchange strchr against ap_strchr / ap_strchr_c
where appropriate :-).
Regards
Rüdiger
Re: svn commit: r312964 - in /httpd/httpd/branches/2.2.x: CHANGES modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c
Posted by Joe Orton <jo...@redhat.com>.
On Wed, Oct 12, 2005 at 12:53:32PM +0200, Ruediger Pluem wrote:
>
>
> On 10/12/2005 12:09 PM, Joe Orton wrote:
> > On Tue, Oct 11, 2005 at 09:43:35PM -0000, Jim Jagielski wrote:
> > ...
> >
> >>Modified: httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c
> >>URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c?rev=312964&r1=312963&r2=312964&view=diff
> >>==============================================================================
> >>--- httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c (original)
> >>+++ httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c Tue Oct 11 14:43:32 2005
> >>@@ -1212,24 +1212,34 @@
> >> const char *url)
> >> {
> >> proxy_worker *worker;
> >>- char *c, *uri = apr_pstrdup(p, url);
> >>+ proxy_worker *max_worker = NULL;
> >>+ int max_match = 0;
> >>+ int url_length;
> >>+ int worker_name_length;
> >>+ char *c;
> >> int i;
> >>
> >>- c = strchr(uri, ':');
> >>+ c = strchr(url, ':');
> >> if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0')
> >> return NULL;
> >
> >
> > That broke the -Wall -Werror build with --enable-maintainer-mode
> >
> > cc1: warnings being treated as errors
> > proxy_util.c: In function 'ap_proxy_get_worker':
> > proxy_util.c:1222: warning: passing argument 1 of 'ap_strchr' discards qualifiers from pointer target type
>
> Sorry for the stupid question: Is this because *url is const?
Yup, if using strchr on a const variable, the correct method to preserve
const-ness and avoid those warnings is:
const char *foo;
foo = ap_strchr_c(some_const_char_string, 'x');
note that 'c' would need to be const above too.
joe
Re: svn commit: r312964 - in /httpd/httpd/branches/2.2.x: CHANGES
modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 10/12/2005 12:09 PM, Joe Orton wrote:
> On Tue, Oct 11, 2005 at 09:43:35PM -0000, Jim Jagielski wrote:
> ...
>
>>Modified: httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c
>>URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c?rev=312964&r1=312963&r2=312964&view=diff
>>==============================================================================
>>--- httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c (original)
>>+++ httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c Tue Oct 11 14:43:32 2005
>>@@ -1212,24 +1212,34 @@
>> const char *url)
>> {
>> proxy_worker *worker;
>>- char *c, *uri = apr_pstrdup(p, url);
>>+ proxy_worker *max_worker = NULL;
>>+ int max_match = 0;
>>+ int url_length;
>>+ int worker_name_length;
>>+ char *c;
>> int i;
>>
>>- c = strchr(uri, ':');
>>+ c = strchr(url, ':');
>> if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0')
>> return NULL;
>
>
> That broke the -Wall -Werror build with --enable-maintainer-mode
>
> cc1: warnings being treated as errors
> proxy_util.c: In function 'ap_proxy_get_worker':
> proxy_util.c:1222: warning: passing argument 1 of 'ap_strchr' discards qualifiers from pointer target type
Sorry for the stupid question: Is this because *url is const?
>
> is this the code which is about to be removed anyway?
Yes, I currently discuss this with Jim. The question is if this check is a valuable quick way or not.
I don't think that this is the case in a correctly configured system, but I would like to wait
for Jims response on that.
Regards
Rüdiger
Re: svn commit: r312964 - in /httpd/httpd/branches/2.2.x: CHANGES modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Oct 12, 2005, at 6:09 AM, Joe Orton wrote:
> On Tue, Oct 11, 2005 at 09:43:35PM -0000, Jim Jagielski wrote:
> ...
>
>> Modified: httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c
>> URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.2.x/
>> modules/proxy/proxy_util.c?rev=312964&r1=312963&r2=312964&view=diff
>> =====================================================================
>> =========
>> --- httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c (original)
>> +++ httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c Tue Oct
>> 11 14:43:32 2005
>> @@ -1212,24 +1212,34 @@
>> const char *url)
>> {
>> proxy_worker *worker;
>> - char *c, *uri = apr_pstrdup(p, url);
>> + proxy_worker *max_worker = NULL;
>> + int max_match = 0;
>> + int url_length;
>> + int worker_name_length;
>> + char *c;
>> int i;
>>
>> - c = strchr(uri, ':');
>> + c = strchr(url, ':');
>> if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0')
>> return NULL;
>>
>
> That broke the -Wall -Werror build with --enable-maintainer-mode
>
> cc1: warnings being treated as errors
> proxy_util.c: In function 'ap_proxy_get_worker':
> proxy_util.c:1222: warning: passing argument 1 of 'ap_strchr'
> discards qualifiers from pointer target type
>
> is this the code which is about to be removed anyway?
>
Argf. Yeah, that should be fixed. Of course, we should also, esp
in the the 2.3/4 trunk and the 2.3/4 branch standardize all our
usage of strchr. It's current a mix and mess of all flavors :)
I'll fix this later on today.
Re: svn commit: r312964 - in /httpd/httpd/branches/2.2.x: CHANGES modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c
Posted by Joe Orton <jo...@redhat.com>.
On Tue, Oct 11, 2005 at 09:43:35PM -0000, Jim Jagielski wrote:
...
> Modified: httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c?rev=312964&r1=312963&r2=312964&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c Tue Oct 11 14:43:32 2005
> @@ -1212,24 +1212,34 @@
> const char *url)
> {
> proxy_worker *worker;
> - char *c, *uri = apr_pstrdup(p, url);
> + proxy_worker *max_worker = NULL;
> + int max_match = 0;
> + int url_length;
> + int worker_name_length;
> + char *c;
> int i;
>
> - c = strchr(uri, ':');
> + c = strchr(url, ':');
> if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0')
> return NULL;
That broke the -Wall -Werror build with --enable-maintainer-mode
cc1: warnings being treated as errors
proxy_util.c: In function 'ap_proxy_get_worker':
proxy_util.c:1222: warning: passing argument 1 of 'ap_strchr' discards qualifiers from pointer target type
is this the code which is about to be removed anyway?
joe