From dc89f7f264c1503d7a4a70fbdcf23024fe2bd504 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 13 Apr 2020 17:57:38 +0200 Subject: [PATCH 1/3] Fixes #4764 - HTTP2 Jetty Server does not send back content-length. Sending Content-Length header if known at the time of sending the response headers. Signed-off-by: Simone Bordet --- .../http2/client/http/ContentLengthTest.java | 105 ++++++++++++++++++ .../http2/server/HttpTransportOverHTTP2.java | 8 ++ 2 files changed, 113 insertions(+) create mode 100644 jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/ContentLengthTest.java diff --git a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/ContentLengthTest.java b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/ContentLengthTest.java new file mode 100644 index 00000000000..6d0bb511513 --- /dev/null +++ b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/ContentLengthTest.java @@ -0,0 +1,105 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.http2.client.http; + +import java.io.IOException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.client.api.ContentResponse; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.handler.gzip.GzipHandler; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class ContentLengthTest extends AbstractTest +{ + @ParameterizedTest + @ValueSource(strings = {"GET", "HEAD", "POST", "PUT"}) + public void testZeroContentLengthAddedByServer(String method) throws Exception + { + start(new EmptyServerHandler()); + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .method(method) + .send(); + + HttpFields responseHeaders = response.getHeaders(); + long contentLength = responseHeaders.getLongField(HttpHeader.CONTENT_LENGTH.asString()); + assertEquals(0, contentLength); + } + + @ParameterizedTest + @ValueSource(strings = {"GET", "HEAD", "POST", "PUT"}) + public void testContentLengthAddedByServer(String method) throws Exception + { + byte[] data = new byte[512]; + start(new EmptyServerHandler() + { + @Override + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + { + response.getOutputStream().write(data); + } + }); + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .method(method) + .send(); + + HttpFields responseHeaders = response.getHeaders(); + long contentLength = responseHeaders.getLongField(HttpHeader.CONTENT_LENGTH.asString()); + assertEquals(data.length, contentLength); + } + + @ParameterizedTest + @ValueSource(strings = {"GET", "HEAD", "POST", "PUT"}) + public void testGzippedContentLengthAddedByServer(String method) throws Exception + { + byte[] data = new byte[4096]; + + GzipHandler gzipHandler = new GzipHandler(); + gzipHandler.addIncludedMethods(method); + gzipHandler.setMinGzipSize(data.length / 2); + gzipHandler.setHandler(new EmptyServerHandler() + { + @Override + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + { + response.setContentLength(data.length); + response.getOutputStream().write(data); + } + }); + + start(gzipHandler); + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .method(method) + .send(); + + HttpFields responseHeaders = response.getHeaders(); + long contentLength = responseHeaders.getLongField(HttpHeader.CONTENT_LENGTH.asString()); + assertTrue(0 < contentLength && contentLength < data.length); + } +} diff --git a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java index 6504977d5d9..a19a8fa071e 100644 --- a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java +++ b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java @@ -23,6 +23,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; @@ -110,6 +111,13 @@ public class HttpTransportOverHTTP2 implements HttpTransport { if (commit.compareAndSet(false, true)) { + if (lastContent) + { + HttpFields responseHeaders = info.getFields(); + if (!responseHeaders.contains(HttpHeader.CONTENT_LENGTH)) + responseHeaders.put(HttpHeader.CONTENT_LENGTH, String.valueOf(BufferUtil.length(content))); + } + if (hasContent) { Callback commitCallback = new Callback.Nested(callback) From 2e85b3e169e30d604b1af11a7518086e5545b0ce Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 14 Apr 2020 11:44:58 +0200 Subject: [PATCH 2/3] Fixes #4764 - HTTP2 Jetty Server does not send back content-length. Updates after review. Now the Content-Length header is generated by HpackEncoder based on MetaData.contentLength, so that the MetaData.HttpFields are not modified. Signed-off-by: Simone Bordet --- .../java/org/eclipse/jetty/http/MetaData.java | 5 +++++ .../eclipse/jetty/http2/hpack/HpackEncoder.java | 14 +++++++++++++- .../http2/server/HttpTransportOverHTTP2.java | 16 ++++++++++++---- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java b/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java index 7591f62e291..8035b086b72 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java @@ -119,6 +119,11 @@ public class MetaData implements Iterable return _contentLength; } + public void setContentLength(long contentLength) + { + _contentLength = contentLength; + } + /** * @return an iterator over the HTTP fields * @see #getFields() diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java index d71df43f044..0788806b34b 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java @@ -217,7 +217,8 @@ public class HpackEncoder // Remove fields as specified in RFC 7540, 8.1.2.2. if (fields != null) { - // For example: Connection: Close, TE, Upgrade, Custom. + // Remove the headers specified in the Connection header, + // for example: Connection: Close, TE, Upgrade, Custom. Set hopHeaders = null; for (String value : fields.getCSV(HttpHeader.CONNECTION, false)) { @@ -225,6 +226,8 @@ public class HpackEncoder hopHeaders = new HashSet<>(); hopHeaders.add(StringUtil.asciiToLowerCase(value)); } + + boolean contentLengthEncoded = false; for (HttpField field : fields) { HttpHeader header = field.getHeader(); @@ -239,8 +242,17 @@ public class HpackEncoder String name = field.getLowerCaseName(); if (hopHeaders != null && hopHeaders.contains(name)) continue; + if (header == HttpHeader.CONTENT_LENGTH) + contentLengthEncoded = true; encode(buffer, field); } + + if (!contentLengthEncoded) + { + long contentLength = metadata.getContentLength(); + if (contentLength >= 0) + encode(buffer, new HttpField(HttpHeader.CONTENT_LENGTH, String.valueOf(contentLength))); + } } // Check size diff --git a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java index a19a8fa071e..03a9812c588 100644 --- a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java +++ b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java @@ -22,8 +22,8 @@ import java.nio.ByteBuffer; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; +import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HttpFields; -import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; @@ -113,9 +113,17 @@ public class HttpTransportOverHTTP2 implements HttpTransport { if (lastContent) { - HttpFields responseHeaders = info.getFields(); - if (!responseHeaders.contains(HttpHeader.CONTENT_LENGTH)) - responseHeaders.put(HttpHeader.CONTENT_LENGTH, String.valueOf(BufferUtil.length(content))); + long realContentLength = BufferUtil.length(content); + long contentLength = info.getContentLength(); + if (contentLength < 0) + { + info.setContentLength(realContentLength); + } + else if (contentLength != realContentLength) + { + callback.failed(new BadMessageException(HttpStatus.INTERNAL_SERVER_ERROR_500, String.format("Incorrect Content-Length %d!=%d", contentLength, realContentLength))); + return; + } } if (hasContent) From 0e30d6b7515df97c02f99c589b43b513043ac920 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 14 Apr 2020 13:05:24 +0200 Subject: [PATCH 3/3] Fixes #4764 - HTTP2 Jetty Server does not send back content-length. Fixed InterleavingTest that was using the wrong MetaData.Response constructor. Fixed handling of HEAD methods in HttpTransportOverHTTP2. Signed-off-by: Simone Bordet --- .../java/org/eclipse/jetty/http2/client/InterleavingTest.java | 4 ++-- .../eclipse/jetty/http2/server/HttpTransportOverHTTP2.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/InterleavingTest.java b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/InterleavingTest.java index ff020d06c1c..5bedd7decc8 100644 --- a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/InterleavingTest.java +++ b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/InterleavingTest.java @@ -111,7 +111,7 @@ public class InterleavingTest extends AbstractTest Stream serverStream1 = serverStreams.get(0); Stream serverStream2 = serverStreams.get(1); - MetaData.Response response1 = new MetaData.Response(HttpVersion.HTTP_2, HttpStatus.OK_200, new HttpFields(), 0); + MetaData.Response response1 = new MetaData.Response(HttpVersion.HTTP_2, HttpStatus.OK_200, new HttpFields()); serverStream1.headers(new HeadersFrame(serverStream1.getId(), response1, null, false), Callback.NOOP); Random random = new Random(); @@ -120,7 +120,7 @@ public class InterleavingTest extends AbstractTest byte[] content2 = new byte[2 * ((ISession)serverStream2.getSession()).updateSendWindow(0)]; random.nextBytes(content2); - MetaData.Response response2 = new MetaData.Response(HttpVersion.HTTP_2, HttpStatus.OK_200, new HttpFields(), 0); + MetaData.Response response2 = new MetaData.Response(HttpVersion.HTTP_2, HttpStatus.OK_200, new HttpFields()); serverStream2.headers(new HeadersFrame(serverStream2.getId(), response2, null, false), new Callback() { @Override diff --git a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java index 03a9812c588..330b6475043 100644 --- a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java +++ b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java @@ -119,7 +119,7 @@ public class HttpTransportOverHTTP2 implements HttpTransport { info.setContentLength(realContentLength); } - else if (contentLength != realContentLength) + else if (hasContent && contentLength != realContentLength) { callback.failed(new BadMessageException(HttpStatus.INTERNAL_SERVER_ERROR_500, String.format("Incorrect Content-Length %d!=%d", contentLength, realContentLength))); return;