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 2020/05/04 20:06:49 UTC
[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 <ma...@apache.org>
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<Header> 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);
+
+ readSimpleGetResponse();
+
+ Assert.assertEquals(
+ "3-HeadersStart\n" +
+ "3-Header-[:status]-[200]\n" +
+ "3-Header-[vary]-[accept-encoding]\n" +
+ "3-Header-[content-encoding]-[gzip]\n" +
+ "3-Header-[content-type]-[text/plain;charset=UTF-8]\n" +
+ "3-Header-[date]-[Wed, 11 Nov 2015 19:18:42 GMT]\n" +
+ "3-HeadersEnd\n" +
+ "3-Body-97\n" +
+ "3-EndOfStream\n", output.getTrace());
+ }
+
+
+ private static class CompressionServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+ throws ServletException, IOException {
+ // Generate content type that is compressible
+ resp.setContentType("text/plain");
+ resp.setCharacterEncoding("UTF-8");
+
+ // Make ir large enough to trigger compression
+ int count = 64 * 1024;
+
+ // One bytes per entry
+ resp.setContentLengthLong(count);
+
+ OutputStream os = resp.getOutputStream();
+ for (int i = 0; i < count; i++) {
+ os.write('X');
+ }
+ }
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 02cb6ec..887d93b 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -131,6 +131,11 @@
keep-alive where the response has a non-2xx status code but the request
body has been fully read. (rjung/markt)
</fix>
+ <fix>
+ <bug>64403</bug>: Ensure that compressed HTTP/2 responses are not sent
+ with a content length header appropriate for the original, uncompressed
+ response. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch master updated: Fix BZ 64403 Compressed h2
responses should not have C-L header
Posted by Mark Thomas <ma...@apache.org>.
On 05/05/2020 06:38, Martin Grigorov wrote:
> On Mon, May 4, 2020 at 11:06 PM <markt@apache.org
<snip/>
>
> + // 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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch master updated: Fix BZ 64403 Compressed h2
responses should not have C-L header
Posted by Martin Grigorov <mg...@apache.org>.
On Mon, May 4, 2020 at 11:06 PM <ma...@apache.org> 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 <ma...@apache.org>
> 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");
> +
>
<snip/>
> + // 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<Header> 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);
> +
> + readSimpleGetResponse();
> +
> + Assert.assertEquals(
> + "3-HeadersStart\n" +
> + "3-Header-[:status]-[200]\n" +
> + "3-Header-[vary]-[accept-encoding]\n" +
> + "3-Header-[content-encoding]-[gzip]\n" +
> + "3-Header-[content-type]-[text/plain;charset=UTF-8]\n" +
> + "3-Header-[date]-[Wed, 11 Nov 2015 19:18:42 GMT]\n" +
> + "3-HeadersEnd\n" +
> + "3-Body-97\n" +
> + "3-EndOfStream\n", output.getTrace());
> + }
> +
> +
> + private static class CompressionServlet extends HttpServlet {
> +
> + private static final long serialVersionUID = 1L;
> +
> + @Override
> + protected void doGet(HttpServletRequest req, HttpServletResponse
> resp)
> + throws ServletException, IOException {
> + // Generate content type that is compressible
> + resp.setContentType("text/plain");
> + resp.setCharacterEncoding("UTF-8");
> +
> + // Make ir large enough to trigger compression
> + int count = 64 * 1024;
> +
> + // One bytes per entry
> + resp.setContentLengthLong(count);
> +
> + OutputStream os = resp.getOutputStream();
> + for (int i = 0; i < count; i++) {
> + os.write('X');
> + }
> + }
> + }
> }
> diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
> index 02cb6ec..887d93b 100644
> --- a/webapps/docs/changelog.xml
> +++ b/webapps/docs/changelog.xml
> @@ -131,6 +131,11 @@
> keep-alive where the response has a non-2xx status code but the
> request
> body has been fully read. (rjung/markt)
> </fix>
> + <fix>
> + <bug>64403</bug>: Ensure that compressed HTTP/2 responses are not
> sent
> + with a content length header appropriate for the original,
> uncompressed
> + response. (markt)
> + </fix>
> </changelog>
> </subsection>
> <subsection name="Jasper">
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>