httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Pane <bp...@pacbell.net>
Subject Re: BUG: http_vhost.c:fix_hostname
Date Sun, 23 Jun 2002 08:55:30 GMT
On Sun, 2002-06-23 at 01:18, Eli Marmor wrote:
> Brian Pane wrote:
> > 
> > On Sat, 2002-06-22 at 13:56, Perry Harrington wrote:
> > > There is a bug in fix_hostname.  The comment above function says that the hostname
> > > is lowercased, but it's not.
> > >
> > > the line which reads:
> > >
> > >         *dst++ = *src++;
> > ...
> > > should read:
> > >
> > >         *dst++ = tolower(*src++);
> > 
> > Thanks, I'll commit a change to convert to lowercase.  All the virtual
> > hosting code that uses r->hostname is case-insensitive, but IMHO it's
> > better to normalize the case early to avoid surprising anyone who later
> > tries to write, for example, a custom vhosting module based on a
> > case-sensitive hash table.
> 
> The patch is needed, but I'm afraid it will be a bad idea to insert
> "++" into tolower(), since under some platforms it is a macro (AND NOT
> A FUNCTION!) defined by ctype.h, with more than 1 instance of the
> parameter, so src ends up being incremented by 2, or 3, or even 4.
> 
> Maybe the following will be better:
> 
> 	*dst++ = tolower(*src);
> 	++src;

The change that I committed for this actually looks like:
            else if (apr_isalpha(*dst)) {
                *dst = apr_tolower(*dst);
            }
so it's safe.

But now that you mention it, ap_strcasestr() is vulnerable
to the multiple-increment problem.  Fortunately, it's not
actually used anywhere in the core server--and there's probably
no reason to start using it now that apr_strmatch() is available--
but I'll fix the ap_strcasestr() code just in case any third
party modules depend on it.

--Brian



Mime
View raw message