Hi!
Regarding this commit:
2013/10/11 <ma...@apache.org>:
> Author: markt
> Date: Thu Oct 10 21:24:59 2013
> New Revision: 1531115
>
> URL: http://svn.apache.org/r1531115
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=54095
> Add support to the Default Servlet for serving gzipped versions of static
> resources directly from disk as an alternative to Tomcat compressing them on
> each request. Patch by Philippe Marschall.
>
> Added:
> tomcat/trunk/test/webapp/index.html.gz (with props)
> Modified:
> tomcat/trunk/conf/web.xml
> tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
> tomcat/trunk/test/org/apache/catalina/servlets/TestDefaultServlet.java
> tomcat/trunk/webapps/docs/changelog.xml
> tomcat/trunk/webapps/docs/default-servlet.xml
>
General:
I think this feature should be opt-in, like the listings feature of
DefaultServlet, being off by default.
Concerns:
(1) Excessive disk access to check existence of gz files.
(2) Additional access path for ".gz" files, which might be not covered
by security constraints
(3) Interoperability with filters that may preprocess or postprocess
the response,
including ISE handling in the following lines of DefaultServlet:
[[[
try {
ostream = response.getOutputStream();
} catch (IllegalStateException e) {
...
writer = response.getWriter();
]]]
(2) and (3) would occur only if both "foo" and "foo.gz" files are present.
As (2) requires the presence and accessibility of "foo" file (as
"resource.exists()" check is done before gzip processing), I see no
exploitable issue here, though we may mention this on the
"security-howto" page.
Technical issues with the code:
1) "If-Modified-Since" header processing is inconsistent,
The checkIfHeaders(...) call happens before gzip processing and checks
the date of original resource, not of gz one,
but "ETag" and "Last-Modified" headers will be for the gz resource.
I think it is better to respond with "ETag" and "Last-Modified" of the
original resource.
2) I think that if it is an "included" resource, the gzip feature
should be disabled.
See lines 751-752 as an example:
[[[
boolean included = (request.getAttribute(
RequestDispatcher.INCLUDE_CONTEXT_PATH) != null);
]]]
3) The following line:
>+ gzipResource.setMimeType(contentType);
I think that line is not needed here, as the "Content-Type" header is
served from a local variable, not property of the resource.
Moreover I suspect that it changes mime-type of a cached resource and
a subsequent direct request for "foo.gz" will be served with wrong
mime-type.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
