Bug 57287 - Sort files listed by DefaultServlet
Summary: Sort files listed by DefaultServlet
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 8.0.15
Hardware: PC All
: P2 enhancement (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
Depends on:
Reported: 2014-11-29 20:13 UTC by Konstantin Kolinko
Modified: 2021-07-02 09:23 UTC (History)
0 users

Bug_57287 patch (9.20 KB, patch)
2014-12-02 07:28 UTC, Oleg Trokhov
Details | Diff
sorting list (6.75 KB, patch)
2014-12-02 09:07 UTC, Oleg Trokhov
Details | Diff
Bug 57287v3 (6.72 KB, patch)
2014-12-05 10:04 UTC, Oleg Trokhov
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Konstantin Kolinko 2014-11-29 20:13:17 UTC
When DefaultServlet has file listings enabled, in Tomcat 8.0.15 and 7.0.57 it does not sort the files.

It lists them in the order provided by the underlying resources implementation. It is a bit harder to see on Windows, where the file system lists the files in a sorted order, but here is a reproduction recipe

1) Unpack apache-tomcat-8.0.15.zip
2) Add the following lines to webapps/ROOT/WEB-INF/web.xml


3) Delete file

4) Create directory
and copy there the following file from source code (apache-tomcat-8.0.15-src.zip):

5) Start Tomcat and access http://localhost:8080/

Actual behaviour:
Note that files "resourceB.jsp" and "resourceF.jsp" are listed at the end of the file list, instead of any alphabetical order.

A mention of this issue by a user:

Expected behaviour:
List the files alphabetically.

Though my preference for the sort order is
a) sort case-insensitively
b) list the directories first

It could be improved by offering a configurable sort order:
a) by a parameter in the request
b) by a user cookie

Tomcat 7
The same recipe reproduces the issue for Tomcat 7.0.57.

DefaultServlet can be configured to generate listing in XML and apply an XSLT transformation to it. XSLT can perform sorting.
Comment 1 Christopher Schultz 2014-12-01 17:14:53 UTC
+1 to "dirs first"

Arguments can be had about whether or not sorting should be done case-sensitive of case-insensitive.
Comment 2 Christopher Schultz 2014-12-01 17:21:31 UTC
I also think that sorting should be optional and not enabled by default: large directories could be problematic for performance and memory usage.

Or maybe there could be a setting that doesn't sort "directories" with more than a certain number of items (1k?)
Comment 3 Oleg Trokhov 2014-12-02 07:28:06 UTC
Created attachment 32248 [details]
Bug_57287 patch
Comment 4 Oleg Trokhov 2014-12-02 07:34:00 UTC
Hi. I added patch with some changes in DefaultServlet. Now by default sorting is Alphabetic ascending, directory first.User can add parameters to http request to configure sorting:
1. http://localhost:8080/?name=asc
2. http://localhost:8080/?name=dsc
3. http://localhost:8080/?size=asc
4. http://localhost:8080/?size=dsc
5. http://localhost:8080/?modify=asc
6. http://localhost:8080/?modify=dsc

Please tell if you have some suggestion or correction in code. And if you apply this change, where is documentation for this functions?
Comment 5 Konstantin Kolinko 2014-12-02 07:45:34 UTC
1. The code have to pass checkstyle rules (see BUILDING.txt on details on how enable checkstyle). One of rules: no ".*" imports.

2. I thought of using the same parameters as Apache HTTPD,
They use query strings like C=N;O=A
C column (N name, M last modified, S size, D description - absent in Tomcat)
O order (A ascending, D descending)

Your parameter naming with a single parameter also looks OK.
Comment 6 Oleg Trokhov 2014-12-02 09:07:50 UTC
Created attachment 32249 [details]
sorting list

I refactor import, so checkstyle task passed. So we leave my parameter naming or rename as as Apache HTTPD?
Comment 7 Violeta Georgieva 2014-12-04 08:43:00 UTC
(In reply to Oleg Trokhov from comment #6)
> Created attachment 32249 [details]
> sorting list
> I refactor import, so checkstyle task passed. So we leave my parameter
> naming or rename as as Apache HTTPD?

+1 for query string as Apache HTTPD

Comment 8 Konstantin Kolinko 2014-12-04 11:46:25 UTC
It can be more simple, with a single parameter such as
(sort Name ascending, Name descending)

A shorter parameter name makes it less obvious that the name is English one, better for i18n point of view.
Comment 9 Oleg Trokhov 2014-12-05 10:04:22 UTC
Created attachment 32260 [details]
Bug 57287v3

Now with patch v3:
1. "s=NA" name=asc
2. "s=ND" name=dsc
3. "s=SA" size=asc
4. "s=SD" size=dsc
5. "s=MA" modify=asc
6. "s=MD" modify=dsc
Comment 10 Christopher Schultz 2019-04-24 15:56:40 UTC
Some comments on this old patch.

1. Protections against accessing WEB-INF are shuffled-around a little in a way which is less efficient than before. [A first glance, I think every WEB-INF and META-INFO directory (or file!) will be removed from the display, which I think is incorrect behavior. We should only hide {context}/WEB-INF and {context}/META-INF, not {context}/foo/META-INF. This is not a problem with the patch; but something that could be improved.]

2. The number of comparator classes can probably be reduced. The complication of "directories always first" makes this less straightforward than it might seem. But there is an opportunity for improvement, here.

3. Comparators are completely thread-safe and do not need to be instantiated for every request.

4. This sorting is not optional (on the part of the server). If the client requests sorting, sorting will be done. This can be a DOS for a large directory. Some protection is necessary to prevent using resources that the administrator does not want to be used.

I'm interested in whether anyone cares whether "alphanumeric" sorting[1] is important.

[1] http://www.davekoelle.com/alphanum.html
Comment 11 Christopher Schultz 2019-05-05 13:37:19 UTC
Fixed in trunk in 65d762ce4c4de4179c9234665d631cf296d2f103 and 3e1496c295e9eab08e5d85fbc793c962dcb99c58
Back-ported to 8.5.x in 888b7699fde58de00e839797c7038ee0b10aaff2 and 65cb0202b3d746609af1a432f909a662ac2aa171

Will be in 9.0.22.
Will be in 8.5.42.
Comment 12 Konstantin Kolinko 2021-07-02 09:23:45 UTC
Please note that by default this feature is off and should be explicitly enabled.

E.g. with