Created attachment 17328 [details]
patch for caching directory listings

This patch addresses the DoS problem and significantly improves directory
listing performance.

Does anyone know what the status regarding this patch is? I believe the problem
still affects the latest versions of tomcat.

Patch is yet to be evaluated.

Is there anything I can do to expidite the process? I would really like to see a
fix for the problem incorporated. Without some fix it is impossible to use the
directory listing feature when even moderately large directories are involved.

There are a few things I dislike in this patch.

You shouldn't catch Throwables and then silently not handle them.  You shouldn't
even do that with Exceptions, but certainly not Throwables.  That by itself
means -1 on the patch as it stands currently.

Then there's the overall weight of the solution: adding a custom cache and tying
it into the default web.xml just for this case seems overweight.  I wonder if
there's an easier solution without caching, and certainly without a time-based
cache which means additional background processing.  One approach that comes to
mind is a no-op XSLT for customizations, as documented in
http://tomcat.apache.org/tomcat-5.0-doc/default-servlet.html#dir

Finally, as you probably know directory listings are easy to disable: see
http://marc.theaimsgroup.com/?l=tomcat-user&m=105525007220640&w=2 for example of
the one setting change required.  For others concerned about this DoS (and I
don't think there are any, seeing as how no one else has opined on this issue
and/or posted such a DoS on the mailing lists), they can simply disable
directory listings.

Because of these, I'm going to mark this particular patch as WONTFIX.  If
someone else wants strongly feels this patch should be applied as-is, they can
say so here.  Otherwise, maybe a more lightweight patch can be attached to the
issue (and the issue itself reopened), that is if anyone still cares.

(In reply to comment #5)
> There are a few things I dislike in this patch.
> 
> You shouldn't catch Throwables and then silently not handle them.  You shouldn't
> even do that with Exceptions, but certainly not Throwables.  That by itself
> means -1 on the patch as it stands currently.

I agree that catching Throwable is a really bad idea, however the DefaultServlet
code does this repeatedly when accessing parameters from the servlet config. I
chose to keep the code consistent and follow the pattern already prevalent when
making this patch. I beleive it should be a simple matter to string replace
Throwable with a more appropriate exception in this code.

> Then there's the overall weight of the solution: adding a custom cache and tying
> it into the default web.xml just for this case seems overweight.  I wonder if
> there's an easier solution without caching, and certainly without a time-based
> cache which means additional background processing.  One approach that comes to
> mind is a no-op XSLT for customizations, as documented in
> http://tomcat.apache.org/tomcat-5.0-doc/default-servlet.html#dir
> 
> Finally, as you probably know directory listings are easy to disable: see
> http://marc.theaimsgroup.com/?l=tomcat-user&m=105525007220640&w=2 for example of
> the one setting change required.  For others concerned about this DoS (and I
> don't think there are any, seeing as how no one else has opined on this issue
> and/or posted such a DoS on the mailing lists), they can simply disable
> directory listings.
> 
> Because of these, I'm going to mark this particular patch as WONTFIX.  If
> someone else wants strongly feels this patch should be applied as-is, they can
> say so here.  Otherwise, maybe a more lightweight patch can be attached to the
> issue (and the issue itself reopened), that is if anyone still cares.

The caching this patch adds is not just a special purpose fix to eliminate a
DoS, it also significantly improves the performance of directory listings in the
general case. Please review my posts to the dev lists containing my benchmarks
comparing performance with and without the patch in place.

Note I've also converted existing DefaultServlet catch(Throwable t) code to
catching exceptions.

I saw your mailing list messages, and I believe the benchmarks.  I still think
this approach is too heavyweight.