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

More optional etag gzip fixes for #5979 (#5986) 

* More optional etag gzip fixes for #5979

IF no separator defined, do not add a suffix to an etag.
Some cleanup of the implementation.

* More optional etag gzip fixes for #5979

updates from review
This commit is contained in:
Greg Wilkins 2021-02-18 17:24:16 +01:00 committed by GitHub
parent a5c8fee872
commit 324ab668de
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 160 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

111
jetty-http/src/main/java/org/eclipse/jetty/http/CompressedContentFormat.java
View File

@ -20,6 +20,7 @@ package org.eclipse.jetty.http;
import java.util.Objects;
import org.eclipse.jetty.util.QuotedStringTokenizer;
import org.eclipse.jetty.util.StringUtil;
public class CompressedContentFormat
@ -37,18 +38,18 @@ public class CompressedContentFormat
public static final CompressedContentFormat BR = new CompressedContentFormat("br", ".br");
public static final CompressedContentFormat[] NONE = new CompressedContentFormat[0];
public final String _encoding;
public final String _extension;
public final String _etag;
public final String _etagQuote;
public final PreEncodedHttpField _contentEncoding;
private final String _encoding;
private final String _extension;
private final String _etagSuffix;
private final String _etagSuffixQuote;
private final PreEncodedHttpField _contentEncoding;
public CompressedContentFormat(String encoding, String extension)
{
_encoding = StringUtil.asciiToLowerCase(encoding);
_extension = StringUtil.asciiToLowerCase(extension);
_etag = ETAG_SEPARATOR + _encoding;
_etagQuote = _etag + "\"";
_etagSuffix = StringUtil.isEmpty(ETAG_SEPARATOR) ? "" : (ETAG_SEPARATOR + _encoding);
_etagSuffixQuote = _etagSuffix + "\"";
_contentEncoding = new PreEncodedHttpField(HttpHeader.CONTENT_ENCODING, _encoding);
}
@ -61,20 +62,104 @@ public class CompressedContentFormat
return Objects.equals(_encoding, ccf._encoding) && Objects.equals(_extension, ccf._extension);
}
public String getEncoding()
{
return _encoding;
}
public String getExtension()
{
return _extension;
}
public String getEtagSuffix()
{
return _etagSuffix;
}
public HttpField getContentEncoding()
{
return _contentEncoding;
}
/** Get an etag with suffix that represents this compressed type.
* @param etag An etag
* @return An etag with compression suffix, or the etag itself if no suffix is configured.
*/
public String etag(String etag)
{
if (StringUtil.isEmpty(ETAG_SEPARATOR))
return etag;
int end = etag.length() - 1;
if (etag.charAt(end) == '"')
return etag.substring(0, end) + _etagSuffixQuote;
return etag + _etagSuffix;
}
@Override
public int hashCode()
{
return Objects.hash(_encoding, _extension);
}
public static boolean tagEquals(String etag, String tag)
/** Check etags for equality, accounting for quoting and compression suffixes.
* @param etag An etag without a compression suffix
* @param etagWithSuffix An etag optionally with a compression suffix.
* @return True if the tags are equal.
*/
public static boolean tagEquals(String etag, String etagWithSuffix)
{
if (etag.equals(tag))
// Handle simple equality
if (etag.equals(etagWithSuffix))
return true;
int separator = tag.lastIndexOf(ETAG_SEPARATOR);
if (separator > 0 && separator == etag.length() - 1)
return etag.regionMatches(0, tag, 0, separator);
return false;
// If no separator defined, then simple equality is only possible positive
if (StringUtil.isEmpty(ETAG_SEPARATOR))
return false;
// Are both tags quoted?
boolean etagQuoted = etag.endsWith("\"");
boolean etagSuffixQuoted = etagWithSuffix.endsWith("\"");
// Look for a separator
int separator = etagWithSuffix.lastIndexOf(ETAG_SEPARATOR);
// If both tags are quoted the same (the norm) then any difference must be the suffix
if (etagQuoted == etagSuffixQuoted)
return separator > 0 && etag.regionMatches(0, etagWithSuffix, 0, separator);
// If either tag is weak then we can't match because weak tags must be quoted
if (etagWithSuffix.startsWith("W/") || etag.startsWith("W/"))
return false;
// compare unquoted strong etags
etag = etagQuoted ? QuotedStringTokenizer.unquote(etag) : etag;
etagWithSuffix = etagSuffixQuoted ? QuotedStringTokenizer.unquote(etagWithSuffix) : etagWithSuffix;
separator = etagWithSuffix.lastIndexOf(ETAG_SEPARATOR);
if (separator > 0)
return etag.regionMatches(0, etagWithSuffix, 0, separator);
return Objects.equals(etag, etagWithSuffix);
}
public String stripSuffixes(String etagsList)
{
if (StringUtil.isEmpty(ETAG_SEPARATOR))
return etagsList;
// This is a poor implementation that ignores list and tag structure
while (true)
{
int i = etagsList.lastIndexOf(_etagSuffix);
if (i < 0)
return etagsList;
etagsList = etagsList.substring(0, i) + etagsList.substring(i + _etagSuffix.length());
}
}
@Override
public String toString()
{
return _encoding;
}
}

10
jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java
View File

@ -71,7 +71,7 @@ public class PrecompressedHttpContent implements HttpContent
@Override
public String getETagValue()
{
return _content.getResource().getWeakETag(_format._etag);
return _content.getResource().getWeakETag(_format.getEtagSuffix());
}
@Override
@ -101,13 +101,13 @@ public class PrecompressedHttpContent implements HttpContent
@Override
public HttpField getContentEncoding()
{
return _format._contentEncoding;
return _format.getContentEncoding();
}
@Override
public String getContentEncodingValue()
{
return _format._contentEncoding.getValue();
return _format.getContentEncoding().getValue();
}
@Override
@ -167,7 +167,9 @@ public class PrecompressedHttpContent implements HttpContent
@Override
public String toString()
{
return String.format("PrecompressedHttpContent@%x{e=%s,r=%s|%s,lm=%s|%s,ct=%s}", hashCode(), _format._encoding,
return String.format("%s@%x{e=%s,r=%s|%s,lm=%s|%s,ct=%s}",
this.getClass().getSimpleName(), hashCode(),
_format,
_content.getResource(), _precompressedContent.getResource(),
_content.getResource().lastModified(), _precompressedContent.getResource().lastModified(),
getContentType());

22
jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java
View File

@ -35,6 +35,8 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import static org.eclipse.jetty.http.CompressedContentFormat.BR;
import static org.eclipse.jetty.http.CompressedContentFormat.GZIP;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertEquals;
@ -80,9 +82,25 @@ public class GZIPContentDecoderTest
{
assertTrue(CompressedContentFormat.tagEquals("tag", "tag"));
assertTrue(CompressedContentFormat.tagEquals("\"tag\"", "\"tag\""));
assertTrue(CompressedContentFormat.tagEquals("\"tag\"", "\"tag--gzip\""));
assertFalse(CompressedContentFormat.tagEquals("Zag", "Xag--gzip"));
assertTrue(CompressedContentFormat.tagEquals("\"tag\"", "\"tag" + GZIP.getEtagSuffix() + "\""));
assertTrue(CompressedContentFormat.tagEquals("\"tag\"", "\"tag" + BR.getEtagSuffix() + "\""));
assertTrue(CompressedContentFormat.tagEquals("W/\"1234567\"", "W/\"1234567\""));
assertTrue(CompressedContentFormat.tagEquals("W/\"1234567\"", "W/\"1234567" + GZIP.getEtagSuffix() + "\""));
assertFalse(CompressedContentFormat.tagEquals("Zag", "Xag" + GZIP.getEtagSuffix()));
assertFalse(CompressedContentFormat.tagEquals("xtag", "tag"));
assertFalse(CompressedContentFormat.tagEquals("W/\"1234567\"", "W/\"1234111\""));
assertFalse(CompressedContentFormat.tagEquals("W/\"1234567\"", "W/\"1234111" + GZIP.getEtagSuffix() + "\""));
assertTrue(CompressedContentFormat.tagEquals("12345", "\"12345\""));
assertTrue(CompressedContentFormat.tagEquals("\"12345\"", "12345"));
assertTrue(CompressedContentFormat.tagEquals("12345", "\"12345" + GZIP.getEtagSuffix() + "\""));
assertTrue(CompressedContentFormat.tagEquals("\"12345\"", "12345" + GZIP.getEtagSuffix()));
assertThat(GZIP.stripSuffixes("12345"), is("12345"));
assertThat(GZIP.stripSuffixes("12345, 666" + GZIP.getEtagSuffix()), is("12345, 666"));
assertThat(GZIP.stripSuffixes("12345, 666" + GZIP.getEtagSuffix() + ",W/\"9999" + GZIP.getEtagSuffix() + "\""),
is("12345, 666,W/\"9999\""));
}
@Test

6
jetty-server/src/main/java/org/eclipse/jetty/server/CachedContentFactory.java
View File

@ -235,7 +235,7 @@ public class CachedContentFactory implements HttpContent.ContentFactory
Map<CompressedContentFormat, CachedHttpContent> precompresssedContents = new HashMap<>(_precompressedFormats.length);
for (CompressedContentFormat format : _precompressedFormats)
{
String compressedPathInContext = pathInContext + format._extension;
String compressedPathInContext = pathInContext + format.getExtension();
CachedHttpContent compressedContent = _cache.get(compressedPathInContext);
if (compressedContent == null || compressedContent.isValid())
{
@ -280,7 +280,7 @@ public class CachedContentFactory implements HttpContent.ContentFactory
Map<CompressedContentFormat, HttpContent> compressedContents = new HashMap<>();
for (CompressedContentFormat format : _precompressedFormats)
{
String compressedPathInContext = pathInContext + format._extension;
String compressedPathInContext = pathInContext + format.getExtension();
CachedHttpContent compressedContent = _cache.get(compressedPathInContext);
if (compressedContent != null && compressedContent.isValid() && compressedContent.getResource().lastModified() >= resource.lastModified())
compressedContents.put(format, compressedContent);
@ -693,7 +693,7 @@ public class CachedContentFactory implements HttpContent.ContentFactory
_content = content;
_precompressedContent = precompressedContent;
_etag = (CachedContentFactory.this._etags) ? new PreEncodedHttpField(HttpHeader.ETAG, _content.getResource().getWeakETag(format._etag)) : null;
_etag = (CachedContentFactory.this._etags) ? new PreEncodedHttpField(HttpHeader.ETAG, _content.getResource().getWeakETag(format.getEtagSuffix())) : null;
}
public boolean isValid()

2
jetty-server/src/main/java/org/eclipse/jetty/server/ResourceContentFactory.java
View File

@ -84,7 +84,7 @@ public class ResourceContentFactory implements ContentFactory
Map<CompressedContentFormat, HttpContent> compressedContents = new HashMap<>(_precompressedFormats.length);
for (CompressedContentFormat format : _precompressedFormats)
{
String compressedPathInContext = pathInContext + format._extension;
String compressedPathInContext = pathInContext + format.getExtension();
Resource compressedResource = _factory.getResource(compressedPathInContext);
if (compressedResource != null && compressedResource.exists() && compressedResource.lastModified() >= resource.lastModified() &&
compressedResource.length() < resource.length())

10
jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java
View File

@ -141,7 +141,7 @@ public class ResourceService
public void setPrecompressedFormats(CompressedContentFormat[] precompressedFormats)
{
_precompressedFormats = precompressedFormats;
_preferredEncodingOrder = stream(_precompressedFormats).map(f -> f._encoding).toArray(String[]::new);
_preferredEncodingOrder = stream(_precompressedFormats).map(f -> f.getEncoding()).toArray(String[]::new);
}
public void setEncodingCacheSize(int encodingCacheSize)
@ -282,7 +282,7 @@ public class ResourceService
if (LOG.isDebugEnabled())
LOG.debug("precompressed={}", precompressedContent);
content = precompressedContent;
response.setHeader(HttpHeader.CONTENT_ENCODING.asString(), precompressedContentEncoding._encoding);
response.setHeader(HttpHeader.CONTENT_ENCODING.asString(), precompressedContentEncoding.getEncoding());
}
}
@ -355,7 +355,7 @@ public class ResourceService
{
for (CompressedContentFormat format : availableFormats)
{
if (format._encoding.equals(encoding))
if (format.getEncoding().equals(encoding))
return format;
}
@ -531,9 +531,9 @@ public class ResourceService
if (etag != null)
{
QuotedCSV quoted = new QuotedCSV(true, ifm);
for (String tag : quoted)
for (String etagWithSuffix : quoted)
{
if (CompressedContentFormat.tagEquals(etag, tag))
if (CompressedContentFormat.tagEquals(etag, etagWithSuffix))
{
match = true;
break;

2
jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java
View File

@ -307,7 +307,7 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory,
{
for (CompressedContentFormat formats : _resourceService.getPrecompressedFormats())
{
if (CompressedContentFormat.GZIP._encoding.equals(formats._encoding))
if (CompressedContentFormat.GZIP.getEncoding().equals(formats.getEncoding()))
return true;
}
return false;

40
jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java
View File

@ -36,7 +36,6 @@ import org.eclipse.jetty.http.CompressedContentFormat;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpHeaderValue;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.http.PreEncodedHttpField;
@ -153,13 +152,13 @@ import org.eclipse.jetty.util.log.Logger;
*/
public class GzipHandler extends HandlerWrapper implements GzipFactory
{
public static final String GZIP_HANDLER_ETAGS = "o.e.j.s.h.gzip.GzipHandler.etag";
public static final String GZIP = "gzip";
public static final String DEFLATE = "deflate";
public static final int DEFAULT_MIN_GZIP_SIZE = 32;
public static final int BREAK_EVEN_GZIP_SIZE = 23;
private static final Logger LOG = Log.getLogger(GzipHandler.class);
private static final HttpField X_CE_GZIP = new PreEncodedHttpField("X-Content-Encoding", "gzip");
private static final HttpField TE_CHUNKED = new PreEncodedHttpField(HttpHeader.TRANSFER_ENCODING, HttpHeaderValue.CHUNKED.asString());
private static final Pattern COMMA_GZIP = Pattern.compile(".*, *gzip");
private int _poolCapacity = -1;
@ -477,7 +476,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
public String[] getExcludedAgentPatterns()
{
Set<String> excluded = _agentPatterns.getExcluded();
return excluded.toArray(new String[excluded.size()]);
return excluded.toArray(new String[0]);
}
/**
@ -489,7 +488,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
public String[] getExcludedMethods()
{
Set<String> excluded = _methods.getExcluded();
return excluded.toArray(new String[excluded.size()]);
return excluded.toArray(new String[0]);
}
/**
@ -501,7 +500,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
public String[] getExcludedMimeTypes()
{
Set<String> excluded = _mimeTypes.getExcluded();
return excluded.toArray(new String[excluded.size()]);
return excluded.toArray(new String[0]);
}
/**
@ -513,7 +512,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
public String[] getExcludedPaths()
{
Set<String> excluded = _paths.getExcluded();
return excluded.toArray(new String[excluded.size()]);
return excluded.toArray(new String[0]);
}
/**
@ -525,7 +524,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
public String[] getIncludedAgentPatterns()
{
Set<String> includes = _agentPatterns.getIncluded();
return includes.toArray(new String[includes.size()]);
return includes.toArray(new String[0]);
}
/**
@ -537,7 +536,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
public String[] getIncludedMethods()
{
Set<String> includes = _methods.getIncluded();
return includes.toArray(new String[includes.size()]);
return includes.toArray(new String[0]);
}
/**
@ -549,7 +548,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
public String[] getIncludedMimeTypes()
{
Set<String> includes = _mimeTypes.getIncluded();
return includes.toArray(new String[includes.size()]);
return includes.toArray(new String[0]);
}
/**
@ -561,7 +560,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
public String[] getIncludedPaths()
{
Set<String> includes = _paths.getIncluded();
return includes.toArray(new String[includes.size()]);
return includes.toArray(new String[0]);
}
/**
@ -689,23 +688,20 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
}
// Special handling for etags
for (ListIterator<HttpField> fields = baseRequest.getHttpFields().listIterator(); fields.hasNext(); )
if (!StringUtil.isEmpty(CompressedContentFormat.ETAG_SEPARATOR))
{
HttpField field = fields.next();
if (field.getHeader() == HttpHeader.IF_NONE_MATCH || field.getHeader() == HttpHeader.IF_MATCH)
for (ListIterator<HttpField> fields = baseRequest.getHttpFields().listIterator(); fields.hasNext(); )
{
String etag = field.getValue();
int i = etag.indexOf(CompressedContentFormat.GZIP._etagQuote);
if (i > 0)
HttpField field = fields.next();
if (field.getHeader() == HttpHeader.IF_NONE_MATCH || field.getHeader() == HttpHeader.IF_MATCH)
{
baseRequest.setAttribute("o.e.j.s.h.gzip.GzipHandler.etag", etag);
while (i >= 0)
String etags = field.getValue();
String etagsNoSuffix = CompressedContentFormat.GZIP.stripSuffixes(etags);
if (!etagsNoSuffix.equals(etags))
{
etag = etag.substring(0, i) + etag.substring(i + CompressedContentFormat.GZIP._etag.length());
i = etag.indexOf(CompressedContentFormat.GZIP._etagQuote, i);
fields.set(new HttpField(field.getHeader(), etagsNoSuffix));
baseRequest.setAttribute(GZIP_HANDLER_ETAGS, etags);
}
fields.set(new HttpField(field.getHeader(), etag));
}
}
}

10
jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java
View File

@ -27,6 +27,7 @@ import java.util.zip.Deflater;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.http.PreEncodedHttpField;
import org.eclipse.jetty.server.HttpChannel;
@ -150,9 +151,9 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
LOG.debug("{} exclude by status {}", this, sc);
noCompression();
if (sc == 304)
if (sc == HttpStatus.NOT_MODIFIED_304)
{
String requestEtags = (String)_channel.getRequest().getAttribute("o.e.j.s.h.gzip.GzipHandler.etag");
String requestEtags = (String)_channel.getRequest().getAttribute(GzipHandler.GZIP_HANDLER_ETAGS);
String responseEtag = response.getHttpFields().get(HttpHeader.ETAG);
if (requestEtags != null && responseEtag != null)
{
@ -217,7 +218,7 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
return;
}
fields.put(GZIP._contentEncoding);
fields.put(GZIP.getContentEncoding());
_crc.reset();
// Adjust headers
@ -245,8 +246,7 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
private String etagGzip(String etag)
{
int end = etag.length() - 1;
return (etag.charAt(end) == '"') ? etag.substring(0, end) + GZIP._etag + '"' : etag + GZIP._etag;
return GZIP.etag(etag);
}
public void noCompression()

4
jetty-servlet/src/test/java/org/eclipse/jetty/servlet/GzipHandlerTest.java
View File

@ -88,7 +88,7 @@ public class GzipHandlerTest
private static final String __micro = __content.substring(0, 10);
private static final String __contentETag = String.format("W/\"%x\"", __content.hashCode());
private static final String __contentETagGzip = String.format("W/\"%x" + CompressedContentFormat.GZIP._etag + "\"", __content.hashCode());
private static final String __contentETagGzip = String.format("W/\"%x" + CompressedContentFormat.GZIP.getEtagSuffix() + "\"", __content.hashCode());
private static final String __icontent = "BEFORE" + __content + "AFTER";
private Server _server;
@ -592,7 +592,7 @@ public class GzipHandlerTest
request.setURI("/ctx/content");
request.setVersion("HTTP/1.0");
request.setHeader("Host", "tester");
request.setHeader("If-Match", "WrongEtag" + CompressedContentFormat.GZIP._etag);
request.setHeader("If-Match", "WrongEtag" + CompressedContentFormat.GZIP.getEtagSuffix());
request.setHeader("accept-encoding", "gzip");
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));

2
jetty-servlets/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipDefaultTest.java
View File

@ -145,7 +145,7 @@ public class GzipDefaultTest
//A HEAD request should have similar headers, but no body
response = tester.executeRequest("HEAD", "/context/file.txt", 5, TimeUnit.SECONDS);
assertThat("Response status", response.getStatus(), is(HttpStatus.OK_200));
assertThat("ETag", response.get("ETag"), containsString(CompressedContentFormat.GZIP._etag));
assertThat("ETag", response.get("ETag"), containsString(CompressedContentFormat.GZIP.getEtagSuffix()));
assertThat("Content encoding", response.get("Content-Encoding"), containsString("gzip"));
assertNull(response.get("Content-Length"), "Content length");