httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ben Laurie <...@algroup.co.uk>
Subject Re: [PATCH] PR 3001- Win32: DocRoot x:/
Date Sat, 26 Sep 1998 14:40:52 GMT
Ken Parzygnat wrote:
> > Yes, but it seems to me that this is not particularly a WIN32 issue - we
> > shouldn't be building paths with double slashes (unless explicitly asked
> > to) on _any_ platform.
> 
> I agree.  This double slash problem will occur when the value
> for DocumentRoot ends in a slash.  I wasn't totally convinced that
> someone on a Unix system would code DocumentRoot to be '/' whereas
> on Windows, it's not too far fetched to have a user code 'x:/' as the
> DocumentRoot.  You can't code x: as the DocumentRoot because that is
> not a directory (as the server will tell you).  Any other directory '/foo',
> just omit the last slash in the .conf file, and everything is OK.  Do you
> think the ifdef should be removed to enable the code for all platforms?

I do.

> > > > > +        while ((d >= buf) && (*d == '/') && nSlashes)
{
> > > > > +          d--;
> > > > > +          nSlashes--;
> > > > > +        }
> > > > > +
> > > > > +        while (nSlashes--) {
> > > > > +            strcat(buf, "/");
> > > > > +        }
> > > >
> > > > Not convinced about this bit. If you are trying to avoid one
> > slash when
> > > > the path is "d:/" why not be more explicit about it?
> > > I'm trying to be very careful.  Back a while ago, you and a bunch of
> > > other folks were killing yourselves to fix this routine.  What
> > I don't want
> > > to break is the cases where we come in with some path
> > > from a CGI request that may have tons of /'s at the end of it.
> > This seemed
> > > like the most generic (and safest) way to handle it.
> >
> > Not convinced. What you are saying is that if canonicalisation adds /s
> > to the end, we should ignore that, assuming it is a mistake. Which seems
> > wrong to me.
> 
> Not exactly.  What I'm saying is if sub_canonical_filename adds a
> trailing slash (and it will add at most one), that it may be ignored
> based on what we did prior to the call to sub_canonical_filename
> during the canonicalization process.

If it will add at most one, why use a loop?

> > > > > -    if (szPath[n - 1] == '\\' || szPath[n - 1] == '/') {
> > > > > +    if ((szPath[n - 1] == '\\' || szPath[n - 1] == '/') &&
> > > > > +        !((n == 3) && (szPath[1] == ':')))
> > > >
> > > > I hate redundant brackets.
> > > Sorry, I tend to add the parens to inner expressions to avoid any
> > > ambiguity in the order of operations.  Helps me and the compiler :-).
> >
> > That you are helped, I can believe. I can't believe it helps the
> > compiler :-)
> >
> > What it makes me do is wonder whether you meant "=" instead of "==".
> 
> Hmmm.. Looking at the code segment above, it looks like a brace got
> dropped.  It's in the original that I sent, but not in what you and I got
> from new-httpd.  Let me see if I can repaste the segment:
> 
> -    if (szPath[n - 1] == '\\' || szPath[n - 1] == '/') {
> +    if ((szPath[n - 1] == '\\' || szPath[n - 1] == '/') &&
> +        !((n == 3) && (szPath[1] == ':')))
> 
>          char buf[_MAX_PATH];
> 
>          ap_assert(n < _MAX_PATH);
> 
> Hopefully the brace on the last inserted line is there.

Not that I can see.

>  With that
> in place, yes, I think I want "==".  I don't want to strip the trailing
> slash if we have "x:/" because I don't want to stat "x:".

:-) I know you want == - the redundant brackets would, in general, cause
me to wonder whether = was the intention (since that is the case where
they are not redundant).

Cheers,

Ben.

-- 
Ben Laurie            |Phone: +44 (181) 735 0686| Apache Group member
Freelance Consultant  |Fax:   +44 (181) 735 0689|http://www.apache.org/
and Technical Director|Email: ben@algroup.co.uk |
A.L. Digital Ltd,     |Apache-SSL author     http://www.apache-ssl.org/
London, England.      |"Apache: TDG" http://www.ora.com/catalog/apache/

WE'RE RECRUITING! http://www.aldigital.co.uk/

Mime
View raw message