Skip to content

Commit

Permalink
Jetty 12 : precompressed content support for ResourceService (#8595)
Browse files Browse the repository at this point in the history 
* Remove Resource.getWeakETag in favor of new EtagUtils
* Protect bad usages of CachingContentFactory
* Working precompressed content for ResourceService
* Fixing DefaultServlet handling of ResourceService.writeHttpError() overrides
* Protect NPE in CachingContentFactory.isValid()
* Complete callback in DefaultServlet.writeHttpError
* Addressing review comments
* Testing for whole content
* Improve MemoryResource handling of name/filename
* EtagUtils handling of Resource
* Better protection of bad Resource impls in EtagUtils
* Update CachingContentFactory
+ Using Resource.lastModified were possible
+ CachingHttpContent has better Resource
  protection, and uses Etag from delegate
+ CachingHttpContent uses delegate where
  possible.
+ Store Etag HttpField in CachingHttpContent
  and allow override in constructor
* Simpler CachingHttpContent.isValid()
* Better handling of PrecompressedHttpContent

Signed-off-by: Joakim Erdfelt <[email protected]>
  • Loading branch information
@joakime
joakime authored Sep 21, 2022
1 parent 594e7cf commit 4fe414a
Show file tree
Hide file tree
Showing 18 changed files with 853 additions and 360 deletions.
103 changes: 57 additions & 46 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import java.nio.ByteBuffer;
import java.nio.channels.SeekableByteChannel;
import java.nio.file.Files;
import java.nio.file.attribute.FileTime;
import java.time.Instant;
import java.util.HashMap;
import java.util.Map;
import java.util.SortedSet;
Expand All @@ -28,9 +28,7 @@

import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.NanoTime;
import org.eclipse.jetty.util.QuotedStringTokenizer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.eclipse.jetty.util.StringUtil;

/**
* HttpContent.ContentFactory implementation that wraps any other HttpContent.ContentFactory instance
Expand All @@ -45,8 +43,6 @@
*/
public class CachingContentFactory implements HttpContent.ContentFactory
{
private static final Logger LOG = LoggerFactory.getLogger(CachingContentFactory.class);

private final HttpContent.ContentFactory _authority;
private final boolean _useFileMappedBuffer;
private final ConcurrentMap<String, CachingHttpContent> _cache = new ConcurrentHashMap<>();
Expand Down Expand Up @@ -171,6 +167,7 @@ public void flushCache()
public HttpContent getContent(String path) throws IOException
{
// TODO load precompressed otherwise it is never served from cache
// TODO: Consider _cache.computeIfAbsent()?
CachingHttpContent cachingHttpContent = _cache.get(path);
if (cachingHttpContent != null)
{
Expand All @@ -184,7 +181,7 @@ public HttpContent getContent(String path) throws IOException
// Do not cache directories or files that are too big
if (httpContent != null && !httpContent.getResource().isDirectory() && httpContent.getContentLengthValue() <= _maxCachedFileSize)
{
httpContent = cachingHttpContent = new CachingHttpContent(path, null, httpContent);
httpContent = cachingHttpContent = new CachingHttpContent(path, httpContent);
_cache.put(path, cachingHttpContent);
_cachedSize.addAndGet(cachingHttpContent.calculateSize());
shrinkCache();
Expand All @@ -195,28 +192,54 @@ public HttpContent getContent(String path) throws IOException
private class CachingHttpContent extends HttpContentWrapper
{
private final ByteBuffer _buffer;
private final FileTime _lastModifiedValue;
private final Instant _lastModifiedValue;
private final String _cacheKey;
private final String _etag;
private final HttpField _etagField;
private final long _contentLengthValue;
private final Map<CompressedContentFormat, CachingHttpContent> _precompressedContents;
private volatile long _lastAccessed;

private CachingHttpContent(String key, String precalculatedEtag, HttpContent httpContent) throws IOException
private CachingHttpContent(String key, HttpContent httpContent) throws IOException
{
this(key, httpContent, httpContent.getETagValue());
}

private CachingHttpContent(String key, HttpContent httpContent, String etagValue) throws IOException
{
super(httpContent);
_etag = precalculatedEtag;
_contentLengthValue = httpContent.getContentLengthValue(); // TODO getContentLengthValue() could return -1

if (_delegate.getResource() == null)
throw new IllegalArgumentException("Null Resource");
if (!_delegate.getResource().exists())
throw new IllegalArgumentException("Resource doesn't exist: " + _delegate.getResource());
if (_delegate.getResource().isDirectory())
throw new IllegalArgumentException("Directory Resources not supported: " + _delegate.getResource());
if (_delegate.getResource().getPath() == null) // only required because we need the Path to access the mapped ByteBuffer or SeekableByteChannel.
throw new IllegalArgumentException("Resource not backed by Path not supported: " + _delegate.getResource());

// Resources with negative length cannot be cached.
// But allow resources with zero length.
long resourceSize = _delegate.getResource().length();
if (resourceSize < 0)
throw new IllegalArgumentException("Resource with negative size: " + _delegate.getResource());

HttpField etagField = _delegate.getETag();
if (StringUtil.isNotBlank(etagValue))
{
etagField = new PreEncodedHttpField(HttpHeader.ETAG, etagValue);
}
_etagField = etagField;
_contentLengthValue = resourceSize;

// map the content into memory if possible
ByteBuffer byteBuffer = _useFileMappedBuffer ? BufferUtil.toMappedBuffer(httpContent.getResource(), 0, _contentLengthValue) : null;
ByteBuffer byteBuffer = _useFileMappedBuffer ? BufferUtil.toMappedBuffer(_delegate.getResource(), 0, _contentLengthValue) : null;

if (byteBuffer == null)
{
// TODO use pool & check length limit
// TODO use pool?
// load the content into memory
byteBuffer = ByteBuffer.allocateDirect((int)_contentLengthValue);
try (SeekableByteChannel channel = Files.newByteChannel(httpContent.getResource().getPath()))
try (SeekableByteChannel channel = Files.newByteChannel(_delegate.getResource().getPath()))
{
// fill buffer
int read = 0;
Expand All @@ -227,26 +250,16 @@ private CachingHttpContent(String key, String precalculatedEtag, HttpContent htt
}

// Load precompressed contents into memory.
Map<CompressedContentFormat, ? extends HttpContent> precompressedContents = httpContent.getPrecompressedContents();
Map<CompressedContentFormat, ? extends HttpContent> precompressedContents = _delegate.getPrecompressedContents();
if (precompressedContents != null)
{
_precompressedContents = new HashMap<>();
for (Map.Entry<CompressedContentFormat, ? extends HttpContent> entry : precompressedContents.entrySet())
{
CompressedContentFormat format = entry.getKey();

// Rewrite the etag to be the content's one with the required suffix all within quotes.
String precompressedEtag = httpContent.getETagValue();
boolean weak = false;
if (precompressedEtag.startsWith("W/"))
{
weak = true;
precompressedEtag = precompressedEtag.substring(2);
}
precompressedEtag = (weak ? "W/\"" : "\"") + QuotedStringTokenizer.unquote(precompressedEtag) + format.getEtagSuffix() + '"';

String precompressedEtag = EtagUtils.rewriteWithSuffix(_delegate.getETagValue(), format.getEtagSuffix());
// The etag of the precompressed content must be the one of the non-compressed content, with the etag suffix appended.
_precompressedContents.put(format, new CachingHttpContent(key, precompressedEtag, entry.getValue()));
_precompressedContents.put(format, new CachingHttpContent(key, entry.getValue(), precompressedEtag));
}
}
else
Expand All @@ -256,7 +269,7 @@ private CachingHttpContent(String key, String precalculatedEtag, HttpContent htt

_cacheKey = key;
_buffer = byteBuffer;
_lastModifiedValue = Files.getLastModifiedTime(httpContent.getResource().getPath());
_lastModifiedValue = _delegate.getResource().lastModified();
_lastAccessed = NanoTime.now();
}

Expand All @@ -273,6 +286,12 @@ long calculateSize()
return totalSize;
}

@Override
public long getContentLengthValue()
{
return _contentLengthValue;
}

@Override
public ByteBuffer getBuffer()
{
Expand All @@ -285,18 +304,11 @@ public ByteBuffer getBuffer()

public boolean isValid()
{
try
Instant lastModifiedTime = _delegate.getResource().lastModified();
if (lastModifiedTime.equals(_lastModifiedValue))
{
FileTime lastModifiedTime = Files.getLastModifiedTime(_delegate.getResource().getPath());
if (lastModifiedTime.equals(_lastModifiedValue))
{
_lastAccessed = NanoTime.now();
return true;
}
}
catch (IOException e)
{
LOG.debug("unable to get delegate path' LastModifiedTime", e);
_lastAccessed = NanoTime.now();
return true;
}
release();
return false;
Expand All @@ -311,17 +323,16 @@ public void release()
@Override
public HttpField getETag()
{
String eTag = getETagValue();
return eTag == null ? null : new HttpField(HttpHeader.ETAG, eTag);
return _etagField;
}

@Override
public String getETagValue()
{
if (_etag != null)
return _etag;
else
return _delegate.getETagValue();
HttpField etag = getETag();
if (etag == null)
return null;
return etag.getValue();
}

@Override
Expand Down
59 changes: 4 additions & 55 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CompressedContentFormat.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,10 @@

import java.util.Objects;

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

public class CompressedContentFormat
{
/**
* The separator within an etag used to indicate a compressed variant. By default the separator is "--"
* So etag for compressed resource that normally has an etag of <code>W/"28c772d6"</code>
* is <code>W/"28c772d6--gzip"</code>. The separator may be changed by the
* "org.eclipse.jetty.http.CompressedContentFormat.ETAG_SEPARATOR" System property. If changed, it should be changed to a string
* that will not be found in a normal etag or at least is very unlikely to be a substring of a normal etag.
*/
public static final String ETAG_SEPARATOR = System.getProperty(CompressedContentFormat.class.getName() + ".ETAG_SEPARATOR", "--");

public static final CompressedContentFormat GZIP = new CompressedContentFormat("gzip", ".gz");
public static final CompressedContentFormat BR = new CompressedContentFormat("br", ".br");
public static final CompressedContentFormat[] NONE = new CompressedContentFormat[0];
Expand All @@ -43,17 +33,16 @@ public CompressedContentFormat(String encoding, String extension)
{
_encoding = StringUtil.asciiToLowerCase(encoding);
_extension = StringUtil.asciiToLowerCase(extension);
_etagSuffix = StringUtil.isEmpty(ETAG_SEPARATOR) ? "" : (ETAG_SEPARATOR + _encoding);
_etagSuffix = StringUtil.isEmpty(EtagUtils.ETAG_SEPARATOR) ? "" : (EtagUtils.ETAG_SEPARATOR + _encoding);
_etagSuffixQuote = _etagSuffix + "\"";
_contentEncoding = new PreEncodedHttpField(HttpHeader.CONTENT_ENCODING, _encoding);
}

@Override
public boolean equals(Object o)
{
if (!(o instanceof CompressedContentFormat))
if (!(o instanceof CompressedContentFormat ccf))
return false;
CompressedContentFormat ccf = (CompressedContentFormat)o;
return Objects.equals(_encoding, ccf._encoding) && Objects.equals(_extension, ccf._extension);
}

Expand Down Expand Up @@ -83,7 +72,7 @@ public HttpField getContentEncoding()
*/
public String etag(String etag)
{
if (StringUtil.isEmpty(ETAG_SEPARATOR))
if (StringUtil.isEmpty(EtagUtils.ETAG_SEPARATOR))
return etag;
int end = etag.length() - 1;
if (etag.charAt(end) == '"')
Expand All @@ -97,49 +86,9 @@ public int hashCode()
return Objects.hash(_encoding, _extension);
}

/** 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)
{
// Handle simple equality
if (etag.equals(etagWithSuffix))
return true;

// 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))
if (StringUtil.isEmpty(EtagUtils.ETAG_SEPARATOR))
return etagsList;

// This is a poor implementation that ignores list and tag structure
Expand Down
Loading

0 comments on commit 4fe414a

Please sign in to comment.