You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2018/02/15 11:38:11 UTC
svn commit: r1824297 - in /tomcat/trunk:
java/org/apache/coyote/CompressionConfig.java webapps/docs/changelog.xml
Author: markt
Date: Thu Feb 15 11:38:11 2018
New Revision: 1824297
URL: http://svn.apache.org/viewvc?rev=1824297&view=rev
Log:
Prevent Tomcat from applying gzip compression to content that is already compressed with brotli compression.
Patch provided by burka.
This closes #99
Modified:
tomcat/trunk/java/org/apache/coyote/CompressionConfig.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/coyote/CompressionConfig.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/CompressionConfig.java?rev=1824297&r1=1824296&r2=1824297&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/CompressionConfig.java (original)
+++ tomcat/trunk/java/org/apache/coyote/CompressionConfig.java Thu Feb 15 11:38:11 2018
@@ -186,9 +186,10 @@ public class CompressionConfig {
MimeHeaders responseHeaders = response.getMimeHeaders();
- // Check if content is not already gzipped
+ // Check if content is not already compressed
MessageBytes contentEncodingMB = responseHeaders.getValue("Content-Encoding");
- if ((contentEncodingMB != null) && (contentEncodingMB.indexOf("gzip") != -1)) {
+ if ((contentEncodingMB != null) && (contentEncodingMB.indexOf("gzip") != -1) &&
+ (contentEncodingMB.indexOf("br") != -1)) {
return false;
}
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1824297&r1=1824296&r2=1824297&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Thu Feb 15 11:38:11 2018
@@ -60,6 +60,10 @@
specific error codes and/or exception types with the
<code>ErrorReportValve</code>. (markt)
</add>
+ <fix>
+ Prevent Tomcat from applying gzip compression to content that is already
+ compressed with brotli compression. Patch provided by burka. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1824297 - in /tomcat/trunk:
java/org/apache/coyote/CompressionConfig.java webapps/docs/changelog.xml
Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Konstantin,
On 2/15/18 8:14 AM, Konstantin Kolinko wrote:
> 2018-02-15 14:38 GMT+03:00 <ma...@apache.org>:
>> Author: markt Date: Thu Feb 15 11:38:11 2018 New Revision:
>> 1824297
>>
>> URL: http://svn.apache.org/viewvc?rev=1824297&view=rev Log:
>> Prevent Tomcat from applying gzip compression to content that is
>> already compressed with brotli compression. Patch provided by
>> burka. This closes #99
>>
>> Modified:
>> tomcat/trunk/java/org/apache/coyote/CompressionConfig.java
>> tomcat/trunk/webapps/docs/changelog.xml
>>
>> Modified:
>> tomcat/trunk/java/org/apache/coyote/CompressionConfig.java URL:
>> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/Comp
ressionConfig.java?rev=1824297&r1=1824296&r2=1824297&view=diff
>>
>>
========================================================================
======
>> --- tomcat/trunk/java/org/apache/coyote/CompressionConfig.java
>> (original) +++
>> tomcat/trunk/java/org/apache/coyote/CompressionConfig.java Thu
>> Feb 15 11:38:11 2018 @@ -186,9 +186,10 @@ public class
>> CompressionConfig {
>>
>> MimeHeaders responseHeaders = response.getMimeHeaders();
>>
>> - // Check if content is not already gzipped + //
>> Check if content is not already compressed MessageBytes
>> contentEncodingMB =
>> responseHeaders.getValue("Content-Encoding"); - if
>> ((contentEncodingMB != null) &&
>> (contentEncodingMB.indexOf("gzip") != -1)) { + if
>> ((contentEncodingMB != null) &&
>> (contentEncodingMB.indexOf("gzip") != -1) && +
>> (contentEncodingMB.indexOf("br") != -1)) {
>
> 1) Wrong. Causes regression. It should be ( != -1 || != -1).
> Either one is enough to skip compression.
>
> 2) "indexOf(br)" sounds weak, there can be false positives. But
> not much of a problem: it would result in serving content as
> non-compresses, it does not break the content.
There will be no false-positives if the value contains an RFC-valid
value. Only "br" contains "br".
I would argue that this should be an equality-check and not an
"indexOf" check, since the response should expect to have exactly one
encoding (as opposed to Accept-Encoding, which can list many
acceptable encodings).
> For "gzip" using indexOf() is OK: It can be "x-gzip" (see
> 3.1.2.1. Content Codings in RFC7231.) [quote] The following
> content-coding values are defined by this specification:
>
> compress (and x-compress): See Section 4.2.1 of [RFC7230]. deflate:
> See Section 4.2.2 of [RFC7230]. gzip (and x-gzip): See Section
> 4.2.3 of [RFC7230]. [/quote]
+1
- -chris
-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQJRBAEBCAA7FiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAlqLAFAdHGNocmlzQGNo
cmlzdG9waGVyc2NodWx0ei5uZXQACgkQHPApP6U8pFinzA/8C6parkPID67fOx4i
V2R9Kq3+KxsS/XMPoPo2nyjAGbgJ2GfSJfRUmQbNQHGyxUrOcQS9n3FbG1fZqtMu
+I8eRTSeACEPxOID5W1J3bVWY+/KEfCFvRtw23ZUZuTs8kaVo8NbnCe7PsV15R2E
bGliecJbyIqvaeeXyUxbGBpDlUy9LV9E7xcW3Cro7WAFPid/vpSsXs4JtAZo2vqx
ZgyY5fGp3LWaWI8aeJZLEHk31aQEjS6xYhUbnxXwn3a2ALwJUrZW/AJ69jpVL7ge
fgDf5qU55g553EtQvKzGpBSogMTqnijlqgD1pEG6P4IBjEZZc8LwjJ+lY9AM+YIJ
Qbg1EPpiFuqxh/oF1csHOMdEztxVJHEEc8qELDkcP/tgUWHdHxoedIlPwYFRRyxV
zu26OQ9rDdqX7sAFngaic8ot178T4a0xvrFvy/8kV8Mp2gsgNgtspEf2ONxoLvav
x1pYGOuiYwb3tpllxfRsqqJZGXNvKO5+YYOiNSW21vCZpB32yVtZF5TPfx4Yvqie
N8HcSXc7JhY8N+HqusjVNlyWC0Y0Y8K+3mP36Ss4rUUI8XJgGpyOfFe9B4sioTXD
FaF5fpOE6pMBvLkBdnAt0X/MWVyMJLW7WYCNDo5mwuIusqM0mqwANJvJInDj2SMi
d4HEBDgTh6HcpHDFJ57I5ljZi5Q=
=hlZq
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1824297 - in /tomcat/trunk:
java/org/apache/coyote/CompressionConfig.java webapps/docs/changelog.xml
Posted by Mark Thomas <ma...@apache.org>.
On 15/02/18 13:14, Konstantin Kolinko wrote:
> 2018-02-15 14:38 GMT+03:00 <ma...@apache.org>:
>> Author: markt
>> Date: Thu Feb 15 11:38:11 2018
>> New Revision: 1824297
>>
>> URL: http://svn.apache.org/viewvc?rev=1824297&view=rev
>> Log:
>> Prevent Tomcat from applying gzip compression to content that is already compressed with brotli compression.
>> Patch provided by burka.
>> This closes #99
>>
>> Modified:
>> tomcat/trunk/java/org/apache/coyote/CompressionConfig.java
>> tomcat/trunk/webapps/docs/changelog.xml
>>
>> Modified: tomcat/trunk/java/org/apache/coyote/CompressionConfig.java
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/CompressionConfig.java?rev=1824297&r1=1824296&r2=1824297&view=diff
>> ==============================================================================
>> --- tomcat/trunk/java/org/apache/coyote/CompressionConfig.java (original)
>> +++ tomcat/trunk/java/org/apache/coyote/CompressionConfig.java Thu Feb 15 11:38:11 2018
>> @@ -186,9 +186,10 @@ public class CompressionConfig {
>>
>> MimeHeaders responseHeaders = response.getMimeHeaders();
>>
>> - // Check if content is not already gzipped
>> + // Check if content is not already compressed
>> MessageBytes contentEncodingMB = responseHeaders.getValue("Content-Encoding");
>> - if ((contentEncodingMB != null) && (contentEncodingMB.indexOf("gzip") != -1)) {
>> + if ((contentEncodingMB != null) && (contentEncodingMB.indexOf("gzip") != -1) &&
>> + (contentEncodingMB.indexOf("br") != -1)) {
>
> 1) Wrong. Causes regression. It should be ( != -1 || != -1). Either
> one is enough to skip compression.
Whoops. I'll fix that.
> 2) "indexOf(br)" sounds weak, there can be false positives. But not
> much of a problem: it would result in serving content as
> non-compresses, it does not break the content.
I did think about that. There aren't that many likely values in
Content-Encoding [1] and none of them conflict with the use of "br".
>> Modified: tomcat/trunk/webapps/docs/changelog.xml
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1824297&r1=1824296&r2=1824297&view=diff
>> ==============================================================================
>> --- tomcat/trunk/webapps/docs/changelog.xml (original)
>> +++ tomcat/trunk/webapps/docs/changelog.xml Thu Feb 15 11:38:11 2018
>> @@ -60,6 +60,10 @@
>> specific error codes and/or exception types with the
>> <code>ErrorReportValve</code>. (markt)
>> </add>
>> + <fix>
>> + Prevent Tomcat from applying gzip compression to content that is already
>> + compressed with brotli compression. Patch provided by burka. (markt)
>
> Based on patch provided...
Ack.
Mark
[1]
https://www.iana.org/assignments/http-parameters/http-parameters.xml#http-parameters-1
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1824297 - in /tomcat/trunk: java/org/apache/coyote/CompressionConfig.java
webapps/docs/changelog.xml
Posted by Konstantin Kolinko <kn...@gmail.com>.
2018-02-15 14:38 GMT+03:00 <ma...@apache.org>:
> Author: markt
> Date: Thu Feb 15 11:38:11 2018
> New Revision: 1824297
>
> URL: http://svn.apache.org/viewvc?rev=1824297&view=rev
> Log:
> Prevent Tomcat from applying gzip compression to content that is already compressed with brotli compression.
> Patch provided by burka.
> This closes #99
>
> Modified:
> tomcat/trunk/java/org/apache/coyote/CompressionConfig.java
> tomcat/trunk/webapps/docs/changelog.xml
>
> Modified: tomcat/trunk/java/org/apache/coyote/CompressionConfig.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/CompressionConfig.java?rev=1824297&r1=1824296&r2=1824297&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/CompressionConfig.java (original)
> +++ tomcat/trunk/java/org/apache/coyote/CompressionConfig.java Thu Feb 15 11:38:11 2018
> @@ -186,9 +186,10 @@ public class CompressionConfig {
>
> MimeHeaders responseHeaders = response.getMimeHeaders();
>
> - // Check if content is not already gzipped
> + // Check if content is not already compressed
> MessageBytes contentEncodingMB = responseHeaders.getValue("Content-Encoding");
> - if ((contentEncodingMB != null) && (contentEncodingMB.indexOf("gzip") != -1)) {
> + if ((contentEncodingMB != null) && (contentEncodingMB.indexOf("gzip") != -1) &&
> + (contentEncodingMB.indexOf("br") != -1)) {
1) Wrong. Causes regression. It should be ( != -1 || != -1). Either
one is enough to skip compression.
2) "indexOf(br)" sounds weak, there can be false positives. But not
much of a problem: it would result in serving content as
non-compresses, it does not break the content.
For "gzip" using indexOf() is OK: It can be "x-gzip" (see 3.1.2.1.
Content Codings in RFC7231.)
[quote]
The following content-coding values are defined by this
specification:
compress (and x-compress): See Section 4.2.1 of [RFC7230].
deflate: See Section 4.2.2 of [RFC7230].
gzip (and x-gzip): See Section 4.2.3 of [RFC7230].
[/quote]
> return false;
> }
>
>
> Modified: tomcat/trunk/webapps/docs/changelog.xml
> URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1824297&r1=1824296&r2=1824297&view=diff
> ==============================================================================
> --- tomcat/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/trunk/webapps/docs/changelog.xml Thu Feb 15 11:38:11 2018
> @@ -60,6 +60,10 @@
> specific error codes and/or exception types with the
> <code>ErrorReportValve</code>. (markt)
> </add>
> + <fix>
> + Prevent Tomcat from applying gzip compression to content that is already
> + compressed with brotli compression. Patch provided by burka. (markt)
Based on patch provided...
> + </fix>
> </changelog>
> </subsection>
> <subsection name="Coyote">
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org