Received: by taz.hyperreal.com (8.8.4/V2.0) id QAA01778;
 Sat, 15 Feb 1997 16:10:14 -0800 (PST)
Received: from scanner.worldgate.com by taz.hyperreal.com (8.8.4/V2.0) with
 ESMTP id QAA01774; Sat, 15 Feb 1997 16:10:11 -0800 (PST)
Received: from znep.com (uucp@localhost) by scanner.worldgate.com
 (8.8.5/8.7.3) with UUCP id RAA23499 for new-httpd@hyperreal.com;
 Sat, 15 Feb 1997 17:10:09 -0700 (MST)
Received: from localhost (marcs@localhost) by alive.znep.com (8.7.5/8.7.3)
 with SMTP id RAA25097 for <new-httpd@hyperreal.com>;
 Sat, 15 Feb 1997 17:05:08 -0700 (MST)
Date: Sat, 15 Feb 1997 17:05:07 -0700 (MST)
From: Marc Slemko <marcs@znep.com>
To: Apache List <new-httpd@hyperreal.com>
Subject: Re: mod_imap.c and infinite loops
In-Reply-To: <199702110508.AAA20089@tripod.tripod.com>
Message-ID: <Pine.BSF.3.95.970215160832.23557E-100000@alive.znep.com>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: new-httpd-owner@apache.org
Precedence: bulk
Reply-To: new-httpd@hyperreal.com

On Tue, 11 Feb 1997, Nathan J Kurz wrote:

> hello. (he said quietly, wondering if he's allowed on this list)
> 
> I haven't written anything here before, but thought now might be a
> good time.  I saw the flurry of posts on new-httpd regarding infinite
> loops in mod_imap.c and thought I might be able to contribute
> something since I wrote the beast called imap_url().
> 
> My thought when handling "base" was that it should it should react
> exactly like the HTML command BASE when given an absolute URL, and
> should do the best it can when given anything else.  For example, if
> you say "base mailto:" a have a directive pointing to
> "nate@tripod.com" it will work.  In retrospect, this is probably a bad
> thing, since it forces you to maintain compatibility with those
> special cases forever.  Sorry about that.
> 
> The infinite loop problem came in because of a patch attempt made
> after I submitted it.  This can easily be fixed by going back to the
> original handling method I had, but this handles ".." cases a little
> differently than it currently does.
> 
> In particular, the question was how should this react:
> 
> base http://host/dir/subdir/
> rect .. 0,0 100,100
> 
> My rationale was that it should work exactly like BASE does. 
> <base href="http://host/dir/subdir/"> is synonymous with 
> <base href="http://host/dir/subdir/index.html">, so I figured
> that ".." should take you to the same place in each case, which would be
> "http://host/dir/subdir".  

If you have a base of http://host/dir/subdir/file.html, then the relative
url of file2.html will be resolved to http://host/dir/subdir/file2.html. 
To me, that means if you have a relative url of ../file.html, it should be
resolved to http://host/dir/file.html.

> 
> The other opinion (Randy's) was that ".." should always take you up a
> level, and thus should take you to "http://host/dir/".  While this may
> make it slightly easier to use, it is no longer reacts the same as
> BASE (when given an absolute URL).
> 
> In retrospect, I think this module would be a lot better as a CGI.
> For how often it is used, it's way too large and tries to do way too
> much.  Now that mod_action.c is distributed, you could drop a CGI
> program in /support, add one line to a configuration file, and have
> everything work the same -- and probably with some gain in performance
> due to the smaller executable, unless your entire site is imagemaps.
> 
> That said, if there are things I can do to try to fix some of the
> other bugs people alluded to (which specifically?) I'd be glad to do
> so.  Or if you'd like the whole thing made in to a standalone CGI, I'd
> be glad to do that as well.  In fact, maybe I'll do that for our own
> purposes.

I'm not specifically sure of any other bugs, but the parsing code looks
ugly enough that there are several places where there should be a problem.
This has to be looked at carefully.  Sorry for throwing around wild
comments that I don't have the time to backup, but right now I'm worried
about other things.

To avoid committing to any behavior which may not be desirable, I have a
patch I will send to the list which aborts with an error if this case
happens.  It does not fix things so they work the way they should, but it
also doesn't introduce any more legacy behavior to support.

> 
> nathan kurz
> nate@tripod.com
> http://www.tripod.com
> 
> Unless I missed something between versions, here's a patch to 1.2b6 to
> go back to a slightly more BASE compatible system of handling and get
> rid of infinite loops:
> 
> --------------------------------------------------------------------
> 363d362
> <   int slen, clen;
> 454,473c453,463
> <       if (directory && (slen = strlen (directory))) {
> < 
> <         /* for each '..',  knock a directory off the end 
> <            by ending the string right at the last slash.
> <            But only consider the directory portion: don't eat
> <            into the server name.  And only try if a directory
> <            portion was found */    
> <         
> <         clen = slen - 1;
> <       
> <         while ((slen - clen) == 1) {
> <       
> <             if ((string_pos = strrchr(directory, '/')))
> <                 *string_pos = '\0';
> <             clen = strlen (directory);
> <             if (clen == 0) break;
> <         }
> < 
> <         value += 2;      /* jump over the '..' that we found in the value */
> <       }
> ---
> >     if ( directory && (string_pos = strrchr(directory, '/')) ) 
> >       *string_pos = '\0';
> >     /* for each '..',  knock a directory off the end 
> >        by ending the string right at the last slash.
> >        But only consider the directory portion: don't eat
> >        into the server name.  And only try if a directory
> >        portion was found */    
> >     
> >     value += 2;      /* jump over the '..' that we found in the value */
> >     
> >     if (! strncmp(value, "/../", 4) || ! strcmp(value, "/..") )
> 475,480c465,468
> <       if (! strncmp(value, "/../", 4) || ! strcmp(value, "/..") )
> < 
> <         value++;       /* step over the '/' if there are more '..' to do.
> <                          this way, we leave the starting '/' on value after
> <                          the last '..', but get rid of it otherwise */ 
> <      
> ---
> >       value++;       /* step over the '/' if there are more '..' to do.
> >                       this way, we leave the starting '/' on value after
> >                       the last '..', but get rid of it otherwise */ 
> >     
> 
> 
>