Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

More optional etag gzip fixes for #5979 #5986

Merged
merged 2 commits into from Feb 18, 2021
Merged

More optional etag gzip fixes for #5979 #5986

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7d28b42
More optional etag gzip fixes for #5979
gregw Feb 18, 2021
9543255
More optional etag gzip fixes for #5979
gregw Feb 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
102 changes: 89 additions & 13 deletions jetty-http/src/main/java/org/eclipse/jetty/http/CompressedContentFormat.java
View file
Open in desktop
Expand Up @@ -20,6 +20,7 @@

import java.util.Objects;

import org.eclipse.jetty.util.QuotedStringTokenizer;
import org.eclipse.jetty.util.StringUtil;

public class CompressedContentFormat
Expand All @@ -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);
}

Expand All @@ -61,20 +62,95 @@ public boolean equals(Object o)
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;
}

public String etag(String etag)
gregw marked this conversation as resolved.
Show resolved Hide resolved
{
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)
public static boolean tagEquals(String etag, String etagWithSuffix)
gregw marked this conversation as resolved.
Show resolved Hide resolved
{
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;
}
}
8 changes: 4 additions & 4 deletions jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java
View file
Open in desktop
Expand Up @@ -71,7 +71,7 @@ public HttpField getETag()
@Override
public String getETagValue()
{
return _content.getResource().getWeakETag(_format._etag);
return _content.getResource().getWeakETag(_format.getEtagSuffix());
}

@Override
Expand Down Expand Up @@ -101,13 +101,13 @@ public String getContentTypeValue()
@Override
public HttpField getContentEncoding()
{
return _format._contentEncoding;
return _format.getContentEncoding();
}

@Override
public String getContentEncodingValue()
{
return _format._contentEncoding.getValue();
return _format.getContentEncoding().getValue();
}

@Override
Expand Down Expand Up @@ -167,7 +167,7 @@ public ReadableByteChannel getReadableByteChannel() throws IOException
@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("PrecompressedHttpContent@%x{e=%s,r=%s|%s,lm=%s|%s,ct=%s}", hashCode(), _format,
gregw marked this conversation as resolved.
Show resolved Hide resolved
_content.getResource(), _precompressedContent.getResource(),
_content.getResource().lastModified(), _precompressedContent.getResource().lastModified(),
getContentType());
Expand Down
22 changes: 20 additions & 2 deletions jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java
View file
Open in desktop
Expand Up @@ -35,6 +35,8 @@
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;
Expand Down Expand Up @@ -80,9 +82,25 @@ public void testCompressedContentFormat()
{
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
Expand Down
6 changes: 3 additions & 3 deletions jetty-server/src/main/java/org/eclipse/jetty/server/CachedContentFactory.java
View file
Open in desktop
Expand Up @@ -235,7 +235,7 @@ private HttpContent load(String pathInContext, Resource resource, int maxBufferS
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())
{
Expand Down Expand Up @@ -280,7 +280,7 @@ private HttpContent load(String pathInContext, Resource resource, int maxBufferS
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);
Expand Down Expand Up @@ -693,7 +693,7 @@ public class CachedPrecompressedHttpContent extends PrecompressedHttpContent
_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()
Expand Down
2 changes: 1 addition & 1 deletion jetty-server/src/main/java/org/eclipse/jetty/server/ResourceContentFactory.java
View file
Open in desktop
Expand Up @@ -84,7 +84,7 @@ private HttpContent load(String pathInContext, Resource resource, int maxBufferS
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())
Expand Down
10 changes: 5 additions & 5 deletions jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java
View file
Open in desktop
Expand Up @@ -141,7 +141,7 @@ public CompressedContentFormat[] getPrecompressedFormats()
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)
Expand Down Expand Up @@ -282,7 +282,7 @@ public boolean doGet(HttpServletRequest request, HttpServletResponse response)
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());
}
}

Expand Down Expand Up @@ -355,7 +355,7 @@ private CompressedContentFormat getBestPrecompressedContent(List<String> preferr
{
for (CompressedContentFormat format : availableFormats)
{
if (format._encoding.equals(encoding))
if (format.getEncoding().equals(encoding))
return format;
}

Expand Down Expand Up @@ -531,9 +531,9 @@ protected boolean passConditionalHeaders(HttpServletRequest request, HttpServlet
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;
Expand Down