diff --git a/core/src/main/java/io/undertow/UndertowLogger.java b/core/src/main/java/io/undertow/UndertowLogger.java index cdc66549e9..aed93fea64 100644 --- a/core/src/main/java/io/undertow/UndertowLogger.java +++ b/core/src/main/java/io/undertow/UndertowLogger.java @@ -458,4 +458,20 @@ void nodeConfigCreated(URI connectionURI, String balancer, String domain, String @LogMessage(level = WARN) @Message(id = 5102, value = "Flushing waiting in a frame more than %s miliseconds. The framed channel will be forcibly closed.") void noFrameflushInTimeout(long timeoutMiliseconds); + + @LogMessage(level = WARN) + @Message(id = 5103, value = "Cache TTL set to wrong value '%sms'. Defaulting to '%sms'.") + void wrongCacheTTLValue(int ttl, int defaultTtl); + + @LogMessage(level = WARN) + @Message(id = 5104, value = "Could not register resource change listener for caching resource manager, automatic invalidation of cached resource will not work. TTL value configured '%sms'. Defaulting to '%sms'.") + void failedToRegisterChangeListener(int ttl, int defaultTtl, @Cause Exception e); + + @LogMessage(level = WARN) + @Message(id = 5105, value = "Cache entry content mismatch for '%s'. Expected length '%s', but was '%s'.") + void cacheEntryMismatchContent(Object key, int cacheIndicatedSize, long written); + + @LogMessage(level = WARN) + @Message(id = 5106, value = "Content mismatch for '%s'. Expected length '%s', but was '%s'.") + void contentEntryMismatch(Object key, long indicatedSize, long written); } diff --git a/core/src/main/java/io/undertow/server/DefaultByteBufferPool.java b/core/src/main/java/io/undertow/server/DefaultByteBufferPool.java index 811b0374e2..16379bdc27 100644 --- a/core/src/main/java/io/undertow/server/DefaultByteBufferPool.java +++ b/core/src/main/java/io/undertow/server/DefaultByteBufferPool.java @@ -26,9 +26,10 @@ import java.nio.ByteBuffer; import java.util.ArrayDeque; import java.util.ArrayList; -import java.util.HashMap; +import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.WeakHashMap; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; @@ -334,7 +335,7 @@ protected void finalize() throws Throwable { // class can be called by a different thread than the one that initialized the data. private static class ThreadLocalCache { - Map localsByThread = new HashMap<>(); + Map localsByThread = Collections.synchronizedMap(new WeakHashMap<>()); ThreadLocalData get() { return localsByThread.get(Thread.currentThread()); diff --git a/core/src/main/java/io/undertow/server/handlers/cache/DirectBufferCache.java b/core/src/main/java/io/undertow/server/handlers/cache/DirectBufferCache.java index 24ddf535ee..520da284be 100644 --- a/core/src/main/java/io/undertow/server/handlers/cache/DirectBufferCache.java +++ b/core/src/main/java/io/undertow/server/handlers/cache/DirectBufferCache.java @@ -28,6 +28,8 @@ import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import java.util.concurrent.ConcurrentHashMap; +import io.undertow.UndertowLogger; +import io.undertow.server.handlers.resource.CachingResourceManager; import io.undertow.util.ConcurrentDirectDeque; import org.xnio.BufferAllocator; @@ -95,14 +97,13 @@ public CacheEntry get(Object key) { return null; } - long expires = cacheEntry.getExpires(); - if(expires != -1) { - if(System.currentTimeMillis() > expires) { + final long expires = cacheEntry.getExpires(); + if(expires == CachingResourceManager.MAX_AGE_NO_CACHING || (expires > 0 && System.currentTimeMillis() > expires)) { remove(key); return null; - } } + //either did not expire or CachingResourceManager.MAX_AGE_NO_EXPIRY if (cacheEntry.hit() % SAMPLE_INTERVAL == 0) { bumpAccess(cacheEntry); @@ -234,12 +235,20 @@ public boolean enabled() { } public void enable() { - if(maxAge == -1) { - this.expires = -1; - } else { + if(this.maxAge == CachingResourceManager.MAX_AGE_NO_CACHING) { + this.expires = CachingResourceManager.MAX_AGE_NO_CACHING; + disable(); + } else if(this.maxAge == CachingResourceManager.MAX_AGE_NO_EXPIRY) { + this.expires = CachingResourceManager.MAX_AGE_NO_EXPIRY; + this.enabled = 2; + } else if(this.maxAge > 0) { this.expires = System.currentTimeMillis() + maxAge; + this.enabled = 2; + } else { + this.expires = CachingResourceManager.MAX_AGE_NO_CACHING; + UndertowLogger.ROOT_LOGGER.wrongCacheTTLValue(this.maxAge, CachingResourceManager.MAX_AGE_NO_CACHING); + disable(); } - this.enabled = 2; } public void disable() { diff --git a/core/src/main/java/io/undertow/server/handlers/cache/ResponseCachingSender.java b/core/src/main/java/io/undertow/server/handlers/cache/ResponseCachingSender.java index c1e370cde5..2193f70a2a 100644 --- a/core/src/main/java/io/undertow/server/handlers/cache/ResponseCachingSender.java +++ b/core/src/main/java/io/undertow/server/handlers/cache/ResponseCachingSender.java @@ -23,6 +23,7 @@ import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import io.undertow.UndertowLogger; import io.undertow.io.IoCallback; import io.undertow.io.Sender; import org.xnio.Buffers; @@ -124,6 +125,7 @@ public void close(final IoCallback callback) { @Override public void close() { if (written != length) { + UndertowLogger.ROOT_LOGGER.contentEntryMismatch(cacheEntry.key(),length, written); cacheEntry.disable(); cacheEntry.dereference(); } @@ -142,7 +144,13 @@ private void handleUpdate(final ByteBuffer origSrc) { //prepare buffers for reading buffer.flip(); } - cacheEntry.enable(); + if(written == cacheEntry.size()) { + cacheEntry.enable(); + } else { + UndertowLogger.ROOT_LOGGER.cacheEntryMismatchContent(cacheEntry.key(),cacheEntry.size(), written); + cacheEntry.disable(); + cacheEntry.dereference(); + } } } @@ -170,7 +178,13 @@ private void handleUpdate(final ByteBuffer[] origSrc, long totalWritten) { //prepare buffers for reading buffer.flip(); } - cacheEntry.enable(); + if(cacheEntry.size() == written) { + cacheEntry.enable(); + } else { + UndertowLogger.ROOT_LOGGER.cacheEntryMismatchContent(cacheEntry.key(),cacheEntry.size(), written); + cacheEntry.disable(); + cacheEntry.dereference(); + } } } } diff --git a/core/src/main/java/io/undertow/server/handlers/cache/ResponseCachingStreamSinkConduit.java b/core/src/main/java/io/undertow/server/handlers/cache/ResponseCachingStreamSinkConduit.java index 83b2c68843..0206ce7623 100644 --- a/core/src/main/java/io/undertow/server/handlers/cache/ResponseCachingStreamSinkConduit.java +++ b/core/src/main/java/io/undertow/server/handlers/cache/ResponseCachingStreamSinkConduit.java @@ -30,6 +30,8 @@ import org.xnio.conduits.Conduits; import org.xnio.conduits.StreamSinkConduit; +import io.undertow.UndertowLogger; + /** * @author Stuart Douglas */ @@ -82,7 +84,13 @@ public int write(final ByteBuffer src) throws IOException { //prepare buffers for reading buffer.flip(); } - cacheEntry.enable(); + if(cacheEntry.size() == written) { + cacheEntry.enable(); + } else { + UndertowLogger.ROOT_LOGGER.cacheEntryMismatchContent(cacheEntry.key(),cacheEntry.size(), written); + cacheEntry.disable(); + cacheEntry.dereference(); + } } } return totalWritten; @@ -120,7 +128,13 @@ public long write(final ByteBuffer[] srcs, final int offs, final int len) throws //prepare buffers for reading buffer.flip(); } - cacheEntry.enable(); + if(cacheEntry.size() == written) { + cacheEntry.enable(); + } else { + UndertowLogger.ROOT_LOGGER.cacheEntryMismatchContent(cacheEntry.key(),cacheEntry.size(), written); + cacheEntry.disable(); + cacheEntry.dereference(); + } } } return totalWritten; diff --git a/core/src/main/java/io/undertow/server/handlers/resource/CachedResource.java b/core/src/main/java/io/undertow/server/handlers/resource/CachedResource.java index a471653b09..aac707b8d2 100644 --- a/core/src/main/java/io/undertow/server/handlers/resource/CachedResource.java +++ b/core/src/main/java/io/undertow/server/handlers/resource/CachedResource.java @@ -65,10 +65,16 @@ public CachedResource(final CachingResourceManager cachingResourceManager, final this.eTag = underlyingResource.getETag(); this.name = underlyingResource.getName(); this.cacheKey = new CacheKey(cachingResourceManager, underlyingResource.getCacheKey()); - if (cachingResourceManager.getMaxAge() > 0) { - nextMaxAgeCheck = System.currentTimeMillis() + cachingResourceManager.getMaxAge(); + final int managerMaxAge = cachingResourceManager.getMaxAge(); + if (managerMaxAge > 0) { + nextMaxAgeCheck = System.currentTimeMillis() + managerMaxAge; + } else if(managerMaxAge == CachingResourceManager.MAX_AGE_NO_CACHING){ + nextMaxAgeCheck = CachingResourceManager.MAX_AGE_NO_CACHING; + } else if(managerMaxAge == CachingResourceManager.MAX_AGE_NO_EXPIRY){ + nextMaxAgeCheck = CachingResourceManager.MAX_AGE_NO_EXPIRY; } else { - nextMaxAgeCheck = -1; + UndertowLogger.ROOT_LOGGER.wrongCacheTTLValue(managerMaxAge, CachingResourceManager.MAX_AGE_NO_CACHING); + this.nextMaxAgeCheck = CachingResourceManager.MAX_AGE_NO_CACHING; } } @@ -127,9 +133,14 @@ public boolean checkStillValid() { if (!underlyingResource.getLastModified().equals(lastModifiedDate)) { return false; } + } else { + return true; } + } else if(nextMaxAgeCheck == CachingResourceManager.MAX_AGE_NO_EXPIRY) { + //this is essentially equal to 0/-1 prior to UNDERTOW-2332 + return true; } - return true; + return false; } @Override diff --git a/core/src/main/java/io/undertow/server/handlers/resource/CachingResourceManager.java b/core/src/main/java/io/undertow/server/handlers/resource/CachingResourceManager.java index 5ed4135895..2c9f88a5da 100644 --- a/core/src/main/java/io/undertow/server/handlers/resource/CachingResourceManager.java +++ b/core/src/main/java/io/undertow/server/handlers/resource/CachingResourceManager.java @@ -31,6 +31,14 @@ */ public class CachingResourceManager implements ResourceManager { + /** + * Max age 0, indicating that entries expire upon creation and are not retained; + */ + public static final int MAX_AGE_NO_CACHING = 0; + /** + * Mage age -1, this force manager to retain entries until underlying resource manager indicate that entries expired/changed + */ + public static final int MAX_AGE_NO_EXPIRY = -1; /** * The biggest file size we cache */ @@ -51,14 +59,21 @@ public class CachingResourceManager implements ResourceManager { */ private final LRUCache cache; - private final int maxAge; + private int maxAge; public CachingResourceManager(final int metadataCacheSize, final long maxFileSize, final DirectBufferCache dataCache, final ResourceManager underlyingResourceManager, final int maxAge) { this.maxFileSize = maxFileSize; this.underlyingResourceManager = underlyingResourceManager; this.dataCache = dataCache; + + if(maxAge > 0 || maxAge == MAX_AGE_NO_CACHING || maxAge == MAX_AGE_NO_EXPIRY) { + this.maxAge = maxAge; + } else { + UndertowLogger.ROOT_LOGGER.wrongCacheTTLValue(maxAge, MAX_AGE_NO_CACHING); + this.maxAge = MAX_AGE_NO_CACHING; + } + this.cache = new LRUCache<>(metadataCacheSize, maxAge); - this.maxAge = maxAge; if(underlyingResourceManager.isResourceChangeListenerSupported()) { try { underlyingResourceManager.registerResourceChangeListener(new ResourceChangeListener() { @@ -70,7 +85,12 @@ public void handleChanges(Collection changes) { } }); } catch (Exception e) { - UndertowLogger.ROOT_LOGGER.couldNotRegisterChangeListener(e); + int errorMaxAge = this.maxAge; + if(!(this.maxAge > 0 || this.maxAge == CachingResourceManager.MAX_AGE_NO_CACHING)) { + errorMaxAge = CachingResourceManager.MAX_AGE_NO_CACHING; + } + UndertowLogger.ROOT_LOGGER.failedToRegisterChangeListener(this.maxAge,errorMaxAge,e); + this.maxAge = errorMaxAge; } } } @@ -116,7 +136,9 @@ public CachedResource getResource(final String p) throws IOException { } final Resource underlying = underlyingResourceManager.getResource(path); if (underlying == null) { - cache.add(path, new NoResourceMarker(maxAge > 0 ? System.currentTimeMillis() + maxAge : -1)); + if(this.maxAge != MAX_AGE_NO_CACHING) { + cache.add(path, new NoResourceMarker(maxAge > 0 ? System.currentTimeMillis() + maxAge : -1)); + } return null; } final CachedResource resource = new CachedResource(this, underlying, path); diff --git a/pom.xml b/pom.xml index 6691df40fc..5bbb0fc072 100644 --- a/pom.xml +++ b/pom.xml @@ -106,6 +106,9 @@ 5.1.1 1.21 + true + false + false @@ -135,6 +138,7 @@ following setting. --> none + ${maven.javadoc.plugin.quiet} @@ -173,6 +177,20 @@ + + + org.apache.maven.plugins + maven-compiler-plugin + ${version.compiler.plugin} + + + true + false + + ${maven.compiler.showDeprecation} + ${maven.compiler.showWarnings} + + @@ -674,6 +692,28 @@ karaf + + jdk18 + + [18,) + + + -Djava.security.manager=allow + + + + verbose + + + findbugs + + + + false + true + false + + diff --git a/servlet/src/test/java/io/undertow/servlet/test/response/writer/AsyncResponseWriterOnPostServlet.java b/servlet/src/test/java/io/undertow/servlet/test/response/writer/AsyncResponseWriterOnPostServlet.java index 45c1598a3c..493e439e1d 100644 --- a/servlet/src/test/java/io/undertow/servlet/test/response/writer/AsyncResponseWriterOnPostServlet.java +++ b/servlet/src/test/java/io/undertow/servlet/test/response/writer/AsyncResponseWriterOnPostServlet.java @@ -38,6 +38,7 @@ protected void doPost(final HttpServletRequest req, final HttpServletResponse re throw new IllegalArgumentException("not a test " + test); } final AsyncContext asyncContext = req.startAsync(); + asyncContext.setTimeout(50000); new Thread(()->{ try { HttpServletResponse response = (HttpServletResponse) asyncContext.getResponse(); diff --git a/servlet/src/test/java/io/undertow/servlet/test/response/writer/ResponseWriterOnPostServlet.java b/servlet/src/test/java/io/undertow/servlet/test/response/writer/ResponseWriterOnPostServlet.java index a1b1d6413c..8e7aafdcdc 100644 --- a/servlet/src/test/java/io/undertow/servlet/test/response/writer/ResponseWriterOnPostServlet.java +++ b/servlet/src/test/java/io/undertow/servlet/test/response/writer/ResponseWriterOnPostServlet.java @@ -57,6 +57,8 @@ protected void doPost(final HttpServletRequest req, final HttpServletResponse re } public static Throwable getExceptionIfAny() { - return exception; + Throwable result = exception; + exception = null; + return result; } }