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/31 17:31:33 UTC
svn commit: r329849 - in /httpd/httpd/trunk: CHANGES
modules/proxy/proxy_util.c
Author: jim
Date: Mon Oct 31 08:31:29 2005
New Revision: 329849
URL: http://svn.apache.org/viewcvs?rev=329849&view=rev
Log:
Fix a problem where we are doing a case insensitive
match between the worker and the URL. Instead, only
the scheme and hostname are insensitive, the rest
should be case sensitive.
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/proxy/proxy_util.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/CHANGES?rev=329849&r1=329848&r2=329849&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Mon Oct 31 08:31:29 2005
@@ -2,6 +2,10 @@
Changes with Apache 2.3.0
[Remove entries to the current 2.0 and 2.2 section below, when backported]
+ *) mod_proxy_balancer: When finding best worker, use case insensitive
+ match for scheme and host, but case sensitive for the rest of
+ the path. [Jim Jagielski]
+
*) Asynchronous write completion for the Event MPM. [Brian Pane]
*) Added an End-Of-Request bucket type. The logging of a request and
Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=329849&r1=329848&r2=329849&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Mon Oct 31 08:31:29 2005
@@ -1216,6 +1216,7 @@
int max_match = 0;
int url_length;
int worker_name_length;
+ int sh_length;
const char *c;
int i;
@@ -1224,6 +1225,18 @@
return NULL;
url_length = strlen(url);
+
+ /*
+ * We need to find the start of the path and
+ * therefore we know the length of the scheme://hostname/
+ * part.
+ */
+ c = ap_strchr_c(c+3, '/');
+ if (c)
+ sh_length = c - url;
+ else
+ sh_length = url_length;
+
worker = (proxy_worker *)conf->workers->elts;
/*
@@ -1231,9 +1244,22 @@
* fits best to the URL.
*/
for (i = 0; i < conf->workers->nelts; i++) {
- if ( ((worker_name_length = strlen(worker->name)) <= url_length)
+ int prefix;
+ int bypass;
+ worker_name_length = strlen(worker->name);
+ if (worker_name_length <= sh_length) {
+ prefix = worker_name_length;
+ bypass = 1;
+ } else {
+ prefix = sh_length;
+ bypass = 0;
+ }
+ if ( (worker_name_length <= url_length)
&& (worker_name_length > max_match)
- && (strncasecmp(url, worker->name, worker_name_length) == 0) ) {
+ && (strncasecmp(url, worker->name, prefix) == 0)
+ && (bypass || (strncmp(url + prefix, worker->name + prefix,
+ worker_name_length - prefix) == 0)) )
+ {
max_worker = worker;
max_match = worker_name_length;
}
Re: svn commit: r329849 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c
Posted by Rüdiger Plüm <r....@gmx.de>.
On 11/01/2005 04:11 PM, Jim Jagielski wrote:
> Ruediger, would the below appease your sensibilities :)
Yes, it does. I am sorry. I guess I was a little too persistent in
this discussion about this patch of comparable limited influence.
I did not mean to step on your toes and nerves :-).
Regards
Rüdiger
[..cut..]
Re: svn commit: r329849 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
Ruediger, would the below appease your sensibilities :)
Index: modules/proxy/proxy_util.c
===================================================================
--- modules/proxy/proxy_util.c (revision 329779)
+++ modules/proxy/proxy_util.c (working copy)
@@ -1217,13 +1217,33 @@
int url_length;
int worker_name_length;
const char *c;
+ char *url_copy;
int i;
c = ap_strchr_c(url, ':');
if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0')
return NULL;
+ url_copy = apr_pstrdup(p, url);
url_length = strlen(url);
+
+ /*
+ * We need to find the start of the path and
+ * therefore we know the length of the scheme://hostname/
+ * part to we can force-lowercase everything up to
+ * the start of the path.
+ */
+ c = ap_strchr_c(c+3, '/');
+ if (c) {
+ char *pathstart;
+ pathstart = url_copy + (c - url);
+ pathstart = '\0';
+ ap_str_tolower(url_copy);
+ pathstart = '/';
+ } else {
+ ap_str_tolower(url_copy);
+ }
+
worker = (proxy_worker *)conf->workers->elts;
/*
@@ -1233,7 +1253,7 @@
for (i = 0; i < conf->workers->nelts; i++) {
if ( ((worker_name_length = strlen(worker->name)) <=
url_length)
&& (worker_name_length > max_match)
- && (strncasecmp(url, worker->name, worker_name_length) ==
0) ) {
+ && (strncmp(url_copy, worker->name, worker_name_length)
== 0) ) {
max_worker = worker;
max_match = worker_name_length;
}
Re: svn commit: r329849 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c
Posted by "Roy T. Fielding" <fi...@gbiv.com>.
> c = ap_strchr_c(url, ':');
> if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0')
> return NULL;
BTW, unless url is somehow limited to http and ftp, the above
is bad code. The proxy should be able to handle arbitrary
schemes (eventually), which means requiring scheme://host
is bogus.
....Roy
Re: svn commit: r329849 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 10/31/2005 05:31 PM, jim@apache.org wrote:
> Author: jim
> Date: Mon Oct 31 08:31:29 2005
> New Revision: 329849
>
> URL: http://svn.apache.org/viewcvs?rev=329849&view=rev
> Log:
> Fix a problem where we are doing a case insensitive
> match between the worker and the URL. Instead, only
> the scheme and hostname are insensitive, the rest
> should be case sensitive.
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/proxy/proxy_util.c
Thanks for looking into this. I think this is also related to PR36906.
Given the fact that the hostname and the schema already get lowercased by
ap_proxy_add_worker, wouldn't it be faster and clearer to do something like
the following (honest question, I do not know the answer and the best code should
go in):
PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
proxy_server_conf *conf,
const char *url)
{
proxy_worker *worker;
proxy_worker *max_worker = NULL;
int max_match = 0;
int url_length;
int worker_name_length;
const char *c;
int i;
char *url_copy;
c = ap_strchr_c(url, ':');
if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0')
return NULL;
url_copy = apr_pstrdup(p, url);
url_length = strlen(url_copy);
/*
* We need to find the start of the path and
* and lowercase all characters before.
*/
c = ap_strchr_c(c+3, '/');
if (c) {
*c = '\0';
ap_str_tolower(url_copy);
*c = '/';
}
else {
ap_str_tolower(url_copy);
}
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 ( ((worker_name_length = strlen(worker->name)) <= url_length)
&& (worker_name_length > max_match)
&& (strncmp(url_copy, worker->name, worker_name_length) == 0)) {
max_worker = worker;
max_match = worker_name_length;
}
worker++;
}
return max_worker;
}
Regards
Rüdiger