From 4fe414a762d059ae7869e90f9b7a7f16f2f1fae4 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 21 Sep 2022 09:39:21 -0500 Subject: [PATCH] Jetty 12 : precompressed content support for `ResourceService` (#8595) * 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 --- .../jetty/http/CachingContentFactory.java | 103 +++--- .../jetty/http/CompressedContentFormat.java | 59 +--- .../org/eclipse/jetty/http/EtagUtils.java | 332 ++++++++++++++++++ .../jetty/http/PrecompressedHttpContent.java | 24 +- .../jetty/http/ResourceHttpContent.java | 11 +- .../org/eclipse/jetty/http/EtagUtilsTest.java | 146 ++++++++ .../jetty/http/GZIPContentDecoderTest.java | 22 +- .../jetty/server/CachedContentFactory.java | 3 +- .../jetty/server/ResourceContentFactory.java | 13 +- .../eclipse/jetty/server/ResourceService.java | 127 ++++--- .../server/handler/ResourceHandlerTest.java | 225 ++++++------ .../java/org/eclipse/jetty/util/FileID.java | 19 + .../jetty/util/resource/MemoryResource.java | 5 +- .../eclipse/jetty/util/resource/Resource.java | 43 --- .../org/eclipse/jetty/util/FileIDTest.java | 23 ++ .../jetty/ee10/servlet/DefaultServlet.java | 44 +++ .../ee9/nested/CachedContentFactory.java | 7 +- .../jetty/ee9/nested/ResourceService.java | 7 +- 18 files changed, 853 insertions(+), 360 deletions(-) create mode 100644 jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/EtagUtils.java create mode 100644 jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/EtagUtilsTest.java diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java index 6acb898cfb88..b9bf99c1eb7a 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java @@ -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; @@ -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 @@ -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 _cache = new ConcurrentHashMap<>(); @@ -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) { @@ -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(); @@ -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 _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; @@ -227,26 +250,16 @@ private CachingHttpContent(String key, String precalculatedEtag, HttpContent htt } // Load precompressed contents into memory. - Map precompressedContents = httpContent.getPrecompressedContents(); + Map precompressedContents = _delegate.getPrecompressedContents(); if (precompressedContents != null) { _precompressedContents = new HashMap<>(); for (Map.Entry 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 @@ -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(); } @@ -273,6 +286,12 @@ long calculateSize() return totalSize; } + @Override + public long getContentLengthValue() + { + return _contentLengthValue; + } + @Override public ByteBuffer getBuffer() { @@ -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; @@ -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 diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CompressedContentFormat.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CompressedContentFormat.java index b8c59533bc8f..c009e6f34835 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CompressedContentFormat.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CompressedContentFormat.java @@ -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 W/"28c772d6" - * is W/"28c772d6--gzip". 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]; @@ -43,7 +33,7 @@ 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); } @@ -51,9 +41,8 @@ public CompressedContentFormat(String encoding, String extension) @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); } @@ -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) == '"') @@ -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 diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/EtagUtils.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/EtagUtils.java new file mode 100644 index 000000000000..6dc230c78806 --- /dev/null +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/EtagUtils.java @@ -0,0 +1,332 @@ +// +// ======================================================================== +// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.http; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Instant; +import java.util.Base64; +import java.util.Objects; + +import org.eclipse.jetty.util.QuotedStringTokenizer; +import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.resource.Resource; + +/** + * Utility classes for Entity Tag behaviors as outlined in RFC9110 : Section 8.8.3 - Entity Tag + */ +public final class EtagUtils +{ + private EtagUtils() + { + // prevent instantiation + } + + /** + *

The separator within an etag used to indicate a compressed variant.

+ * + *

Default separator is {@code --}

+ * + *

Adding a suffix of {@code gzip} to an etag of {@code W/"28c772d6"} will result in {@code W/"28c772d6--gzip"}

+ * + *

The separator may be changed by the + * {@code org.eclipse.jetty.http.EtagUtil.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(EtagUtils.class.getName() + ".ETAG_SEPARATOR", "--"); + + /** + * Create a new {@link HttpField} {@link HttpHeader#ETAG} field suitable to represent the provided Resource. + * + * @param resource the resource to represent with an Etag. + * @return the field if possible to create etag, or null if not possible. + */ + public static HttpField createWeakEtagField(Resource resource) + { + return createWeakEtagField(resource, null); + } + + /** + * Create a new {@link HttpField} {@link HttpHeader#ETAG} field suitable to represent the provided Resource. + * + * @param resource the resource to represent with an Etag. + * @param etagSuffix the suffix for the etag + * @return the field if possible to create etag, or null if not possible. + */ + public static HttpField createWeakEtagField(Resource resource, String etagSuffix) + { + Path path = resource.getPath(); + if (path == null) + return null; + + String etagValue = EtagUtils.computeWeakEtag(path, etagSuffix); + if (etagValue != null) + return new PreEncodedHttpField(HttpHeader.ETAG, etagValue); + return null; + } + + /** + * Compute the weak etag for the provided {@link Resource} reference. + * + *

+ * Will use the {@link Resource#getPath()} if the resource provides it, otherwise it will + * use the {@link Resource} provided details from {@link Resource#getName()}, {@link Resource#lastModified()}, + * and {@link Resource#length()} to build the ETag. + *

+ * + * @param resource the resource to calculate etag from + * @return the weak etag + */ + public static String computeWeakEtag(Resource resource) + { + return computeWeakEtag(resource, null); + } + + /** + * Compute the weak etag for the provided {@link Resource} reference. + * + *

+ * Will use the {@link Resource#getPath()} if the resource provides it, otherwise it will + * use the {@link Resource} provided details from {@link Resource#getName()}, {@link Resource#lastModified()}, + * and {@link Resource#length()} to build the ETag. + *

+ * + * @param resource the resource to calculate etag from + * @param etagSuffix the suffix for the etag + * @return the weak etag + */ + public static String computeWeakEtag(Resource resource, String etagSuffix) + { + if (resource == null || !resource.exists() || resource.isDirectory()) + return null; + + Path path = resource.getPath(); + if (path != null) + { + // This is the most reliable technique. + // ResourceCollection can return a different resource for each call name/lastModified/length + // Using Path here ensures that if a Path is available, we can use it to get the name/lastModified/length + // for same referenced Path object (something that ResourceCollection does not guarantee) + return computeWeakEtag(path, etagSuffix); + } + else + { + // Use the URI / lastModified / size in case the Resource does not support Path + // These fields must be returned by the implementation of Resource for the Resource to be valid + String name = resource.getName(); + Instant lastModified = resource.lastModified(); + long size = resource.length(); + return computeWeakEtag(name, lastModified, size, etagSuffix); + } + } + + /** + * Compute the weak etag for the provided {@link Path} reference. + * + *

+ * This supports relative path references as well. + * Which can be useful to establish a reliable etag if the base changes. + *

+ * + * @param path the path to calculate from + * @return the weak etag + */ + public static String computeWeakEtag(Path path) + { + return computeWeakEtag(path, null); + } + + /** + * Compute the weak etag for the provided {@link Path} reference. + * + * @param path the path to calculate from + * @param suffix the optional suffix for the ETag + * @return the weak etag + */ + public static String computeWeakEtag(Path path, String suffix) + { + if (path == null) + return null; + + String name = path.toAbsolutePath().toString(); + Instant lastModified = Instant.EPOCH; + try + { + lastModified = Files.getLastModifiedTime(path).toInstant(); + } + catch (IOException ignored) + { + } + long size = -1; + try + { + size = Files.size(path); + } + catch (IOException ignored) + { + } + return computeWeakEtag(name, lastModified, size, suffix); + } + + /** + * Main algorithm of how Jetty builds a unique Etag. + * + * @param name the name of the resource + * @param lastModified the last modified time of the resource + * @param size the size of the resource + * @param etagSuffix the optional etag suffix + * @return the calculated etag for the resource + */ + private static String computeWeakEtag(String name, Instant lastModified, long size, String etagSuffix) + { + StringBuilder b = new StringBuilder(32); + b.append("W/\""); + + int length = name.length(); + long lhash = 0; + for (int i = 0; i < length; i++) + { + lhash = 31 * lhash + name.charAt(i); + } + + Base64.Encoder encoder = Base64.getEncoder().withoutPadding(); + long lastModifiedMillis = lastModified.toEpochMilli(); + b.append(encoder.encodeToString(longToBytes(lastModifiedMillis ^ lhash))); + b.append(encoder.encodeToString(longToBytes(size ^ lhash))); + if (etagSuffix != null) + b.append(etagSuffix); + b.append('"'); + return b.toString(); + } + + /** + * Rewrite etag with a new suffix, satisfying quoting rules, and preserving optional weak flag. + * + * @param etag the original etag + * @param newSuffix the new suffix to add or change (if a preexisting suffix exists) + * @return the new etag, + */ + public static String rewriteWithSuffix(String etag, String newSuffix) + { + StringBuilder ret = new StringBuilder(); + boolean weak = etag.startsWith("W/"); + int start = 0; + if (etag.startsWith("W/")) + { + weak = true; + start = 2; + } + + // ignore quotes + while (etag.charAt(start) == '"') + { + start++; + } + int end = etag.length(); + while (etag.charAt(end - 1) == '"') + { + end--; + } + // find suffix (if present) + int suffixIdx = etag.lastIndexOf('-', end); + if (suffixIdx >= 0 && suffixIdx >= start) + end = suffixIdx; + + // build new etag + if (weak) + ret.append("W/"); + ret.append('"'); + ret.append(etag, start, end); + ret.append(newSuffix); + ret.append('"'); + return ret.toString(); + } + + /** + * Test if the given Etag is considered weak. + * + * @param etag the etag to test + * @return true if weak. + */ + public static boolean isWeak(String etag) + { + return etag.startsWith("W/"); + } + + /** + * Test if the given Etag is considered strong (not weak). + * + * @param etag the etag to test + * @return true if strong (not weak). + */ + public static boolean isStrong(String etag) + { + return !isWeak(etag); + } + + /** + *

Test if etag matches against an etagWithOptionalSuffix.

+ * + * @param etag An etag without a compression suffix + * @param etagWithOptionalSuffix An etag optionally with a compression suffix. + * @return True if the tags are equal. + * @see RFC9110: Section 8.8.3.2 - Etag Comparison. + */ + public static boolean matches(String etag, String etagWithOptionalSuffix) + { + // Handle simple equality + if (etag.equals(etagWithOptionalSuffix)) + 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 = etagWithOptionalSuffix.endsWith("\""); + + // Look for a separator + int separator = etagWithOptionalSuffix.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, etagWithOptionalSuffix, 0, separator); + + // If either tag is weak then we can't matches because weak tags must be quoted + if (etagWithOptionalSuffix.startsWith("W/") || etag.startsWith("W/")) + return false; + + // compare unquoted strong etags + etag = etagQuoted ? QuotedStringTokenizer.unquote(etag) : etag; + etagWithOptionalSuffix = etagSuffixQuoted ? QuotedStringTokenizer.unquote(etagWithOptionalSuffix) : etagWithOptionalSuffix; + separator = etagWithOptionalSuffix.lastIndexOf(ETAG_SEPARATOR); + if (separator > 0) + return etag.regionMatches(0, etagWithOptionalSuffix, 0, separator); + + return Objects.equals(etag, etagWithOptionalSuffix); + } + + private static byte[] longToBytes(long value) + { + byte[] result = new byte[Long.BYTES]; + for (int i = Long.BYTES - 1; i >= 0; i--) + { + result[i] = (byte)(value & 0xFF); + value >>= 8; + } + return result; + } +} diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java index 1edad1805a76..ce4b19f08004 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java @@ -24,47 +24,51 @@ public class PrecompressedHttpContent implements HttpContent private final HttpContent _content; private final HttpContent _precompressedContent; private final CompressedContentFormat _format; + private final HttpField _etag; public PrecompressedHttpContent(HttpContent content, HttpContent precompressedContent, CompressedContentFormat format) { + if (content == null) + throw new IllegalArgumentException("Null HttpContent"); + if (precompressedContent == null) + throw new IllegalArgumentException("Null Precompressed HttpContent"); + if (format == null) + throw new IllegalArgumentException("Null Compressed Content Format"); + _content = content; _precompressedContent = precompressedContent; _format = format; - if (_precompressedContent == null || _format == null) - { - throw new NullPointerException("Missing compressed content and/or format"); - } + _etag = EtagUtils.createWeakEtagField(_content.getResource(), _format.getEtagSuffix()); } @Override public Resource getResource() { - return _content.getResource(); + return _precompressedContent.getResource(); } @Override public HttpField getETag() { - return new HttpField(HttpHeader.ETAG, getETagValue()); + return _etag; } @Override public String getETagValue() { - //return _content.getResource().getWeakETag(_format.getEtagSuffix()); - return null; + return getETag().getValue(); } @Override public HttpField getLastModified() { - return _content.getLastModified(); + return _precompressedContent.getLastModified(); } @Override public String getLastModifiedValue() { - return _content.getLastModifiedValue(); + return _precompressedContent.getLastModifiedValue(); } @Override diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ResourceHttpContent.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ResourceHttpContent.java index f3d1ea901ad3..ea1b84ee109b 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ResourceHttpContent.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ResourceHttpContent.java @@ -33,8 +33,8 @@ public class ResourceHttpContent implements HttpContent final Resource _resource; final Path _path; final String _contentType; + final HttpField _etag; Map _precompressedContents; - String _etag; public ResourceHttpContent(final Resource resource, final String contentType) { @@ -46,6 +46,7 @@ public ResourceHttpContent(final Resource resource, final String contentType, Ma _resource = resource; _path = resource.getPath(); _contentType = contentType; + _etag = EtagUtils.createWeakEtagField(resource); if (precompressedContents == null) { _precompressedContents = null; @@ -113,19 +114,21 @@ public String getLastModifiedValue() @Override public HttpField getETag() { - return new HttpField(HttpHeader.ETAG, getETagValue()); + return _etag; } @Override public String getETagValue() { - return _resource.getWeakETag(); + if (_etag == null) + return null; + return _etag.getValue(); } @Override public HttpField getContentLength() { - long l = _resource.length(); + long l = getContentLengthValue(); return l == -1 ? null : new HttpField.LongValueHttpField(HttpHeader.CONTENT_LENGTH, l); } diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/EtagUtilsTest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/EtagUtilsTest.java new file mode 100644 index 000000000000..c0a6c3678a6c --- /dev/null +++ b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/EtagUtilsTest.java @@ -0,0 +1,146 @@ +// +// ======================================================================== +// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.http; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.stream.Stream; + +import org.eclipse.jetty.toolchain.test.FS; +import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; +import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@ExtendWith(WorkDirExtension.class) +public class EtagUtilsTest +{ + public WorkDir workDir; + + @Test + public void testCalcWeakETag() throws IOException + { + Path root = workDir.getEmptyPathDir(); + Path testFile = root.resolve("test.dat"); + Files.writeString(testFile, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); + + String weakEtag = EtagUtils.computeWeakEtag(testFile); + assertThat(weakEtag, startsWith("W/\"")); + assertThat(weakEtag, endsWith("\"")); + } + + @Test + public void testCalcWeakETagSameFileDifferentLocations() throws IOException + { + Path root = workDir.getEmptyPathDir(); + Path testFile = root.resolve("test.dat"); + Files.writeString(testFile, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); + + Path altDir = root.resolve("alt"); + FS.ensureDirExists(altDir); + Path altFile = altDir.resolve("test.dat"); + Files.copy(testFile, altFile); + + String weakEtagOriginal = EtagUtils.computeWeakEtag(testFile); + assertThat(weakEtagOriginal, startsWith("W/\"")); + assertThat(weakEtagOriginal, endsWith("\"")); + + String weakEtagAlt = EtagUtils.computeWeakEtag(altFile); + assertThat(weakEtagAlt, startsWith("W/\"")); + assertThat(weakEtagAlt, endsWith("\"")); + + // When referenced locations are different, the etag should be different as well + assertThat(weakEtagAlt, not(is(weakEtagOriginal))); + } + + public static Stream rewriteWithSuffixCases() + { + return Stream.of( + // Simple, not quoted, no suffix in original + Arguments.of("ABCDEF", "-br", "\"ABCDEF-br\""), + // Weak, not quoted, no suffix in original + Arguments.of("W/ABCDEF", "-br", "W/\"ABCDEF-br\""), + // Simple, quoted, no suffix in original + Arguments.of("\"ABCDEF\"", "-br", "\"ABCDEF-br\""), + // Weak, quoted, no suffix in original + Arguments.of("W/\"ABCDEF\"", "--gzip", "W/\"ABCDEF--gzip\""), + // Simple, quoted, gzip suffix in original + Arguments.of("\"ABCDEF-gzip\"", "-br", "\"ABCDEF-br\""), + // Simple, not quoted, gzip suffix in original + Arguments.of("ABCDEF-gzip", "-br", "\"ABCDEF-br\""), + // Weak, quoted, gzip suffix in original, different ETAG_SEPARATOR size + Arguments.of("W/\"ABCDEF-gzip\"", "--br", "W/\"ABCDEF--br\"") + ); + } + + @ParameterizedTest + @MethodSource("rewriteWithSuffixCases") + public void testRewriteWithSuffix(String input, String newSuffix, String expected) + { + String actual = EtagUtils.rewriteWithSuffix(input, newSuffix); + assertThat(actual, is(expected)); + } + + public static Stream matchTrueCases() + { + return Stream.of( + Arguments.of("tag", "tag"), + Arguments.of("\"tag\"", "\"tag\""), + Arguments.of("\"tag\"", "\"tag--gz\""), + Arguments.of("\"tag\"", "\"tag--br\""), + Arguments.of("W/\"1234567\"", "W/\"1234567\""), + Arguments.of("W/\"1234567\"", "W/\"1234567--br\""), + Arguments.of("12345", "\"12345\""), + Arguments.of("\"12345\"", "12345"), + Arguments.of("12345", "\"12345--gzip\""), + Arguments.of("\"12345\"", "12345--gzip") + ); + } + + @ParameterizedTest + @MethodSource("matchTrueCases") + public void testMatchTrue(String etag, String etagWithOptionalSuffix) + { + assertTrue(EtagUtils.matches(etag, etagWithOptionalSuffix)); + } + + public static Stream matchFalseCases() + { + return Stream.of( + Arguments.of("Zag", "Xag--gzip"), + Arguments.of("xtag", "tag"), + Arguments.of("W/\"1234567\"", "W/\"1234111\""), + Arguments.of("W/\"1234567\"", "W/\"1234111--gzip\"") + ); + } + + @ParameterizedTest + @MethodSource("matchFalseCases") + public void testMatchFalse(String etag, String etagWithOptionalSuffix) + { + assertFalse(EtagUtils.matches(etag, etagWithOptionalSuffix)); + } +} diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java index 1a2377673925..b0f2916f43cc 100644 --- a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java +++ b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java @@ -30,7 +30,6 @@ 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; @@ -73,25 +72,8 @@ public void after() } @Test - public void testCompressedContentFormat() + public void testStripSuffixes() { - assertTrue(CompressedContentFormat.tagEquals("tag", "tag")); - assertTrue(CompressedContentFormat.tagEquals("\"tag\"", "\"tag\"")); - 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() + "\""), @@ -132,7 +114,7 @@ public void testStreamBigBlockOneByteAtATime() throws Exception { baos.write(read); } - assertEquals(data, new String(baos.toByteArray(), StandardCharsets.UTF_8)); + assertEquals(data, baos.toString(StandardCharsets.UTF_8)); } @Test diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/CachedContentFactory.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/CachedContentFactory.java index 70386686e4d9..390bcb407e38 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/CachedContentFactory.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/CachedContentFactory.java @@ -30,6 +30,7 @@ import org.eclipse.jetty.http.CompressedContentFormat; import org.eclipse.jetty.http.DateGenerator; +import org.eclipse.jetty.http.EtagUtils; import org.eclipse.jetty.http.HttpContent; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; @@ -405,7 +406,7 @@ public class CachedHttpContent implements HttpContent _lastAccessed = Instant.now(); - _etag = CachedContentFactory.this._etags ? new PreEncodedHttpField(HttpHeader.ETAG, resource.getWeakETag()) : null; + _etag = CachedContentFactory.this._etags ? new PreEncodedHttpField(HttpHeader.ETAG, EtagUtils.computeWeakEtag(resource.getPath())) : null; if (precompressedResources != null) { diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceContentFactory.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceContentFactory.java index 4dfbadbf56a0..e4110315e0c8 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceContentFactory.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceContentFactory.java @@ -73,7 +73,6 @@ public HttpContent getContent(String pathInContext) throws IOException } private HttpContent load(String pathInContext, Resource resource) - throws IOException { if (resource == null || !resource.exists()) return null; @@ -91,10 +90,14 @@ private HttpContent load(String pathInContext, Resource resource) { String compressedPathInContext = pathInContext + format.getExtension(); Resource compressedResource = this._factory.newResource(compressedPathInContext); - if (compressedResource != null && compressedResource.exists() && compressedResource.lastModified().isAfter(resource.lastModified()) && - compressedResource.length() < resource.length()) - compressedContents.put(format, - new ResourceHttpContent(compressedResource, _mimeTypes.getMimeByExtension(compressedPathInContext))); + if (compressedResource == null || !compressedResource.exists()) + continue; + if (compressedResource.lastModified().isBefore(resource.lastModified())) + continue; + if (compressedResource.length() >= resource.length()) + continue; + + compressedContents.put(format, new ResourceHttpContent(compressedResource, _mimeTypes.getMimeByExtension(compressedPathInContext))); } if (!compressedContents.isEmpty()) return new ResourceHttpContent(resource, mt, compressedContents); diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java index 126f4f5e9fdb..9d00c68859bc 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java @@ -29,6 +29,7 @@ import org.eclipse.jetty.http.ByteRange; import org.eclipse.jetty.http.CompressedContentFormat; import org.eclipse.jetty.http.DateParser; +import org.eclipse.jetty.http.EtagUtils; import org.eclipse.jetty.http.HttpContent; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; @@ -175,7 +176,7 @@ public void doGet(Request request, Response response, Callback callback, HttpCon pathInContext = pathInContext.substring(0, pathInContext.length() - 1); if (q != null && q.length() != 0) pathInContext += "?" + q; - Response.sendRedirect(request, response, callback, URIUtil.addPaths(request.getContext().getContextPath(), pathInContext)); + sendRedirect(request, response, callback, URIUtil.addPaths(request.getContext().getContextPath(), pathInContext)); return; } @@ -202,7 +203,7 @@ public void doGet(Request request, Response response, Callback callback, HttpCon } } - // TODO this should be done by HttpContent#getContentEncoding + // TODO this should be done by HttpContent#getContentEncoding? if (isGzippedContent(pathInContext)) response.getHeaders().put(HttpHeader.CONTENT_ENCODING, "gzip"); @@ -214,16 +215,36 @@ public void doGet(Request request, Response response, Callback callback, HttpCon { if (LOG.isDebugEnabled()) LOG.debug("InvalidPathException for pathInContext: {}", pathInContext, e); - Response.writeError(request, response, callback, HttpStatus.NOT_FOUND_404); + writeHttpError(request, response, callback, HttpStatus.NOT_FOUND_404); } catch (IllegalArgumentException e) { LOG.warn("Failed to serve resource: {}", pathInContext, e); if (!response.isCommitted()) - Response.writeError(request, response, callback, e); + writeHttpError(request, response, callback, e); } } + protected void writeHttpError(Request request, Response response, Callback callback, int status) + { + Response.writeError(request, response, callback, status); + } + + protected void writeHttpError(Request request, Response response, Callback callback, Throwable cause) + { + Response.writeError(request, response, callback, cause); + } + + protected void writeHttpError(Request request, Response response, Callback callback, int status, String msg, Throwable cause) + { + Response.writeError(request, response, callback, status, msg, cause); + } + + protected void sendRedirect(Request request, Response response, Callback callback, String target) + { + Response.sendRedirect(request, response, callback, target); + } + private List getPreferredEncodingOrder(Request request) { Enumeration headers = request.getHeaders().getValues(HttpHeader.ACCEPT_ENCODING.asString()); @@ -327,51 +348,31 @@ protected boolean passConditionalHeaders(Request request, Response response, Htt if (_etags) { String etag = content.getETagValue(); - if (ifm != null) + if (etag != null) { - boolean match = false; - if (etag != null && !etag.startsWith("W/")) + if (ifm != null) { - QuotedCSV quoted = new QuotedCSV(true, ifm); - for (String etagWithSuffix : quoted) + String matched = matchesEtag(etag, ifm); + if (matched == null) { - if (CompressedContentFormat.tagEquals(etag, etagWithSuffix)) - { - match = true; - break; - } + writeHttpError(request, response, callback, HttpStatus.PRECONDITION_FAILED_412); + return true; } } - if (!match) - { - Response.writeError(request, response, callback, HttpStatus.PRECONDITION_FAILED_412); - return true; - } - } - - if (ifnm != null && etag != null) - { - // Handle special case of exact match OR gzip exact match - if (CompressedContentFormat.tagEquals(etag, ifnm) && ifnm.indexOf(',') < 0) - { - Response.writeError(request, response, callback, HttpStatus.NOT_MODIFIED_304); - return true; - } - - // Handle list of tags - QuotedCSV quoted = new QuotedCSV(true, ifnm); - for (String tag : quoted) + if (ifnm != null) { - if (CompressedContentFormat.tagEquals(etag, tag)) + String matched = matchesEtag(etag, ifnm); + if (matched != null) { - Response.writeError(request, response, callback, HttpStatus.NOT_MODIFIED_304); + response.getHeaders().put(HttpHeader.ETAG, matched); + writeHttpError(request, response, callback, HttpStatus.NOT_MODIFIED_304); return true; } - } - // If etag requires content to be served, then do not check if-modified-since - return false; + // If etag requires content to be served, then do not check if-modified-since + return false; + } } } @@ -382,14 +383,14 @@ protected boolean passConditionalHeaders(Request request, Response response, Htt String mdlm = content.getLastModifiedValue(); if (ifms.equals(mdlm)) { - Response.writeError(request, response, callback, HttpStatus.NOT_MODIFIED_304); + writeHttpError(request, response, callback, HttpStatus.NOT_MODIFIED_304); return true; } long ifmsl = request.getHeaders().getDateField(HttpHeader.IF_MODIFIED_SINCE); if (ifmsl != -1 && Files.getLastModifiedTime(content.getResource().getPath()).toMillis() / 1000 <= ifmsl / 1000) { - Response.writeError(request, response, callback, HttpStatus.NOT_MODIFIED_304); + writeHttpError(request, response, callback, HttpStatus.NOT_MODIFIED_304); return true; } } @@ -397,20 +398,51 @@ protected boolean passConditionalHeaders(Request request, Response response, Htt // Parse the if[un]modified dates and compare to resource if (ifums != -1 && Files.getLastModifiedTime(content.getResource().getPath()).toMillis() / 1000 > ifums / 1000) { - Response.writeError(request, response, callback, HttpStatus.PRECONDITION_FAILED_412); + writeHttpError(request, response, callback, HttpStatus.PRECONDITION_FAILED_412); return true; } } catch (IllegalArgumentException iae) { if (!response.isCommitted()) - Response.writeError(request, response, callback, HttpStatus.BAD_REQUEST_400, null, iae); + writeHttpError(request, response, callback, HttpStatus.BAD_REQUEST_400, null, iae); throw iae; } return false; } + /** + * Find a matches between a Content ETag and a Request Field ETag reference. + * @param contentETag the content etag to match against (can be null) + * @param requestEtag the request etag (can be null, a single entry, or even a CSV list) + * @return the matched etag, or null if no matches. + */ + private String matchesEtag(String contentETag, String requestEtag) + { + if (contentETag == null || requestEtag == null) + { + return null; + } + + // Per https://www.rfc-editor.org/rfc/rfc9110#section-8.8.3 + // An Etag header field value can contain a "," (comma) within itself. + // If-Match: W/"abc,xyz", "123456" + // This means we have to parse with QuotedCSV all the time, as we cannot just + // test for the existence of a "," (comma) in the value to know if it's delimited or not + QuotedCSV quoted = new QuotedCSV(true, requestEtag); + for (String tag : quoted) + { + if (EtagUtils.matches(contentETag, tag)) + { + return tag; + } + } + + // no matches + return null; + } + protected void sendWelcome(HttpContent content, String pathInContext, boolean endsWithSlash, Request request, Response response, Callback callback) throws Exception { // Redirect to directory @@ -425,7 +457,7 @@ protected void sendWelcome(HttpContent content, String pathInContext, boolean en uri.param(parameter); response.getHeaders().putLongField(HttpHeader.CONTENT_LENGTH, 0); // TODO: can writeRedirect (override) also work for WelcomeActionType.REDIRECT? - Response.sendRedirect(request, response, callback, uri.getPathQuery()); + sendRedirect(request, response, callback, uri.getPathQuery()); return; } } @@ -480,7 +512,7 @@ protected void welcomeActionProcess(Request request, Response response, Callback case REDIRECT -> { response.getHeaders().putLongField(HttpHeader.CONTENT_LENGTH, 0); - Response.sendRedirect(request, response, callback, welcomeAction.target); + sendRedirect(request, response, callback, welcomeAction.target); } case SERVE -> { @@ -518,11 +550,11 @@ private WelcomeAction processWelcome(Request request, Response response) throws return new WelcomeAction(WelcomeActionType.SERVE, welcomeTarget); } - private void sendDirectory(Request request, Response response, HttpContent httpContent, Callback callback, String pathInContext) throws IOException + private void sendDirectory(Request request, Response response, HttpContent httpContent, Callback callback, String pathInContext) { if (!_dirAllowed) { - Response.writeError(request, response, callback, HttpStatus.FORBIDDEN_403); + writeHttpError(request, response, callback, HttpStatus.FORBIDDEN_403); return; } @@ -530,7 +562,7 @@ private void sendDirectory(Request request, Response response, HttpContent httpC String listing = ResourceListing.getAsHTML(httpContent.getResource(), base, pathInContext.length() > 1, request.getHttpURI().getQuery()); if (listing == null) { - Response.writeError(request, response, callback, HttpStatus.FORBIDDEN_403); + writeHttpError(request, response, callback, HttpStatus.FORBIDDEN_403); return; } @@ -755,6 +787,7 @@ public void setPrecompressedFormats(List precompressedF { _precompressedFormats.clear(); _precompressedFormats.addAll(precompressedFormats); + // TODO: this preferred encoding order should be a separate configurable _preferredEncodingOrder.clear(); _preferredEncodingOrder.addAll(_precompressedFormats.stream().map(CompressedContentFormat::getEncoding).toList()); } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java index 3999e7ca7e70..32ab5ef68a25 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java @@ -37,6 +37,7 @@ import org.eclipse.jetty.http.CachingContentFactory; import org.eclipse.jetty.http.CompressedContentFormat; import org.eclipse.jetty.http.DateGenerator; +import org.eclipse.jetty.http.EtagUtils; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; @@ -95,8 +96,6 @@ /** * Resource Handler test - * - * TODO: increase the testing going on here */ @ExtendWith(WorkDirExtension.class) public class ResourceHandlerTest @@ -750,8 +749,35 @@ public void testBigger() throws Exception } @Test - @Disabled - public void testBrotli() throws Exception + public void testBrotliInitialCompressed() throws Exception + { + Files.writeString(docRoot.resolve("data0.txt"), "Hello Text 0", UTF_8); + Files.writeString(docRoot.resolve("data0.txt.br"), "fake brotli", UTF_8); + + _rootResourceHandler.setDirAllowed(false); + _rootResourceHandler.setRedirectWelcome(false); + _rootResourceHandler.setPrecompressedFormats(CompressedContentFormat.BR); + _rootResourceHandler.setEtags(true); + + String rawResponse = _local.getResponse(""" + GET /context/data0.txt HTTP/1.1\r + Host: localhost:8080\r + Connection: close\r + Accept-Encoding: gzip;q=0.9,br\r + \r + """); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200)); + assertThat(response, containsHeaderValue(HttpHeader.CONTENT_LENGTH, "11")); + assertThat(response, containsHeaderValue(HttpHeader.CONTENT_TYPE, "text/plain")); + assertThat(response, containsHeaderValue(HttpHeader.VARY, "Accept-Encoding")); + assertThat(response, containsHeaderValue(HttpHeader.CONTENT_ENCODING, "br")); + String body = response.getContent(); + assertThat(body, containsString("fake brotli")); + } + + @Test + public void testBrotliWithEtags() throws Exception { Files.writeString(docRoot.resolve("data0.txt"), "Hello Text 0", UTF_8); Files.writeString(docRoot.resolve("data0.txt.br"), "fake brotli", UTF_8); @@ -782,13 +808,13 @@ public void testBrotli() throws Exception assertThat(body, containsString("Hello Text 0")); String etag = response.get(HttpHeader.ETAG); - String etagBr = etag.replaceFirst("([^\"]*)\"(.*)\"", "$1\"$2--br\""); + String etagBr = EtagUtils.rewriteWithSuffix(etag, "--br"); rawResponse = _local.getResponse(""" GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:gzip;q=0.9,br\r + Accept-Encoding: gzip;q=0.9,br\r \r """); response = HttpTester.parseResponse(rawResponse); @@ -805,7 +831,7 @@ public void testBrotli() throws Exception GET /context/data0.txt.br HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:br,gzip\r + Accept-Encoding: br,gzip\r \r """); response = HttpTester.parseResponse(rawResponse); @@ -823,7 +849,7 @@ public void testBrotli() throws Exception GET /context/data0.txt.br HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:gzip\r + Accept-Encoding: gzip\r If-None-Match: W/"wobble"\r \r """); @@ -842,7 +868,7 @@ public void testBrotli() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:br\r + Accept-Encoding: br\r If-None-Match: @ETAG@\r \r """.replace("@ETAG@", etagBr)); @@ -854,7 +880,7 @@ public void testBrotli() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:br\r + Accept-Encoding: br\r If-None-Match: @ETAG@\r \r """.replace("@ETAG@", etag)); @@ -866,7 +892,7 @@ public void testBrotli() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:br\r + Accept-Encoding: br\r If-None-Match: W/"foobar",@ETAG@\r \r """.replace("@ETAG@", etagBr)); @@ -878,7 +904,7 @@ public void testBrotli() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:br\r + Accept-Encoding: br\r If-None-Match: W/"foobar",@ETAG@\r \r """.replace("@ETAG@", etag)); @@ -888,7 +914,6 @@ public void testBrotli() throws Exception } @Test - @Disabled public void testCachedBrotli() throws Exception { Files.writeString(docRoot.resolve("data0.txt"), "Hello Text 0", UTF_8); @@ -920,13 +945,13 @@ public void testCachedBrotli() throws Exception assertThat(body, containsString("Hello Text 0")); String etag = response.get(HttpHeader.ETAG); - String etagBr = etag.replaceFirst("([^\"]*)\"(.*)\"", "$1\"$2--br\""); + String etagBr = EtagUtils.rewriteWithSuffix(etag, "--br"); rawResponse = _local.getResponse(""" GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:br\r + Accept-Encoding: br\r \r """); response = HttpTester.parseResponse(rawResponse); @@ -943,7 +968,7 @@ public void testCachedBrotli() throws Exception GET /context/data0.txt.br HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:br\r + Accept-Encoding: br\r \r """); response = HttpTester.parseResponse(rawResponse); @@ -961,7 +986,7 @@ public void testCachedBrotli() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:br\r + Accept-Encoding: br\r If-None-Match: @ETAG@\r \r """.replace("@ETAG@", etagBr)); @@ -973,7 +998,7 @@ public void testCachedBrotli() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:br\r + Accept-Encoding: br\r If-None-Match: @ETAG@\r \r """.replace("@ETAG@", etag)); @@ -985,7 +1010,7 @@ public void testCachedBrotli() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:br\r + Accept-Encoding: br\r If-None-Match: W/"foobar",@ETAG@\r \r """.replace("@ETAG@", etagBr)); @@ -997,7 +1022,7 @@ public void testCachedBrotli() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:br\r + Accept-Encoding: br\r If-None-Match: W/"foobar",@ETAG@\r \r """.replace("@ETAG@", etag)); @@ -1007,7 +1032,6 @@ public void testCachedBrotli() throws Exception } @Test - @Disabled public void testCachedGzip() throws Exception { FS.ensureDirExists(docRoot); @@ -1042,13 +1066,13 @@ public void testCachedGzip() throws Exception assertThat(body, containsString("Hello Text 0")); String etag = response.get(HttpHeader.ETAG); - String etagGzip = etag.replaceFirst("([^\"]*)\"(.*)\"", "$1\"$2--gzip\""); + String etagGzip = EtagUtils.rewriteWithSuffix(etag, "--gzip"); rawResponse = _local.getResponse(""" GET /context/data0.txt HTTP/1.1\r Connection: close\r Host: localhost:8080\r - Accept-Encoding:gzip\r + Accept-Encoding: gzip\r \r """); response = HttpTester.parseResponse(rawResponse); @@ -1065,7 +1089,7 @@ public void testCachedGzip() throws Exception GET /context/data0.txt.gz HTTP/1.1\r Connection: close\r Host: localhost:8080\r - Accept-Encoding:gzip\r + Accept-Encoding: gzip\r \r """); response = HttpTester.parseResponse(rawResponse); @@ -1083,7 +1107,7 @@ public void testCachedGzip() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:gzip\r + Accept-Encoding: gzip\r If-None-Match: @ETAG@\r \r """.replace("@ETAG@", etagGzip)); @@ -1095,7 +1119,7 @@ public void testCachedGzip() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:gzip\r + Accept-Encoding: gzip\r If-None-Match: @ETAG@\r \r """.replace("@ETAG@", etag)); @@ -1107,7 +1131,7 @@ public void testCachedGzip() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:gzip\r + Accept-Encoding: gzip\r If-None-Match: W/"foobar",@ETAG@\r \r """.replace("@ETAG@", etagGzip)); @@ -1119,7 +1143,7 @@ public void testCachedGzip() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:gzip\r + Accept-Encoding: gzip\r If-None-Match: W/"foobar",@ETAG@\r \r """.replace("@ETAG@", etag)); @@ -1273,7 +1297,6 @@ public void testCachingMaxCachedFileSizeRespected() throws Exception public void testCachingMaxCachedFilesRespected() throws Exception { copySimpleTestResource(docRoot); - // TODO explicitly turn on caching long expectedSizeBig = Files.size(docRoot.resolve("big.txt")); long expectedSizeSimple = Files.size(docRoot.resolve("simple.txt")); CachingContentFactory contentFactory = (CachingContentFactory)_rootResourceHandler.getContentFactory(); @@ -1316,7 +1339,6 @@ public void testCachingMaxCachedFilesRespected() throws Exception @Test public void testCachingNotFoundNotCached() throws Exception { - // TODO explicitly turn on caching CachingContentFactory contentFactory = (CachingContentFactory)_rootResourceHandler.getContentFactory(); for (int i = 0; i < 10; i++) @@ -1337,15 +1359,14 @@ public void testCachingNotFoundNotCached() throws Exception } @Test - @Disabled public void testCachingPrecompressedFilesCached() throws Exception { - // TODO explicitly turn on caching + setupBigFiles(docRoot); long expectedSize = Files.size(docRoot.resolve("big.txt")) + Files.size(docRoot.resolve("big.txt.gz")); - CachingContentFactory contentFactory = (CachingContentFactory)_rootResourceHandler.getContentFactory(); _rootResourceHandler.setPrecompressedFormats(CompressedContentFormat.GZIP); + CachingContentFactory contentFactory = (CachingContentFactory)_rootResourceHandler.getContentFactory(); for (int i = 0; i < 10; i++) { @@ -1389,16 +1410,15 @@ public void testCachingPrecompressedFilesCached() throws Exception } @Test - @Disabled public void testCachingPrecompressedFilesCachedEtagged() throws Exception { - // TODO explicitly turn on caching + setupBigFiles(docRoot); long expectedSize = Files.size(docRoot.resolve("big.txt")) + Files.size(docRoot.resolve("big.txt.gz")); - CachingContentFactory contentFactory = (CachingContentFactory)_rootResourceHandler.getContentFactory(); _rootResourceHandler.setPrecompressedFormats(CompressedContentFormat.GZIP); _rootResourceHandler.setEtags(true); + CachingContentFactory contentFactory = (CachingContentFactory)_rootResourceHandler.getContentFactory(); for (int i = 0; i < 10; i++) { @@ -1474,7 +1494,6 @@ public void testCachingPrecompressedFilesCachedEtagged() throws Exception @Test public void testCachingRefreshing() throws Exception { - // TODO explicitly turn on caching Path tempPath = docRoot.resolve("temp.txt"); Files.writeString(tempPath, "temp file"); long expectedSize = Files.size(tempPath); @@ -1529,7 +1548,6 @@ public void testCachingRefreshing() throws Exception public void testCachingWelcomeFileCached() throws Exception { copySimpleTestResource(docRoot); - // TODO explicitly turn on caching long expectedSize = Files.size(docRoot.resolve("directory/welcome.txt")); CachingContentFactory contentFactory = (CachingContentFactory)_rootResourceHandler.getContentFactory(); @@ -1608,7 +1626,6 @@ public void testControlCharacter() throws Exception } @Test - @Disabled public void testCustomCompressionFormats() throws Exception { Files.writeString(docRoot.resolve("data0.txt"), "Hello Text 0", UTF_8); @@ -1630,7 +1647,7 @@ public void testCustomCompressionFormats() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:bzip2, br, gzip\r + Accept-Encoding: bzip2, br, gzip\r \r """); response = HttpTester.parseResponse(rawResponse); @@ -1642,12 +1659,10 @@ public void testCustomCompressionFormats() throws Exception body = response.getContent(); assertThat(body, containsString("fake bzip2")); - // TODO: show accept-encoding search order issue (shouldn't this request return data0.txt.br?) - rawResponse = _local.getResponse(""" GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r - Accept-Encoding:br, gzip\r + Accept-Encoding: br, gzip\r \r """); response = HttpTester.parseResponse(rawResponse); @@ -1661,45 +1676,50 @@ public void testCustomCompressionFormats() throws Exception } @Test - @Disabled public void testDefaultBrotliOverGzip() throws Exception { - Files.writeString(docRoot.resolve("data0.txt"), "Hello Text 0", UTF_8); - Files.writeString(docRoot.resolve("data0.txt.br"), "fake brotli", UTF_8); - Files.writeString(docRoot.resolve("data0.txt.gz"), "fake gzip", UTF_8); + Path textFile = docRoot.resolve("data0.txt"); + Path textBrFile = docRoot.resolve("data0.txt.br"); + Path textGzipFile = docRoot.resolve("data0.txt.gz"); + Files.writeString(textFile, "Hello Text 0", UTF_8); + Files.writeString(textBrFile, "fake brotli", UTF_8); + Files.writeString(textGzipFile, "fake gzip", UTF_8); - _rootResourceHandler.setPrecompressedFormats(CompressedContentFormat.GZIP, CompressedContentFormat.BR); + // This tests the ResourceService Preferred Encoding Order configuration + _rootResourceHandler.setPrecompressedFormats(CompressedContentFormat.BR, CompressedContentFormat.GZIP); String rawResponse; HttpTester.Response response; String body; + // Request Ordered [gzip, compress, br] - should favor [br] due to ResourceService preferred encoding order rawResponse = _local.getResponse(""" GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:gzip, compress, br\r + Accept-Encoding: gzip, compress, br\r \r """); response = HttpTester.parseResponse(rawResponse); assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200)); - assertThat(response, containsHeaderValue(HttpHeader.CONTENT_LENGTH, "11")); + assertThat(response.toString(), response.getField(HttpHeader.CONTENT_LENGTH).getLongValue(), is(Files.size(textBrFile))); assertThat(response, containsHeaderValue(HttpHeader.CONTENT_TYPE, "text/plain")); assertThat(response, containsHeaderValue(HttpHeader.VARY, "Accept-Encoding")); assertThat(response, containsHeaderValue(HttpHeader.CONTENT_ENCODING, "br")); body = response.getContent(); assertThat(body, containsString("fake brotli")); + // Request weighted [br] lower than defaults of [gzip, compress] - should favor [gzip] due to weighting rawResponse = _local.getResponse(""" GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:gzip, compress, br;q=0.9\r + Accept-Encoding: gzip, compress, br;q=0.9\r \r """); response = HttpTester.parseResponse(rawResponse); assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200)); - assertThat(response, containsHeaderValue(HttpHeader.CONTENT_LENGTH, "9")); + assertThat(response.toString(), response.getField(HttpHeader.CONTENT_LENGTH).getLongValue(), is(Files.size(textGzipFile))); assertThat(response, containsHeaderValue(HttpHeader.CONTENT_TYPE, "text/plain")); assertThat(response, containsHeaderValue(HttpHeader.VARY, "Accept-Encoding")); assertThat(response, containsHeaderValue(HttpHeader.CONTENT_ENCODING, "gzip")); @@ -1892,35 +1912,6 @@ public void testDirectoryOfCollections() throws Exception assertThat(response.getStatus(), is(HttpStatus.OK_200)); } - @Test - public void testEtagIfMatchAlwaysFailsDueToWeakEtag() throws Exception - { - copyBigText(docRoot); - _rootResourceHandler.setEtags(true); - - HttpTester.Response response = HttpTester.parseResponse( - _local.getResponse(""" - GET /context/big.txt HTTP/1.1\r - Host: local\r - Connection: close\r - \r - """)); - - assertThat(response.getStatus(), is(HttpStatus.OK_200)); - assertThat(response.get(ETAG), notNullValue()); - String etag = response.get(ETAG); - - response = HttpTester.parseResponse( - _local.getResponse(""" - GET /context/big.txt HTTP/1.1\r - Host: local\r - Connection: close\r - If-Match: %s\r - \r - """.formatted(etag))); - assertThat(response.getStatus(), is(HttpStatus.PRECONDITION_FAILED_412)); - } - @Test public void testEtagIfNoneMatchModifiedFile() throws Exception { @@ -2042,7 +2033,7 @@ public void testGetUtf8NfcFile() throws Exception rawResponse = _local.getResponse(""" GET /context/swedish-%C3%A5.txt HTTP/1.1\r Host: test\r - Connection:close\r + Connection: close\r \r """); response = HttpTester.parseResponse(rawResponse); @@ -2053,7 +2044,7 @@ public void testGetUtf8NfcFile() throws Exception rawResponse = _local.getResponse(""" GET /context/swedish-a%CC%8A.txt HTTP/1.1\r Host: test\r - Connection:close\r + Connection: close\r \r """); response = HttpTester.parseResponse(rawResponse); @@ -2088,7 +2079,7 @@ public void testGetUtf8NfdFile() throws Exception rawResponse = _local.getResponse(""" GET /context/swedish-a%CC%8A.txt HTTP/1.1\r Host: test\r - Connection:close\r + Connection: close\r \r """); response = HttpTester.parseResponse(rawResponse); @@ -2099,7 +2090,7 @@ public void testGetUtf8NfdFile() throws Exception rawResponse = _local.getResponse(""" GET /context/swedish-%C3%A5.txt HTTP/1.1\r Host: test\r - Connection:close\r + Connection: close\r \r """); response = HttpTester.parseResponse(rawResponse); @@ -2115,7 +2106,6 @@ public void testGetUtf8NfdFile() throws Exception } @Test - @Disabled public void testGzip() throws Exception { FS.ensureDirExists(docRoot); @@ -2149,7 +2139,7 @@ public void testGzip() throws Exception body = response.getContent(); assertThat(body, containsString("Hello Text 0")); String etag = response.get(HttpHeader.ETAG); - String etagGzip = etag.replaceFirst("([^\"]*)\"(.*)\"", "$1\"$2--gzip\""); + String etagGzip = EtagUtils.rewriteWithSuffix(etag, "--gzip"); rawResponse = _local.getResponse(""" GET /context/data0.txt HTTP/1.1\r @@ -2205,12 +2195,12 @@ public void testGzip() throws Exception body = response.getContent(); assertThat(body, containsString("fake gzip")); - String badEtagGzip = etag.replaceFirst("([^\"]*)\"(.*)\"", "$1\"$2X--gzip\""); + String badEtagGzip = EtagUtils.rewriteWithSuffix(etag, "-gzip"); rawResponse = _local.getResponse(""" GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:gzip\r + Accept-Encoding: gzip\r If-None-Match: @ETAG@\r \r """.replace("@ETAG@", badEtagGzip)); @@ -2221,7 +2211,7 @@ public void testGzip() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:gzip\r + Accept-Encoding: gzip\r If-None-Match: @ETAG@\r \r """.replace("@ETAG@", etagGzip)); @@ -2233,7 +2223,7 @@ public void testGzip() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:gzip\r + Accept-Encoding: gzip\r If-None-Match: @ETAG@\r \r """.replace("@ETAG@", etag)); @@ -2245,7 +2235,7 @@ public void testGzip() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:gzip\r + Accept-Encoding: gzip\r If-None-Match: W/"foobar",@ETAG@\r \r """.replace("@ETAG@", etagGzip)); @@ -2256,7 +2246,7 @@ public void testGzip() throws Exception rawResponse = _local.getResponse(""" GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r - Accept-Encoding:gzip\r + Accept-Encoding: gzip\r If-None-Match: W/"foobar",@ETAG@\r \r """.replace("@ETAG@", etag)); @@ -2316,16 +2306,9 @@ public void testHeaders() throws Exception "Hello World", "Now is the time for all good men to come to the aid of the party" }) - @Disabled public void testIfETag(String content) throws Exception { Files.writeString(docRoot.resolve("file.txt"), content, UTF_8); - - /* TODO: need way to configure resource cache? - resourceHandler.setInitParameter("maxCacheSize", "4096"); - resourceHandler.setInitParameter("maxCachedFileSize", "25"); - resourceHandler.setInitParameter("maxCachedFiles", "100"); - */ _rootResourceHandler.setEtags(true); String rawResponse; @@ -2334,7 +2317,7 @@ public void testIfETag(String content) throws Exception rawResponse = _local.getResponse(""" GET /context/file.txt HTTP/1.1\r Host:test\r - Connection:close\r + Connection: close\r \r """); response = HttpTester.parseResponse(rawResponse); @@ -2346,7 +2329,7 @@ public void testIfETag(String content) throws Exception rawResponse = _local.getResponse(""" GET /context/file.txt HTTP/1.1\r Host:test\r - Connection:close\r + Connection: close\r If-None-Match: @ETAG@\r \r """.replace("@ETAG@", etag)); @@ -2356,17 +2339,17 @@ public void testIfETag(String content) throws Exception rawResponse = _local.getResponse(""" GET /context/file.txt HTTP/1.1\r Host: test\r - Connection:close\r + Connection: close\r If-None-Match: wibble,@ETAG@,wobble\r \r - """.replace("@ETAG", etag)); + """.replace("@ETAG@", etag)); response = HttpTester.parseResponse(rawResponse); assertThat(response.toString(), response.getStatus(), is(HttpStatus.NOT_MODIFIED_304)); rawResponse = _local.getResponse(""" GET /context/file.txt HTTP/1.1\r Host: test\r - Connection:close\r + Connection: close\r If-None-Match: wibble\r \r """); @@ -2376,7 +2359,7 @@ public void testIfETag(String content) throws Exception rawResponse = _local.getResponse(""" GET /context/file.txt HTTP/1.1\r Host: test\r - Connection:close\r + Connection: close\r If-None-Match: wibble, wobble\r \r """); @@ -2386,27 +2369,27 @@ public void testIfETag(String content) throws Exception rawResponse = _local.getResponse(""" GET /context/file.txt HTTP/1.1\r Host: test\r - Connection:close\r + Connection: close\r If-Match: @ETAG@\r \r - """.replace("@ETAG", etag)); + """.replace("@ETAG@", etag)); response = HttpTester.parseResponse(rawResponse); assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200)); rawResponse = _local.getResponse(""" GET /context/file.txt HTTP/1.1\r Host: test\r - Connection:close\r + Connection: close\r If-Match: wibble,@ETAG@,wobble\r \r - """.replace("@ETAG", etag)); + """.replace("@ETAG@", etag)); response = HttpTester.parseResponse(rawResponse); assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200)); rawResponse = _local.getResponse(""" GET /context/file.txt HTTP/1.1\r Host: test\r - Connection:close\r + Connection: close\r If-Match: wibble\r \r """); @@ -2416,7 +2399,7 @@ public void testIfETag(String content) throws Exception rawResponse = _local.getResponse(""" GET /context/file.txt HTTP/1.1\r Host: test\r - Connection:close\r + Connection: close\r If-Match: wibble, wobble\r \r """); @@ -2456,7 +2439,7 @@ public void testIfModified(String content) throws Exception rawResponse = _local.getResponse(""" GET /context/file.txt HTTP/1.1\r Host:test\r - Connection:close\r + Connection: close\r \r """); response = HttpTester.parseResponse(rawResponse); @@ -2468,7 +2451,7 @@ public void testIfModified(String content) throws Exception rawResponse = _local.getResponse(""" GET /context/file.txt HTTP/1.1\r Host:test\r - Connection:close\r + Connection: close\r If-Modified-Since: @LASTMODIFIED@\r \r """.replace("@LASTMODIFIED@", lastModified)); @@ -2478,7 +2461,7 @@ public void testIfModified(String content) throws Exception rawResponse = _local.getResponse(""" GET /context/file.txt HTTP/1.1\r Host:test\r - Connection:close\r + Connection: close\r If-Modified-Since: @DATE@\r \r """.replace("@DATE@", DateGenerator.formatDate(System.currentTimeMillis() - 10000))); @@ -2488,7 +2471,7 @@ public void testIfModified(String content) throws Exception rawResponse = _local.getResponse(""" GET /context/file.txt HTTP/1.1\r Host:test\r - Connection:close\r + Connection: close\r If-Modified-Since: @DATE@\r \r """.replace("@DATE@", DateGenerator.formatDate(System.currentTimeMillis() + 10000))); @@ -2498,7 +2481,7 @@ public void testIfModified(String content) throws Exception rawResponse = _local.getResponse(""" GET /context/file.txt HTTP/1.1\r Host:test\r - Connection:close\r + Connection: close\r If-Unmodified-Since: @DATE@\r \r """.replace("@DATE@", DateGenerator.formatDate(System.currentTimeMillis() + 10000))); @@ -2508,7 +2491,7 @@ public void testIfModified(String content) throws Exception rawResponse = _local.getResponse(""" GET /context/file.txt HTTP/1.1\r Host:test\r - Connection:close\r + Connection: close\r If-Unmodified-Since: @DATE@\r \r """.replace("@DATE@", DateGenerator.formatDate(System.currentTimeMillis() - 10000))); @@ -2917,7 +2900,6 @@ public void testPrecompressedGzipNoopsWhenCompressedFileDoesNotExist() throws Ex } @Test - @Disabled public void testPrecompressedGzipWorks() throws Exception { setupBigFiles(docRoot); @@ -3017,7 +2999,6 @@ public void testPrecompressedPreferredEncodingOrderWithQuality() throws Exceptio } @Test - @Disabled public void testProgrammaticCustomCompressionFormats() throws Exception { Files.writeString(docRoot.resolve("data0.txt"), "Hello Text 0", UTF_8); @@ -3039,7 +3020,7 @@ public void testProgrammaticCustomCompressionFormats() throws Exception GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r Connection: close\r - Accept-Encoding:bzip2, br, gzip\r + Accept-Encoding: bzip2, br, gzip\r \r """); response = HttpTester.parseResponse(rawResponse); @@ -3056,7 +3037,7 @@ public void testProgrammaticCustomCompressionFormats() throws Exception rawResponse = _local.getResponse(""" GET /context/data0.txt HTTP/1.1\r Host: localhost:8080\r - Accept-Encoding:br, gzip\r + Accept-Encoding: br, gzip\r \r """); response = HttpTester.parseResponse(rawResponse); diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/FileID.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/FileID.java index 277741f6fc0a..b890a332d839 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/FileID.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/FileID.java @@ -45,6 +45,25 @@ public static String getBasename(Path path) return basename; } + /** + * Get the last segment of the URI returning it as the filename + * + * @param uri the URI to look for the filename + * @return The last segment of the uri + */ + public static String getFileName(URI uri) + { + if (uri == null) + return ""; + String path = uri.getPath(); + if (path == null || "/".equals(path)) + return ""; + int idx = path.lastIndexOf('/'); + if (idx >= 0) + return path.substring(idx + 1); + return path; + } + /** * Retrieve the extension of a URI path. * This is the name of the last segment of the URI path with a substring diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/MemoryResource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/MemoryResource.java index 103782732e80..c61595e04ca8 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/MemoryResource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/MemoryResource.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Objects; +import org.eclipse.jetty.util.FileID; import org.eclipse.jetty.util.IO; /** @@ -77,7 +78,7 @@ public String getName() { Path p = getPath(); if (p == null) - return null; + return _uri.toASCIIString(); return p.toAbsolutePath().toString(); } @@ -86,7 +87,7 @@ public String getFileName() { Path p = getPath(); if (p == null) - return null; + return FileID.getFileName(_uri); Path fn = p.getFileName(); if (fn == null) return ""; // no segments, so no filename diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java index 75075595749d..8db2c97142bd 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java @@ -29,7 +29,6 @@ import java.nio.file.attribute.FileTime; import java.time.Instant; import java.util.ArrayList; -import java.util.Base64; import java.util.Collection; import java.util.Iterator; import java.util.List; @@ -372,48 +371,6 @@ public void copyTo(Path destination) } } - /** - * Generate a weak ETag reference for this Resource. - * - * @return the weak ETag reference for this resource. - */ - public String getWeakETag() - { - return getWeakETag(""); - } - - public String getWeakETag(String suffix) - { - StringBuilder b = new StringBuilder(32); - b.append("W/\""); - - String name = getName(); - int length = name.length(); - long lhash = 0; - for (int i = 0; i < length; i++) - { - lhash = 31 * lhash + name.charAt(i); - } - - Base64.Encoder encoder = Base64.getEncoder().withoutPadding(); - b.append(encoder.encodeToString(longToBytes(lastModified().toEpochMilli() ^ lhash))); - b.append(encoder.encodeToString(longToBytes(length() ^ lhash))); - b.append(suffix); - b.append('"'); - return b.toString(); - } - - private static byte[] longToBytes(long value) - { - byte[] result = new byte[Long.BYTES]; - for (int i = Long.BYTES - 1; i >= 0; i--) - { - result[i] = (byte)(value & 0xFF); - value >>= 8; - } - return result; - } - public Collection getAllResources() { try diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/FileIDTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/FileIDTest.java index 7bd4175cbacb..4b15ce610ad7 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/FileIDTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/FileIDTest.java @@ -95,6 +95,29 @@ public void testGetBasename(String input, String expected) throws IOException } } + public static Stream fileNameSource() + { + return Stream.of( + Arguments.of(null, ""), + Arguments.of(URI.create("file:/"), ""), + Arguments.of(URI.create("file:///"), ""), + Arguments.of(URI.create("file:zed/"), ""), + Arguments.of(URI.create("file:///path/to/test.txt"), "test.txt"), + Arguments.of(URI.create("file:///path/to/dir/"), ""), + Arguments.of(URI.create("http://eclipse.org/jetty/"), ""), + Arguments.of(URI.create("http://eclipse.org/jetty/index.html"), "index.html"), + Arguments.of(URI.create("http://eclipse.org/jetty/docs.html?query=val#anchor"), "docs.html") + ); + } + + @ParameterizedTest + @MethodSource("fileNameSource") + public void testGetFileName(URI uri, String expected) + { + String actual = FileID.getFileName(uri); + assertThat(actual, is(expected)); + } + public static Stream hasNamedPathSegmentTrueCases() { return Stream.of( diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java index 81134890e19e..df4f20adee09 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java @@ -40,6 +40,7 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.http.HttpServletResponseWrapper; +import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.CachingContentFactory; import org.eclipse.jetty.http.CompressedContentFormat; import org.eclipse.jetty.http.HttpContent; @@ -48,6 +49,7 @@ import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeaderValue; +import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http.MimeTypes; @@ -1014,6 +1016,48 @@ protected void welcomeActionProcess(Request coreRequest, Response coreResponse, } } + @Override + protected void writeHttpError(Request coreRequest, Response coreResponse, Callback callback, int statusCode) + { + writeHttpError(coreRequest, coreResponse, callback, statusCode, null, null); + } + + @Override + protected void writeHttpError(Request coreRequest, Response coreResponse, Callback callback, Throwable cause) + { + int statusCode = HttpStatus.INTERNAL_SERVER_ERROR_500; + String reason = null; + if (cause instanceof BadMessageException badMessageException) + { + statusCode = badMessageException.getCode(); + reason = badMessageException.getReason(); + } + writeHttpError(coreRequest, coreResponse, callback, statusCode, reason, cause); + } + + @Override + protected void writeHttpError(Request coreRequest, Response coreResponse, Callback callback, int statusCode, String reason, Throwable cause) + { + HttpServletRequest request = getServletRequest(coreRequest); + HttpServletResponse response = getServletResponse(coreResponse); + try + { + // TODO: not sure if this is allowed here. + if (cause != null) + request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, cause); + response.sendError(statusCode, reason); + } + catch (IOException e) + { + // TODO: Need a better exception? + throw new RuntimeException(e); + } + finally + { + callback.succeeded(); + } + } + @Override protected boolean passConditionalHeaders(Request request, Response response, HttpContent content, Callback callback) throws IOException { diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CachedContentFactory.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CachedContentFactory.java index c11d486fd4bb..b65a2c2173e8 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CachedContentFactory.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CachedContentFactory.java @@ -30,6 +30,7 @@ import org.eclipse.jetty.http.CompressedContentFormat; import org.eclipse.jetty.http.DateGenerator; +import org.eclipse.jetty.http.EtagUtils; import org.eclipse.jetty.http.HttpContent; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; @@ -403,7 +404,7 @@ public class CachedHttpContent implements HttpContent _lastAccessed = Instant.now(); - _etag = CachedContentFactory.this._etags ? new PreEncodedHttpField(HttpHeader.ETAG, resource.getWeakETag()) : null; + _etag = CachedContentFactory.this._etags ? EtagUtils.createWeakEtagField(resource) : null; if (precompressedResources != null) { @@ -444,6 +445,8 @@ public HttpField getETag() @Override public String getETagValue() { + if (_etag == null) + return null; return _etag.getValue(); } @@ -603,7 +606,7 @@ public class CachedPrecompressedHttpContent extends PrecompressedHttpContent _content = content; _precompressedContent = precompressedContent; - _etag = (CachedContentFactory.this._etags) ? new PreEncodedHttpField(HttpHeader.ETAG, _content.getResource().getWeakETag(format.getEtagSuffix())) : null; + _etag = (CachedContentFactory.this._etags) ? EtagUtils.createWeakEtagField(_content.getResource(), format.getEtagSuffix()) : null; } public boolean isValid() diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java index fdbb88a75d99..d00c848ef540 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java @@ -39,6 +39,7 @@ import org.eclipse.jetty.ee9.nested.resource.SeekableByteChannelRangeWriter; import org.eclipse.jetty.http.CompressedContentFormat; import org.eclipse.jetty.http.DateParser; +import org.eclipse.jetty.http.EtagUtils; import org.eclipse.jetty.http.HttpContent; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; @@ -545,7 +546,7 @@ protected boolean passConditionalHeaders(HttpServletRequest request, HttpServlet QuotedCSV quoted = new QuotedCSV(true, ifm); for (String etagWithSuffix : quoted) { - if (CompressedContentFormat.tagEquals(etag, etagWithSuffix)) + if (EtagUtils.matches(etag, etagWithSuffix)) { match = true; break; @@ -563,7 +564,7 @@ protected boolean passConditionalHeaders(HttpServletRequest request, HttpServlet if (ifnm != null && etag != null) { // Handle special case of exact match OR gzip exact match - if (CompressedContentFormat.tagEquals(etag, ifnm) && ifnm.indexOf(',') < 0) + if (EtagUtils.matches(etag, ifnm) && ifnm.indexOf(',') < 0) { sendStatus(response, HttpServletResponse.SC_NOT_MODIFIED, ifnm::toString); return false; @@ -573,7 +574,7 @@ protected boolean passConditionalHeaders(HttpServletRequest request, HttpServlet QuotedCSV quoted = new QuotedCSV(true, ifnm); for (String tag : quoted) { - if (CompressedContentFormat.tagEquals(etag, tag)) + if (EtagUtils.matches(etag, tag)) { sendStatus(response, HttpServletResponse.SC_NOT_MODIFIED, tag::toString); return false;