From: rst@ai.mit.edu (Robert S. Thau)
Date: Thu, 27 Jul 95 21:34:19 EDT
Message-Id: <9507280134.AA21775@volterra>
To: new-httpd@hyperreal.com
Cc: new-httpd@hyperreal.com
In-Reply-To: <m0sbUJE-0003pVC@cass39.ast.cam.ac.uk> (drtr@ast.cam.ac.uk)
Subject: Re: REPOST: URL decoding bugs in apache 0.8.3
Sender: owner-new-httpd@apache.org
Precedence: bulk
Reply-To: new-httpd@apache.org

   Date: Thu, 27 Jul 95 15:53 BST
   From: drtr@ast.cam.ac.uk (David Robinson)
   Precedence: bulk
   Reply-To: new-httpd@hyperreal.com

   I think that apache showing problems for URL paths containing (decoded) '%'
   characters should be considered signficicant.

For all such paths --- of course.  For paths which are produced in
circumstances which are highly unlikely to come up in practice, I'm
not so sure.  My test for this sort of thing is, as I said before,
whether the marginal cases in question have actually come up in
practice, on someone's real site (or whether there's a *clearly*
plausible way in which they could).

For your bugs 1 & 2, I can come up with plausible situations where it
would screw up (and the fixes are simple and local, which makes
unintended consequences less likely).  For #3, I'm not so sure ---
particularly when the fix involves changing a somewhat tricky routine.

   But the current code has similar bugs;
   a lookup of bar%41?moo would be inherit the args of the original request,
   instead of setting it to 'moo', and the path would not be URL decoded
   to barA. And it would fail if the original path contained a (decoded)
   '%' character.

Hmmm... this patch makes me nervous (non-trivial changes to a somewhat
subtle routine), but it looks inoccuous enough, and I can at least see
how to trigger the bug(s) it fixes.  I'd appreciate it if someone at a
heavy includes-using site would try it out (SSI could be totally
trashed and I'd never notice here); if it doesn't wreck anything, I'm
happy to have it.

(Incidentally, it doesn't seem to fix your bugs 1&2 (PATH_TRANSLATED
(bleah) and internal_redirect.  A separate patch for these would be
very helpful, if you could make one up).

   >   4. (May not be a bug) sub_req_lookup_file wasn't setting the args of the
   >       new request.
   >
   >Not a bug, in my view --- URIs may have arguments, by the '...?foo'
   >convention, but filenames do not.

   Only a suggestion; but it seemed easier to set args rather than try and
   guarantee that no existing or future code would ever try and access the args
   for a request returned by sub_req_look_file.

If it does, it will find... no args, the same as if args were
supported but there simply hadn't been any.  I'm not sure I see the
problem.

rst