On 10/10/2005 05:43 PM, Jim Jagielski wrote:
> For consideration:
[..cut..]
Thanks for your thoughts.
BTW: It was a little bit tricky to apply the patch as my Mozilla
seems to have changed things in the mail spaces / empty lines.
So I guess attached patches are easier to handle :-).
I think I found one big problem with the patch:
When we call ap_proxy_get_worker in line 1316 of proxy_util.c
(ap_proxy_pre_request), then the parameter url already contains
the complete URL we must call on the remote server. Example:
ServerName example.com
ProxyPass /mirror/foo http://backend.com/rsync
This would result in a worker named
http://backend.com/rsync
Calling http://example.com/mirror/foo/bar would be translated to
proxy:http://backend.com/rsync/bar in the translate_name hook of mod_proxy.
Thus ap_proxy_get_worker would be called with the URL
http://backend.com/rsync/bar
which would lead to a no match. I guess instead of a simple strcasecmp
we need something like a "longest match" here since I think we cannot rely
on the order the workers got stored. Do you know if a "longest match" function
is already at hand somewhere in apr / apr-util or httpd?
BTW: A similar problem should exist with the current code as in this case
ap_proxy_get_worker tries to compare http://backend.com/ with
http://backend.com/rsync
which also fails.
Other smaller problems that need to be solved:
1. Some style and output issues with the manager html interface (e.g.delete
column scheme)
I already did this and extended your patch with these things.
Please have a look if you like the design :-).
2. The xml output is currently broken as it only works with schema and hostname.
I am not quite sure if
1. there is an xml schema already defined for this and needs to be adjusted
2. we need to parse the worker name and replace some of the characters with
xml entities.
I think we need to fix this or if not at least a remark in the documentation
should be made that the xml output is currently broken.
Regards
Rüdiger
Index: modules/proxy/mod_proxy_balancer.c
===================================================================
--- modules/proxy/mod_proxy_balancer.c (Revision 312708)
+++ modules/proxy/mod_proxy_balancer.c (Arbeitskopie)
@@ -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);
Index: modules/proxy/proxy_util.c
===================================================================
--- modules/proxy/proxy_util.c (Revision 312708)
+++ modules/proxy/proxy_util.c (Arbeitskopie)
@@ -1218,10 +1218,6 @@
c = strchr(uri, ':');
if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0')
return NULL;
- /* remove path from uri */
- if ((c = strchr(c + 3, '/')))
- *c = '\0';
-
worker = (proxy_worker *)conf->workers->elts;
for (i = 0; i < conf->workers->nelts; i++) {
if (strcasecmp(worker->name, uri) == 0) {
