Re: svn commit: r1824297 - in /tomcat/trunk: java/org/apache/coyote/CompressionConfig.java webapps/docs/changelog.xml
-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: >> Author: markt Date: Thu Feb 15 11:38:11 2018 New Revision: >> 1824297 >> >> URL: http://svn.apache.org/viewvc?rev=1824297=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=1824296=1824297=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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: svn commit: r1824297 - in /tomcat/trunk: java/org/apache/coyote/CompressionConfig.java webapps/docs/changelog.xml
On 15/02/18 13:14, Konstantin Kolinko wrote: > 2018-02-15 14:38 GMT+03:00: >> Author: markt >> Date: Thu Feb 15 11:38:11 2018 >> New Revision: 1824297 >> >> URL: http://svn.apache.org/viewvc?rev=1824297=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=1824296=1824297=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=1824296=1824297=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 >> ErrorReportValve. (markt) >> >> + >> +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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: svn commit: r1824297 - in /tomcat/trunk: java/org/apache/coyote/CompressionConfig.java webapps/docs/changelog.xml
2018-02-15 14:38 GMT+03:00: > Author: markt > Date: Thu Feb 15 11:38:11 2018 > New Revision: 1824297 > > URL: http://svn.apache.org/viewvc?rev=1824297=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=1824296=1824297=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=1824296=1824297=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 > ErrorReportValve. (markt) > > + > +Prevent Tomcat from applying gzip compression to content that is > already > +compressed with brotli compression. Patch provided by burka. (markt) Based on patch provided... > + > > > > > > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org