httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marc Slemko <ma...@znep.com>
Subject Re: mod_imap.c and infinite loops
Date Sun, 16 Feb 1997 00:05:07 GMT
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 */ 
> >     
> 
> 
> 


Mime
View raw message