Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fixes #8926 - HttpClient GZIPContentDecoder should remove Content-Len… (#10326) 

Now Content-Length and Content-Encoding are removed/modified by the decoder.
In this way, applications have a correct sets of headers to decide whether to decode the content themselves.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
Simone Bordet 2023-08-21 08:58:22 +02:00 committed by GitHub
parent 003e46cae4
commit e91a68923a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 243 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
demos/embedded/src/test/java/org/eclipse/jetty/demos/ManyHandlersTest.java
View File

@ -15,10 +15,10 @@ package org.eclipse.jetty.demos;
import java.net.URI;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpHeaderValue;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.server.Server;
@ -53,9 +53,15 @@ public class ManyHandlersTest extends AbstractEmbeddedTest
{
URI uri = server.getURI().resolve("/params?a=b&foo=bar");
AtomicReference<String> contentEncoding = new AtomicReference<>();
ContentResponse response = client.newRequest(uri)
.method(HttpMethod.GET)
.headers(headers -> headers.put(HttpHeader.ACCEPT_ENCODING, HttpHeaderValue.GZIP))
.onResponseHeader((r, field) ->
{
if (field.getHeader() == HttpHeader.CONTENT_ENCODING)
contentEncoding.set(field.getValue());
return true;
})
.send();
assertThat("HTTP Response Status", response.getStatus(), is(HttpStatus.OK_200));
@ -63,8 +69,7 @@ public class ManyHandlersTest extends AbstractEmbeddedTest
// test gzip
// Test that Gzip was used to produce the response
String contentEncoding = response.getHeaders().get(HttpHeader.CONTENT_ENCODING);
assertThat("Content-Encoding", contentEncoding, containsString("gzip"));
assertThat("Content-Encoding", contentEncoding.get(), containsString("gzip"));
// test response content
String responseBody = response.getContentAsString();
@ -78,18 +83,25 @@ public class ManyHandlersTest extends AbstractEmbeddedTest
public void testGetHello() throws Exception
{
URI uri = server.getURI().resolve("/hello");
AtomicReference<String> contentEncoding = new AtomicReference<>();
ContentResponse response = client.newRequest(uri)
.method(HttpMethod.GET)
.headers(headers -> headers.put(HttpHeader.ACCEPT_ENCODING, HttpHeaderValue.GZIP))
.onResponseHeader((r, field) ->
{
if (field.getHeader() == HttpHeader.CONTENT_ENCODING)
contentEncoding.set(field.getValue());
return true;
})
.send();
assertThat("HTTP Response Status", response.getStatus(), is(HttpStatus.OK_200));
// dumpResponseHeaders(response);
// test gzip
// Test that Gzip was used to produce the response
String contentEncoding = response.getHeaders().get(HttpHeader.CONTENT_ENCODING);
assertThat("Content-Encoding", contentEncoding, containsString("gzip"));
assertThat("Content-Encoding", contentEncoding.get(), containsString("gzip"));
// test expected header from wrapper
String welcome = response.getHeaders().get("X-Welcome");

24
jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java
View File

@ -22,6 +22,18 @@ import java.nio.ByteBuffer;
*/
public interface ContentDecoder
{
/**
* <p>Processes the exchange just before the decoding of the response content.</p>
* <p>Typical processing may involve modifying the response headers, for example
* by temporarily removing the {@code Content-Length} header, or modifying the
* {@code Content-Encoding} header.</p>
*
* @param exchange the exchange to process before decoding the response content
*/
public default void beforeDecoding(HttpExchange exchange)
{
}
/**
* <p>Decodes the bytes in the given {@code buffer} and returns decoded bytes, if any.</p>
*
@ -39,6 +51,18 @@ public interface ContentDecoder
{
}
/**
* <p>Processes the exchange after the response content has been decoded.</p>
* <p>Typical processing may involve modifying the response headers, for example
* updating the {@code Content-Length} header to the length of the decoded
* response content.
*
* @param exchange the exchange to process after decoding the response content
*/
public default void afterDecoding(HttpExchange exchange)
{
}
/**
* Factory for {@link ContentDecoder}s; subclasses must implement {@link #newContentDecoder()}.
* <p>

46
jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java
View File

@ -14,7 +14,10 @@
package org.eclipse.jetty.client;
import java.nio.ByteBuffer;
import java.util.ListIterator;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.io.ByteBufferPool;
/**
@ -24,6 +27,8 @@ public class GZIPContentDecoder extends org.eclipse.jetty.http.GZIPContentDecode
{
public static final int DEFAULT_BUFFER_SIZE = 8192;
private long decodedLength;
public GZIPContentDecoder()
{
this(DEFAULT_BUFFER_SIZE);
@ -39,13 +44,54 @@ public class GZIPContentDecoder extends org.eclipse.jetty.http.GZIPContentDecode
super(byteBufferPool, bufferSize);
}
@Override
public void beforeDecoding(HttpExchange exchange)
{
exchange.getResponse().headers(headers ->
{
ListIterator<HttpField> iterator = headers.listIterator();
while (iterator.hasNext())
{
HttpField field = iterator.next();
HttpHeader header = field.getHeader();
if (header == HttpHeader.CONTENT_LENGTH)
{
// Content-Length is not valid anymore while we are decoding.
iterator.remove();
}
else if (header == HttpHeader.CONTENT_ENCODING)
{
// Content-Encoding should be removed/modified as the content will be decoded.
String value = field.getValue();
int comma = value.lastIndexOf(",");
if (comma < 0)
iterator.remove();
else
iterator.set(new HttpField(HttpHeader.CONTENT_ENCODING, value.substring(0, comma)));
break;
}
}
});
}
@Override
protected boolean decodedChunk(ByteBuffer chunk)
{
decodedLength += chunk.remaining();
super.decodedChunk(chunk);
return true;
}
@Override
public void afterDecoding(HttpExchange exchange)
{
exchange.getResponse().headers(headers ->
{
headers.remove(HttpHeader.TRANSFER_ENCODING);
headers.putLongField(HttpHeader.CONTENT_LENGTH, decodedLength);
});
}
/**
* Specialized {@link ContentDecoder.Factory} for the "gzip" encoding.
*/

57
jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java
View File

@ -31,8 +31,11 @@ import java.util.function.LongUnaryOperator;
import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.api.Result;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.QuotedCSV;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.MathUtils;
@ -286,33 +289,45 @@ public abstract class HttpReceiver
return false;
HttpResponse response = exchange.getResponse();
HttpFields responseHeaders = response.getHeaders();
if (LOG.isDebugEnabled())
LOG.debug("Response headers {}{}{}", response, System.lineSeparator(), response.getHeaders().toString().trim());
LOG.debug("Response headers {}{}{}", response, System.lineSeparator(), responseHeaders.toString().trim());
// HEAD responses may have Content-Encoding
// and Content-Length, but have no content.
if (!HttpMethod.HEAD.is(exchange.getRequest().getMethod()))
{
// Content-Encoding may have multiple values in the order they
// are applied, but we only support one decoding pass, the last one.
String contentEncoding = responseHeaders.get(HttpHeader.CONTENT_ENCODING);
if (contentEncoding != null)
{
int comma = contentEncoding.indexOf(",");
if (comma > 0)
{
QuotedCSV parser = new QuotedCSV(false);
parser.addValue(contentEncoding);
List<String> values = parser.getValues();
contentEncoding = values.get(values.size() - 1);
}
// If there is a matching content decoder factory, build a decoder.
for (ContentDecoder.Factory factory : getHttpDestination().getHttpClient().getContentDecoderFactories())
{
if (factory.getEncoding().equalsIgnoreCase(contentEncoding))
{
decoder = new Decoder(exchange, factory.newContentDecoder());
break;
}
}
}
}
ResponseNotifier notifier = getHttpDestination().getResponseNotifier();
List<Response.ResponseListener> responseListeners = exchange.getConversation().getResponseListeners();
notifier.notifyHeaders(responseListeners, response);
contentListeners.reset(responseListeners);
contentListeners.notifyBeforeContent(response);
if (!contentListeners.isEmpty())
{
List<String> contentEncodings = response.getHeaders().getCSV(HttpHeader.CONTENT_ENCODING.asString(), false);
if (contentEncodings != null && !contentEncodings.isEmpty())
{
for (ContentDecoder.Factory factory : getHttpDestination().getHttpClient().getContentDecoderFactories())
{
for (String encoding : contentEncodings)
{
if (factory.getEncoding().equalsIgnoreCase(encoding))
{
decoder = new Decoder(exchange, factory.newContentDecoder());
break;
}
}
}
}
}
if (updateResponseState(ResponseState.TRANSIENT, ResponseState.HEADERS))
{
boolean hasDemand = hasDemandOrStall();
@ -755,6 +770,7 @@ public abstract class HttpReceiver
{
this.exchange = exchange;
this.decoder = Objects.requireNonNull(decoder);
decoder.beforeDecoding(exchange);
}
private boolean decode(ByteBuffer encoded, Callback callback)
@ -868,6 +884,7 @@ public abstract class HttpReceiver
@Override
public void destroy()
{
decoder.afterDecoding(exchange);
if (decoder instanceof Destroyable)
((Destroyable)decoder).destroy();
}

6
jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java
View File

@ -422,6 +422,12 @@ public interface Request
Request onResponseHeader(Response.HeaderListener listener);
/**
* <p>Registers a listener for the headers event.</p>
* <p>Note that the response headers at this event
* may be different from the headers received in
* {@link #onResponseHeader(Response.HeaderListener)}
* as specified in {@link Response#getHeaders()}.</p>
*
* @param listener a listener for response headers event
* @return this request object
*/

12
jetty-client/src/main/java/org/eclipse/jetty/client/api/Response.java
View File

@ -65,6 +65,18 @@ public interface Response
String getReason();
/**
* <p>Returns the headers of this response.</p>
* <p>Some headers sent by the server may not be present,
* or be present but modified, while the content is being
* processed.
* A typical example is the {@code Content-Length} header
* when the content is sent compressed by the server and
* automatically decompressed by the client: the
* {@code Content-Length} header will be removed.</p>
* <p>Similarly, the {@code Content-Encoding} header
* may be removed or modified, as the content is
* decoded by the client.</p>
*
* @return the headers of this response
*/
HttpFields getHeaders();

88
jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java
View File

@ -13,6 +13,7 @@
package org.eclipse.jetty.client;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
@ -22,6 +23,8 @@ import java.util.Arrays;
import java.util.Random;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import java.util.zip.GZIPInputStream;
import java.util.zip.GZIPOutputStream;
import javax.servlet.ServletOutputStream;
import javax.servlet.http.HttpServletRequest;
@ -30,6 +33,7 @@ import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.util.InputStreamResponseListener;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.io.ByteBufferPool;
@ -43,6 +47,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.lessThan;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assumptions.assumeTrue;
@ -72,6 +77,81 @@ public class HttpClientGZIPTest extends AbstractHttpClientServerTest
assertEquals(200, response.getStatus());
assertArrayEquals(data, response.getContent());
HttpFields responseHeaders = response.getHeaders();
// The content has been decoded, so Content-Encoding must be absent.
assertNull(responseHeaders.get(HttpHeader.CONTENT_ENCODING));
// The Content-Length must be the decoded one.
assertEquals(data.length, responseHeaders.getLongField(HttpHeader.CONTENT_LENGTH));
}
@ParameterizedTest
@ArgumentsSource(ScenarioProvider.class)
public void testMultipleContentEncodingsFooGZIP(Scenario scenario) throws Exception
{
final byte[] data = "HELLO WORLD".getBytes(StandardCharsets.UTF_8);
start(scenario, new EmptyServerHandler()
{
@Override
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
response.setHeader("Content-Encoding", "foo,gzip");
GZIPOutputStream gzipOutput = new GZIPOutputStream(response.getOutputStream());
gzipOutput.write(data);
gzipOutput.finish();
}
});
ContentResponse response = client.newRequest("localhost", connector.getLocalPort())
.scheme(scenario.getScheme())
.timeout(5, TimeUnit.SECONDS)
.send();
assertEquals(200, response.getStatus());
assertArrayEquals(data, response.getContent());
HttpFields responseHeaders = response.getHeaders();
// The content has been decoded, so Content-Encoding must be only be "foo".
assertEquals("foo", responseHeaders.get(HttpHeader.CONTENT_ENCODING));
// The Content-Length must be the decoded one.
assertEquals(data.length, responseHeaders.getLongField(HttpHeader.CONTENT_LENGTH));
}
@ParameterizedTest
@ArgumentsSource(ScenarioProvider.class)
public void testMultipleContentEncodingsGZIPFoo(Scenario scenario) throws Exception
{
final byte[] data = "HELLO WORLD".getBytes(StandardCharsets.UTF_8);
start(scenario, new EmptyServerHandler()
{
@Override
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
response.setHeader("Content-Encoding", "gzip,foo");
GZIPOutputStream gzipOutput = new GZIPOutputStream(response.getOutputStream());
gzipOutput.write(data);
gzipOutput.finish();
}
});
// There is no "foo" content decoder factory, so content must remain gzipped.
AtomicLong encodedContentLength = new AtomicLong();
ContentResponse response = client.newRequest("localhost", connector.getLocalPort())
.scheme(scenario.getScheme())
.onResponseHeader((r, field) ->
{
if (field.getHeader() == HttpHeader.CONTENT_LENGTH)
encodedContentLength.set(field.getLongValue());
return true;
})
.timeout(5, TimeUnit.SECONDS)
.send();
assertEquals(200, response.getStatus());
byte[] content = IO.readBytes(new GZIPInputStream(new ByteArrayInputStream(response.getContent())));
assertArrayEquals(data, content);
HttpFields responseHeaders = response.getHeaders();
assertEquals("gzip,foo", responseHeaders.get(HttpHeader.CONTENT_ENCODING));
assertEquals(encodedContentLength.get(), responseHeaders.getLongField(HttpHeader.CONTENT_LENGTH));
}
@ParameterizedTest
@ -297,6 +377,12 @@ public class HttpClientGZIPTest extends AbstractHttpClientServerTest
Response response = listener.get(20, TimeUnit.SECONDS);
assertEquals(HttpStatus.OK_200, response.getStatus());
// No Content-Length because HttpClient does not know yet the length of the decoded content.
assertNull(response.getHeaders().get(HttpHeader.CONTENT_LENGTH));
// No Content-Encoding, because the content will be decoded automatically.
// In this way applications will know that the content is already un-gzipped
// and will not do the un-gzipping themselves.
assertNull(response.getHeaders().get(HttpHeader.CONTENT_ENCODING));
ByteArrayOutputStream output = new ByteArrayOutputStream();
try (InputStream input = listener.getInputStream())
@ -304,6 +390,8 @@ public class HttpClientGZIPTest extends AbstractHttpClientServerTest
IO.copy(input, output);
}
assertArrayEquals(content, output.toByteArray());
// After the content has been decoded, the length is known again.
assertEquals(content.length, response.getHeaders().getLongField(HttpHeader.CONTENT_LENGTH));
}
private static void sleep(long ms) throws IOException

11
jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/ContentLengthTest.java
View File

@ -20,13 +20,15 @@ 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.http.HttpMethod;
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.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.lessThan;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class ContentLengthTest extends AbstractTest
{
@ -94,7 +96,10 @@ public class ContentLengthTest extends AbstractTest
.send();
HttpFields responseHeaders = response.getHeaders();
long contentLength = responseHeaders.getLongField(HttpHeader.CONTENT_LENGTH.asString());
assertTrue(0 < contentLength && contentLength < data.length);
long contentLength = responseHeaders.getLongField(HttpHeader.CONTENT_LENGTH);
if (HttpMethod.HEAD.is(method))
assertThat(contentLength, lessThan((long)data.length));
else
assertEquals(data.length, contentLength);
}
}

5
tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DemoModulesTests.java
View File

@ -105,7 +105,6 @@ public class DemoModulesTests extends AbstractJettyHomeTest
String[] argsStart = {
"jetty.http.port=" + httpPort,
"jetty.httpConfig.port=" + httpsPort,
"jetty.ssl.port=" + httpsPort
};
@ -149,7 +148,6 @@ public class DemoModulesTests extends AbstractJettyHomeTest
String[] argsStart = {
"jetty.http.port=" + httpPort,
"jetty.httpConfig.port=" + httpsPort,
"jetty.ssl.port=" + httpsPort
};
@ -205,7 +203,6 @@ public class DemoModulesTests extends AbstractJettyHomeTest
String[] argsStart = {
"jetty.http.port=" + httpPort,
"jetty.httpConfig.port=" + httpsPort,
"jetty.ssl.port=" + httpsPort
};
@ -260,7 +257,6 @@ public class DemoModulesTests extends AbstractJettyHomeTest
String[] argsStart = {
"--jpms",
"jetty.http.port=" + httpPort,
"jetty.httpConfig.port=" + httpsPort,
"jetty.ssl.port=" + httpsPort
};
try (JettyHomeTester.Run runStart = distribution.start(argsStart))
@ -301,7 +297,6 @@ public class DemoModulesTests extends AbstractJettyHomeTest
int httpsPort = distribution.freePort();
String[] argsStart = {
"jetty.http.port=" + httpPort,
"jetty.httpConfig.port=" + httpsPort,
"jetty.ssl.port=" + httpsPort
};
try (JettyHomeTester.Run runStart = distribution.start(argsStart))

9
tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/GzipModuleTests.java
View File

@ -55,8 +55,7 @@ public class GzipModuleTests extends AbstractJettyHomeTest
assertEquals(0, runConfig.getExitValue());
String[] argsStart = {
"jetty.http.port=" + httpPort,
"jetty.httpConfig.port=" + httpPort
"jetty.http.port=" + httpPort
};
File war = distribution.resolveArtifact("org.eclipse.jetty.demos:demo-simple-webapp:war:" + jettyVersion);
@ -70,7 +69,7 @@ public class GzipModuleTests extends AbstractJettyHomeTest
ContentResponse response = client.GET("http://localhost:" + httpPort + "/demo/index.html");
String responseDetails = toResponseDetails(response);
assertEquals(HttpStatus.OK_200, response.getStatus(), responseDetails);
assertThat(responseDetails, response.getHeaders().get(HttpHeader.CONTENT_ENCODING), containsString("gzip"));
assertThat(responseDetails, containsString("Hello"));
}
}
}
@ -99,8 +98,7 @@ public class GzipModuleTests extends AbstractJettyHomeTest
assertEquals(0, runConfig.getExitValue());
String[] argsStart = {
"jetty.http.port=" + httpPort,
"jetty.httpConfig.port=" + httpPort
"jetty.http.port=" + httpPort
};
File war = distribution.resolveArtifact("org.eclipse.jetty.demos:demo-simple-webapp:war:" + jettyVersion);
@ -145,7 +143,6 @@ public class GzipModuleTests extends AbstractJettyHomeTest
String[] argsStart = {
"jetty.http.port=" + httpPort,
"jetty.httpConfig.port=" + httpPort,
"jetty.gzip.excludedMimeTypeList=image/vnd.microsoft.icon"
};