httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ken Parzygnat" <kp...@raleigh.ibm.com>
Subject RE: [PATCH] PR 3001- Win32: DocRoot x:/
Date Sun, 27 Sep 1998 14:17:33 GMT
> > > 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.
Excellent, easily enough done.
>
> > > > > > +        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?

Good question.  I'm being overly cautious in counting trailing
slashes, when there really is no reason to be.  I'll change it
to a simple:
if (nSlashes && buf[strlen(buf)-1] == '/')
    nSlashes--;

> >
> > 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.

This is distrubing indeed.  I posted the same segment in a test
post to another mailing list and nothing was changed.  Seems
that something from the time I send the patch to
new-httpd to the time I recieve it from new-httpd
is manipulating that line by replacing the '{' with
a 'CRLF'.  There also seems to be a removal of trailing
whitespace occurring on several lines.
I'll try removing any whitespace before sending:

     n = strlen(szPath);
-    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);

If that didn't work, here are a few tests, all should end with a '{'

+        !((n == 3) && (szPath[1] == ':'))) {
+        ((n == 3) && (szPath[1] == ':'))) {
+        ((n == 3) && (szPath[1] == ':')))

+        {
+

!((n == 3) && (szPath[1] == ':'))) {
!((n == 3) && (szPath[1] == ':')))


>
> >  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).
>
Ahhh!  Gotcha.

- - - - - - - - - - - - - - - - - -
Ken Parzygnat
email: kparz@raleigh.ibm.com


Mime
View raw message