2014-08-12 14:19 GMT+04:00 Mark Thomas <ma...@apache.org>:
> On 12/08/2014 00:14, Konstantin Kolinko wrote:
>> 2014-08-04 21:22 GMT+04:00 <ma...@apache.org>:
>>> Author: markt
>>> Date: Mon Aug 4 17:22:01 2014
>>> New Revision: 1615697
>>>
>>> URL: http://svn.apache.org/r1615697
>>> Log:
>>> When the gzip option is enabled for the DefaultServlet ensure that a
>>> suitable Vary header is returned for resources that might be returned
>>> directly in compressed form.
>>>
>>> Modified:
>>> tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
>>> tomcat/trunk/webapps/docs/changelog.xml
>>>
>>> Modified: tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
>>> URL:
>>> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java?rev=1615697&r1=1615696&r2=1615697&view=diff
>>> ==============================================================================
>>> --- tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
>>> (original)
>>> +++ tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java Mon
>>> Aug 4 17:22:01 2014
>>> @@ -799,16 +799,15 @@ public class DefaultServlet extends Http
>>>
>>> // Serve a gzipped version of the file if present
>>> boolean usingGzippedVersion = false;
>>> - if (gzip &&
>>> - resource.isFile() &&
>>> - !included &&
>>
>> 1). I do get why "!included" check was moved to the place below. I
>> think it should not be moved.
>>
>> I think that "included" flag does not depend on "accept-encoding"
>> value sent by user.
>
> Agreed. Fixed.
Good.
>> 2). There may already be a Vary header in the response.
>
> Agreed. A filter could set one. This has been fixed too.
The updated HTTP specification does not allow multiple Vary headers in
the response.
Multiple headers are allowed only if value is defined as a list, but
here it is defined as "asterisk or a list".
>> 3) The code may check response.isCommitted(). (In other words, that
>> addHeader("Content-Encoding", "gzip")
>> call has been successful).
>
> Is that necessary? It isn't something we check anywhere else in the
> DefaultServlet (and I am not aware of any problems in that area).
Just an additional check to add to the condition where "if(!included)"
is checked.
I guess that majority of such use cases are already covered by
"if(!included)" check.
It matters only if some content have already been generated by a Filter.
This gz file serving feature is relatively new and it is off by
default, so I think there are not many who are using it to report
errors.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
