Avoid updating Content-Length header in a 304 response.
I observed the following problem: `Transfer-Encoding` and `Content-Length` headers should be mutually exclusive and because I use chunked transfer, the `Transfer-Encoding` header is set in the response while the `Content-Length` header is not. In case of a 304 during a revalidation, the header contains Content-Length=0. Probably a proxy is responsible for this, just like the comment "Some well-known proxies respond with Content-Length=0, when returning 304" in the method CachedHttpResponseGenerator::addMissingContentLengthHeader is saying. In CacheEntryUpdater::mergeHeaders the Content-Length=0 is merged into the cached entry, but the cached entry contains also a `Transfer-Encoding` header, so in the cached entry these headers aren't mutually exclusive anymore. Because of the `Transfer-Encoding` header the method CachedHttpResponseGenerator::addMissingContentLengthHeader isn't fixing the `Content-Length` header and Content-Length=0 causes returning null instead of the cached content. IMHO the `Content-Length` header should not be merged into the cached response in case of a 304, at least if the cached entry contains a `Transfer-Encoding` header.
This commit is contained in:
parent
f024a5884e
commit
8151d9e51a
|
@ -111,8 +111,9 @@ class CacheEntryUpdater {
|
|||
// Remove cache headers that match response
|
||||
for (final HeaderIterator it = response.headerIterator(); it.hasNext(); ) {
|
||||
final Header responseHeader = it.nextHeader();
|
||||
// Since we do not expect a content in a 304 response, should retain the original Content-Encoding header
|
||||
if (HTTP.CONTENT_ENCODING.equals(responseHeader.getName())) {
|
||||
// Since we do not expect a content in a 304 response, should retain the original Content-Encoding and Content-Length header
|
||||
if (HTTP.CONTENT_ENCODING.equals(responseHeader.getName())
|
||||
|| HTTP.CONTENT_LEN.equals(responseHeader.getName())) {
|
||||
continue;
|
||||
}
|
||||
final Header[] matchingHeaders = headerGroup.getHeaders(responseHeader.getName());
|
||||
|
@ -133,8 +134,9 @@ class CacheEntryUpdater {
|
|||
}
|
||||
for (final HeaderIterator it = response.headerIterator(); it.hasNext(); ) {
|
||||
final Header responseHeader = it.nextHeader();
|
||||
// Since we do not expect a content in a 304 response, should avoid updating Content-Encoding header
|
||||
if (HTTP.CONTENT_ENCODING.equals(responseHeader.getName())) {
|
||||
// Since we do not expect a content in a 304 response, should avoid updating Content-Encoding and Content-Length header
|
||||
if (HTTP.CONTENT_ENCODING.equals(responseHeader.getName())
|
||||
|| HTTP.CONTENT_LEN.equals(responseHeader.getName())) {
|
||||
continue;
|
||||
}
|
||||
headerGroup.addHeader(responseHeader);
|
||||
|
|
|
@ -260,6 +260,27 @@ public class TestCacheEntryUpdater {
|
|||
headersNotContain(updatedHeaders, "Content-Encoding", "gzip");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testContentLengthIsNotAddedWhenTransferEncodingIsPresent() throws IOException {
|
||||
final Header[] headers = {
|
||||
new BasicHeader("Date", DateUtils.formatDate(requestDate)),
|
||||
new BasicHeader("ETag", "\"etag\""),
|
||||
new BasicHeader("Transfer-Encoding", "chunked")};
|
||||
|
||||
entry = HttpTestUtils.makeCacheEntry(headers);
|
||||
response.setHeaders(new Header[]{
|
||||
new BasicHeader("Last-Modified", DateUtils.formatDate(responseDate)),
|
||||
new BasicHeader("Cache-Control", "public"),
|
||||
new BasicHeader("Content-Length", "0")});
|
||||
|
||||
final HttpCacheEntry updatedEntry = impl.updateCacheEntry(null, entry,
|
||||
new Date(), new Date(), response);
|
||||
|
||||
final Header[] updatedHeaders = updatedEntry.getAllHeaders();
|
||||
headersContain(updatedHeaders, "Transfer-Encoding", "chunked");
|
||||
headersNotContain(updatedHeaders, "Content-Length", "0");
|
||||
}
|
||||
|
||||
private void headersContain(final Header[] headers, final String name, final String value) {
|
||||
for (final Header header : headers) {
|
||||
if (header.getName().equals(name)) {
|
||||
|
|
Loading…
Reference in New Issue