Re: [tomcat] branch master updated: Fix BZ 64403 Compressed h2 responses should not have C-L header
On 05/05/2020 06:38, Martin Grigorov wrote: > On Mon, May 4, 2020 at 11:06 PM > > + // Enable compression for the LargeServlet > > > What is LargeServlet ? > There are only SimpleServlet and CompressionServlet in this test. Ah. The comment is out of date. I started writing the text case using LargeServlet but found it was easier to use a new Servlet to generate the output to be compressed than handle all the differences when enabling compression for LargeServlet. I'll correct the comment. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] branch master updated: Fix BZ 64403 Compressed h2 responses should not have C-L header
On Mon, May 4, 2020 at 11:06 PM wrote: > This is an automated email from the ASF dual-hosted git repository. > > markt pushed a commit to branch master > in repository https://gitbox.apache.org/repos/asf/tomcat.git > > > The following commit(s) were added to refs/heads/master by this push: > new 956d9f7 Fix BZ 64403 Compressed h2 responses should not have C-L > header > 956d9f7 is described below > > commit 956d9f70279f35da45d63922607b328f44a5f776 > Author: Mark Thomas > AuthorDate: Mon May 4 21:06:16 2020 +0100 > > Fix BZ 64403 Compressed h2 responses should not have C-L header > > https://bz.apache.org/bugzilla/show_bug.cgi?id=64403 > --- > java/org/apache/coyote/http2/StreamProcessor.java | 19 ++--- > .../apache/coyote/http2/TestStreamProcessor.java | 81 > ++ > webapps/docs/changelog.xml | 5 ++ > 3 files changed, 97 insertions(+), 8 deletions(-) > > diff --git a/java/org/apache/coyote/http2/StreamProcessor.java > b/java/org/apache/coyote/http2/StreamProcessor.java > index 15bfcab..e03f307 100644 > --- a/java/org/apache/coyote/http2/StreamProcessor.java > +++ b/java/org/apache/coyote/http2/StreamProcessor.java > @@ -147,6 +147,17 @@ class StreamProcessor extends AbstractProcessor { > // Add the pseudo header for status > > headers.addValue(":status").setString(Integer.toString(statusCode)); > > + > +// Compression can't be used with sendfile > +// Need to check for compression (and set headers appropriately) > before > +// adding headers below > +if (noSendfile && protocol != null && > +protocol.useCompression(coyoteRequest, coyoteResponse)) { > +// Enable compression. Headers will have been set. Need to > configure > +// output filter at this point. > +stream.addOutputFilter(new GzipOutputFilter()); > +} > + > // Check to see if a response body is present > if (!(statusCode < 200 || statusCode == 204 || statusCode == 205 > || statusCode == 304)) { > String contentType = coyoteResponse.getContentType(); > @@ -178,14 +189,6 @@ class StreamProcessor extends AbstractProcessor { > if (statusCode >= 200 && headers.getValue("date") == null) { > > headers.addValue("date").setString(FastHttpDateFormat.getCurrentDate()); > } > - > -// Compression can't be used with sendfile > -if (noSendfile && protocol != null && > -protocol.useCompression(coyoteRequest, coyoteResponse)) { > -// Enable compression. Headers will have been set. Need to > configure > -// output filter at this point. > -stream.addOutputFilter(new GzipOutputFilter()); > -} > } > > > diff --git a/test/org/apache/coyote/http2/TestStreamProcessor.java > b/test/org/apache/coyote/http2/TestStreamProcessor.java > index 214a65e..e72be5f 100644 > --- a/test/org/apache/coyote/http2/TestStreamProcessor.java > +++ b/test/org/apache/coyote/http2/TestStreamProcessor.java > @@ -18,6 +18,7 @@ package org.apache.coyote.http2; > > import java.io.File; > import java.io.IOException; > +import java.io.OutputStream; > import java.io.PrintWriter; > import java.nio.ByteBuffer; > import java.util.ArrayList; > @@ -34,6 +35,7 @@ import org.junit.Test; > > import org.apache.catalina.Context; > import org.apache.catalina.Wrapper; > +import org.apache.catalina.connector.Connector; > import org.apache.catalina.startup.Tomcat; > import org.apache.tomcat.util.compat.JrePlatform; > import org.apache.tomcat.util.http.FastHttpDateFormat; > @@ -223,4 +225,83 @@ public class TestStreamProcessor extends > Http2TestBase { > }); > } > } > + > + > +@Test > +public void testCompression() throws Exception { > +enableHttp2(); > + > +Tomcat tomcat = getTomcatInstance(); > + > +Context ctxt = tomcat.addContext("", null); > +Tomcat.addServlet(ctxt, "simple", new SimpleServlet()); > +ctxt.addServletMappingDecoded("/simple", "simple"); > +Tomcat.addServlet(ctxt, "compression", new CompressionServlet()); > +ctxt.addServletMappingDecoded("/compression", "compression"); > + > > +// Enable compression for the LargeServlet > What is LargeServlet ? There are only SimpleServlet and CompressionServlet in this test. > +Connector connector = tomcat.getConnector(); > +Assert.assertTrue(connector.setProperty("compression", "on")); > + > +tomcat.start(); > + > +enableHttp2(); > +openClientConnection(); > +doHttpUpgrade(); > +sendClientPreface(); > +validateHttp2InitialResponse(); > + > + > +byte[] frameHeader = new byte[9]; > +ByteBuffer headersPayload = ByteBuffer.allocate(128); > + > +List headers = new ArrayList<>(3); > +headers.add(new Header(":method", "GET")); > +headers.add(new Header(":schem
[tomcat] branch master updated: Fix BZ 64403 Compressed h2 responses should not have C-L header
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/master by this push: new 956d9f7 Fix BZ 64403 Compressed h2 responses should not have C-L header 956d9f7 is described below commit 956d9f70279f35da45d63922607b328f44a5f776 Author: Mark Thomas AuthorDate: Mon May 4 21:06:16 2020 +0100 Fix BZ 64403 Compressed h2 responses should not have C-L header https://bz.apache.org/bugzilla/show_bug.cgi?id=64403 --- java/org/apache/coyote/http2/StreamProcessor.java | 19 ++--- .../apache/coyote/http2/TestStreamProcessor.java | 81 ++ webapps/docs/changelog.xml | 5 ++ 3 files changed, 97 insertions(+), 8 deletions(-) diff --git a/java/org/apache/coyote/http2/StreamProcessor.java b/java/org/apache/coyote/http2/StreamProcessor.java index 15bfcab..e03f307 100644 --- a/java/org/apache/coyote/http2/StreamProcessor.java +++ b/java/org/apache/coyote/http2/StreamProcessor.java @@ -147,6 +147,17 @@ class StreamProcessor extends AbstractProcessor { // Add the pseudo header for status headers.addValue(":status").setString(Integer.toString(statusCode)); + +// Compression can't be used with sendfile +// Need to check for compression (and set headers appropriately) before +// adding headers below +if (noSendfile && protocol != null && +protocol.useCompression(coyoteRequest, coyoteResponse)) { +// Enable compression. Headers will have been set. Need to configure +// output filter at this point. +stream.addOutputFilter(new GzipOutputFilter()); +} + // Check to see if a response body is present if (!(statusCode < 200 || statusCode == 204 || statusCode == 205 || statusCode == 304)) { String contentType = coyoteResponse.getContentType(); @@ -178,14 +189,6 @@ class StreamProcessor extends AbstractProcessor { if (statusCode >= 200 && headers.getValue("date") == null) { headers.addValue("date").setString(FastHttpDateFormat.getCurrentDate()); } - -// Compression can't be used with sendfile -if (noSendfile && protocol != null && -protocol.useCompression(coyoteRequest, coyoteResponse)) { -// Enable compression. Headers will have been set. Need to configure -// output filter at this point. -stream.addOutputFilter(new GzipOutputFilter()); -} } diff --git a/test/org/apache/coyote/http2/TestStreamProcessor.java b/test/org/apache/coyote/http2/TestStreamProcessor.java index 214a65e..e72be5f 100644 --- a/test/org/apache/coyote/http2/TestStreamProcessor.java +++ b/test/org/apache/coyote/http2/TestStreamProcessor.java @@ -18,6 +18,7 @@ package org.apache.coyote.http2; import java.io.File; import java.io.IOException; +import java.io.OutputStream; import java.io.PrintWriter; import java.nio.ByteBuffer; import java.util.ArrayList; @@ -34,6 +35,7 @@ import org.junit.Test; import org.apache.catalina.Context; import org.apache.catalina.Wrapper; +import org.apache.catalina.connector.Connector; import org.apache.catalina.startup.Tomcat; import org.apache.tomcat.util.compat.JrePlatform; import org.apache.tomcat.util.http.FastHttpDateFormat; @@ -223,4 +225,83 @@ public class TestStreamProcessor extends Http2TestBase { }); } } + + +@Test +public void testCompression() throws Exception { +enableHttp2(); + +Tomcat tomcat = getTomcatInstance(); + +Context ctxt = tomcat.addContext("", null); +Tomcat.addServlet(ctxt, "simple", new SimpleServlet()); +ctxt.addServletMappingDecoded("/simple", "simple"); +Tomcat.addServlet(ctxt, "compression", new CompressionServlet()); +ctxt.addServletMappingDecoded("/compression", "compression"); + +// Enable compression for the LargeServlet +Connector connector = tomcat.getConnector(); +Assert.assertTrue(connector.setProperty("compression", "on")); + +tomcat.start(); + +enableHttp2(); +openClientConnection(); +doHttpUpgrade(); +sendClientPreface(); +validateHttp2InitialResponse(); + + +byte[] frameHeader = new byte[9]; +ByteBuffer headersPayload = ByteBuffer.allocate(128); + +List headers = new ArrayList<>(3); +headers.add(new Header(":method", "GET")); +headers.add(new Header(":scheme", "http")); +headers.add(new Header(":path", "/compression")); +headers.add(new Header(":authority", "localhost:" + getPort())); +headers.add(new Header("accept-encoding", "gzip")); + +buildGetRequest(frameHeader, headersPayload, null, headers, 3); + +writeFrame(frameHeader, headersPayload); + +rea