Skip to content

Commit

Permalink
Merge pull request #8906 from eclipse/fix/jetty-10-gziphandler-status…
Browse files Browse the repository at this point in the history 
…-304-vary

Issue #8905 - GzipHandler should include `Vary` header on 304 (Not Modified) responses (per RFC9110)
  • Loading branch information
@joakime
joakime authored Nov 21, 2022
2 parents 83154b4 + cf01934 commit 4466657
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 28 deletions.
2 changes: 2 additions & 0 deletions ...server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ protected void commit(ByteBuffer content, boolean complete, Callback callback)
String responseEtagGzip = etagGzip(responseEtag);
if (requestEtags.contains(responseEtagGzip))
response.getHttpFields().put(HttpHeader.ETAG, responseEtagGzip);
if (_vary != null)
response.getHttpFields().ensureField(_vary);
}
}

Expand Down
95 changes: 67 additions & 28 deletions jetty-servlet/src/test/java/org/eclipse/jetty/servlet/GzipHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.eclipse.jetty.http.CompressedContentFormat;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.server.HttpOutput;
import org.eclipse.jetty.server.LocalConnector;
import org.eclipse.jetty.server.Server;
Expand Down Expand Up @@ -335,13 +336,13 @@ public void testNotGzipHandler() throws Exception
assertThat(response.getStatus(), is(200));
assertThat(response.get("Content-Encoding"), not(equalToIgnoringCase("gzip")));
assertThat(response.get("ETag"), is(__contentETag));
assertThat(response.getCSV("Vary", false), Matchers.contains("Other", "Accept-Encoding"));
assertThat(response.getCSV("Vary", false), contains("Other", "Accept-Encoding"));

InputStream testIn = new ByteArrayInputStream(response.getContentBytes());
ByteArrayOutputStream testOut = new ByteArrayOutputStream();
IO.copy(testIn, testOut);

assertEquals(__content, testOut.toString("UTF8"));
assertEquals(__content, testOut.toString(StandardCharsets.UTF_8));
}

@Test
Expand All @@ -360,15 +361,53 @@ public void testBlockingResponse() throws Exception
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));

assertThat(response.getStatus(), is(200));
assertThat(response.get("Content-Encoding"), Matchers.equalToIgnoringCase("gzip"));
assertThat(response.get("Content-Encoding"), equalToIgnoringCase("gzip"));
assertThat(response.get("ETag"), is(__contentETagGzip));
assertThat(response.getCSV("Vary", false), Matchers.contains("Accept-Encoding", "Other"));
assertThat(response.getCSV("Vary", false), contains("Accept-Encoding", "Other"));

InputStream testIn = new GZIPInputStream(new ByteArrayInputStream(response.getContentBytes()));
ByteArrayOutputStream testOut = new ByteArrayOutputStream();
IO.copy(testIn, testOut);

assertEquals(__content, testOut.toString("UTF8"));
assertEquals(__content, testOut.toString(StandardCharsets.UTF_8));
}

@Test
public void testGzipNotModifiedVaryHeader() throws Exception
{
HttpTester.Request request;
HttpTester.Response response;

// Request for content, initial
request = HttpTester.newRequest();
request.setMethod("GET");
request.setURI("/ctx/content");
request.setVersion(HttpVersion.HTTP_1_1);
request.setHeader("Host", "tester");
request.setHeader("Accept-Encoding", "gzip");

response = HttpTester.parseResponse(_connector.getResponse(request.generate()));

assertThat(response.getStatus(), is(HttpStatus.OK_200));
assertThat(response.get("Content-Encoding"), equalToIgnoringCase("gzip"));
assertThat(response.get("ETag"), is(__contentETagGzip));
assertThat(response.getCSV("Vary", false), contains("Accept-Encoding"));

// Now request with `If-None-Match`
request = HttpTester.newRequest();
request.setMethod("GET");
request.setURI("/ctx/content");
request.setVersion(HttpVersion.HTTP_1_1);
request.setHeader("If-None-Match", __contentETagGzip);
request.setHeader("Host", "tester");
request.setHeader("Accept-Encoding", "gzip");

response = HttpTester.parseResponse(_connector.getResponse(request.generate()));

assertThat(response.getStatus(), is(HttpStatus.NOT_MODIFIED_304));
assertThat(response.get("Content-Encoding"), not(containsString("gzip")));
assertThat(response.get("ETag"), is(__contentETagGzip));
assertThat(response.getCSV("Vary", false), contains("Accept-Encoding"));
}

@Test
Expand All @@ -387,14 +426,14 @@ public void testAsyncResponse() throws Exception
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));

assertThat(response.getStatus(), is(200));
assertThat(response.get("Content-Encoding"), Matchers.equalToIgnoringCase("gzip"));
assertThat(response.getCSV("Vary", false), Matchers.contains("Accept-Encoding"));
assertThat(response.get("Content-Encoding"), equalToIgnoringCase("gzip"));
assertThat(response.getCSV("Vary", false), contains("Accept-Encoding"));

InputStream testIn = new GZIPInputStream(new ByteArrayInputStream(response.getContentBytes()));
ByteArrayOutputStream testOut = new ByteArrayOutputStream();
IO.copy(testIn, testOut);

assertEquals(__content, testOut.toString("UTF8"));
assertEquals(__content, testOut.toString(StandardCharsets.UTF_8));
}

@Test
Expand All @@ -413,14 +452,14 @@ public void testBufferResponse() throws Exception
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));

assertThat(response.getStatus(), is(200));
assertThat(response.get("Content-Encoding"), Matchers.equalToIgnoringCase("gzip"));
assertThat(response.getCSV("Vary", false), Matchers.contains("Accept-Encoding"));
assertThat(response.get("Content-Encoding"), equalToIgnoringCase("gzip"));
assertThat(response.getCSV("Vary", false), contains("Accept-Encoding"));

InputStream testIn = new GZIPInputStream(new ByteArrayInputStream(response.getContentBytes()));
ByteArrayOutputStream testOut = new ByteArrayOutputStream();
IO.copy(testIn, testOut);

assertEquals(__content, testOut.toString("UTF8"));
assertEquals(__content, testOut.toString(StandardCharsets.UTF_8));
}

@Test
Expand All @@ -440,8 +479,8 @@ public void testAsyncLargeResponse() throws Exception
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));

assertThat(response.getStatus(), is(200));
assertThat(response.get("Content-Encoding"), Matchers.equalToIgnoringCase("gzip"));
assertThat(response.getCSV("Vary", false), Matchers.contains("Accept-Encoding"));
assertThat(response.get("Content-Encoding"), equalToIgnoringCase("gzip"));
assertThat(response.getCSV("Vary", false), contains("Accept-Encoding"));

InputStream testIn = new GZIPInputStream(new ByteArrayInputStream(response.getContentBytes()));
ByteArrayOutputStream testOut = new ByteArrayOutputStream();
Expand Down Expand Up @@ -474,8 +513,8 @@ public void testAsyncEmptyResponse() throws Exception
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));

assertThat(response.getStatus(), is(200));
assertThat(response.get("Content-Encoding"), Matchers.equalToIgnoringCase("gzip"));
assertThat(response.getCSV("Vary", false), Matchers.contains("Accept-Encoding"));
assertThat(response.get("Content-Encoding"), equalToIgnoringCase("gzip"));
assertThat(response.getCSV("Vary", false), contains("Accept-Encoding"));
}

@Test
Expand All @@ -495,15 +534,15 @@ public void testGzipHandlerWithMultipleAcceptEncodingHeaders() throws Exception
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));

assertThat(response.getStatus(), is(200));
assertThat(response.get("Content-Encoding"), Matchers.equalToIgnoringCase("gzip"));
assertThat(response.get("Content-Encoding"), equalToIgnoringCase("gzip"));
assertThat(response.get("ETag"), is(__contentETagGzip));
assertThat(response.getCSV("Vary", false), Matchers.contains("Accept-Encoding", "Other"));
assertThat(response.getCSV("Vary", false), contains("Accept-Encoding", "Other"));

InputStream testIn = new GZIPInputStream(new ByteArrayInputStream(response.getContentBytes()));
ByteArrayOutputStream testOut = new ByteArrayOutputStream();
IO.copy(testIn, testOut);

assertEquals(__content, testOut.toString("UTF8"));
assertEquals(__content, testOut.toString(StandardCharsets.UTF_8));
}

@Test
Expand All @@ -530,7 +569,7 @@ public void testGzipNotMicro() throws Exception
ByteArrayOutputStream testOut = new ByteArrayOutputStream();
IO.copy(testIn, testOut);

assertEquals(__micro, testOut.toString("UTF8"));
assertEquals(__micro, testOut.toString(StandardCharsets.UTF_8));
}

@Test
Expand Down Expand Up @@ -559,7 +598,7 @@ public void testGzipNotMicroChunked() throws Exception
ByteArrayOutputStream testOut = new ByteArrayOutputStream();
IO.copy(testIn, testOut);

assertEquals(__micro, testOut.toString("UTF8"));
assertEquals(__micro, testOut.toString(StandardCharsets.UTF_8));
}

@Test
Expand All @@ -579,7 +618,7 @@ public void testETagNotGzipHandler() throws Exception
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));

assertThat(response.getStatus(), is(304));
assertThat(response.get("Content-Encoding"), not(Matchers.equalToIgnoringCase("gzip")));
assertThat(response.get("Content-Encoding"), not(equalToIgnoringCase("gzip")));
assertThat(response.get("ETag"), is(__contentETag));
}

Expand All @@ -600,7 +639,7 @@ public void testETagGzipHandler() throws Exception
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));

assertThat(response.getStatus(), is(304));
assertThat(response.get("Content-Encoding"), not(Matchers.equalToIgnoringCase("gzip")));
assertThat(response.get("Content-Encoding"), not(equalToIgnoringCase("gzip")));
assertThat(response.get("ETag"), is(__contentETagGzip));
}

Expand All @@ -620,7 +659,7 @@ public void testDeleteETagGzipHandler() throws Exception
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));

assertThat(response.getStatus(), is(HttpServletResponse.SC_NOT_MODIFIED));
assertThat(response.get("Content-Encoding"), not(Matchers.equalToIgnoringCase("gzip")));
assertThat(response.get("Content-Encoding"), not(equalToIgnoringCase("gzip")));

request = HttpTester.newRequest();
request.setMethod("DELETE");
Expand All @@ -633,7 +672,7 @@ public void testDeleteETagGzipHandler() throws Exception
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));

assertThat(response.getStatus(), is(HttpServletResponse.SC_NO_CONTENT));
assertThat(response.get("Content-Encoding"), not(Matchers.equalToIgnoringCase("gzip")));
assertThat(response.get("Content-Encoding"), not(equalToIgnoringCase("gzip")));
}

@Test
Expand All @@ -652,15 +691,15 @@ public void testForwardGzipHandler() throws Exception
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));

assertThat(response.getStatus(), is(200));
assertThat(response.get("Content-Encoding"), Matchers.equalToIgnoringCase("gzip"));
assertThat(response.get("Content-Encoding"), equalToIgnoringCase("gzip"));
assertThat(response.get("ETag"), is(__contentETagGzip));
assertThat(response.get("Vary"), is("Accept-Encoding"));

InputStream testIn = new GZIPInputStream(new ByteArrayInputStream(response.getContentBytes()));
ByteArrayOutputStream testOut = new ByteArrayOutputStream();
IO.copy(testIn, testOut);

assertEquals(__content, testOut.toString("UTF8"));
assertEquals(__content, testOut.toString(StandardCharsets.UTF_8));
}

@Test
Expand All @@ -679,15 +718,15 @@ public void testIncludeGzipHandler() throws Exception
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));

assertThat(response.getStatus(), is(200));
assertThat(response.get("Content-Encoding"), Matchers.equalToIgnoringCase("gzip"));
assertThat(response.get("Content-Encoding"), equalToIgnoringCase("gzip"));
assertThat(response.get("ETag"), nullValue());
assertThat(response.get("Vary"), is("Accept-Encoding"));

InputStream testIn = new GZIPInputStream(new ByteArrayInputStream(response.getContentBytes()));
ByteArrayOutputStream testOut = new ByteArrayOutputStream();
IO.copy(testIn, testOut);

assertEquals(__icontent, testOut.toString("UTF8"));
assertEquals(__icontent, testOut.toString(StandardCharsets.UTF_8));
}

@Test
Expand Down

0 comments on commit 4466657

Please sign in to comment.