Merge pull request #1610 from fl4via/2.2.x_backport-bug-fixes

[UNDERTOW-2291][UNDERTOW-2332][UNDERTOW-2400][UNDERTOW-2385] Backport bug fixes to 2.2.x branch

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);
 }

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<Thread, ThreadLocalData> localsByThread = new HashMap<>();
+        Map<Thread, ThreadLocalData> localsByThread = Collections.synchronizedMap(new WeakHashMap<>());
 
         ThreadLocalData get() {
             return localsByThread.get(Thread.currentThread());

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() {

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();
+            }
         }
     }
 }

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;

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

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<String, Object> 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<ResourceChangeEvent> 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);

pom.xml

@@ -106,6 +106,9 @@
         <version.bundle.plugin>5.1.1</version.bundle.plugin>
 
         <version.jmh>1.21</version.jmh>
+        <maven.javadoc.plugin.quiet>true</maven.javadoc.plugin.quiet>
+        <maven.compiler.showDeprecation>false</maven.compiler.showDeprecation>
+        <maven.compiler.showWarnings>false</maven.compiler.showWarnings>
     </properties>
 
     <modules>
@@ -135,6 +138,7 @@
                         following setting.
                     -->
                     <doclint>none</doclint>
+                    <quiet>${maven.javadoc.plugin.quiet}</quiet>
                     <!-- @implNote is a non-standard tag and must be declared or the build will fail -->
                     <tags>
                         <tag>
@@ -173,6 +177,20 @@
         </plugins>
         <pluginManagement>
             <plugins>
+                <!-- Compiler -->
+                <plugin>
+                    <groupId>org.apache.maven.plugins</groupId>
+                    <artifactId>maven-compiler-plugin</artifactId>
+                    <version>${version.compiler.plugin}</version>
+                    <configuration>
+                        <!-- fork is needed so compiler args can be used -->
+                        <fork>true</fork>
+                        <failOnError>false</failOnError>
+                        <!-- need to specify directly, for some reason properties are ignored -->
+                        <showDeprecation>${maven.compiler.showDeprecation}</showDeprecation>
+                        <showWarnings>${maven.compiler.showWarnings}</showWarnings>
+                    </configuration>
+                </plugin>
 
                 <!-- Checkstyle -->
                 <plugin>
@@ -674,6 +692,28 @@
                 <module>karaf</module>
             </modules>
         </profile>
+        <profile>
+            <id>jdk18</id>
+            <activation>
+                <jdk>[18,)</jdk>
+            </activation>
+            <properties>
+                <modular.jdk.props>-Djava.security.manager=allow</modular.jdk.props>
+            </properties>
+        </profile>
+		<profile>
+			<id>verbose</id>
+			<activation>
+				<property>
+					<name>findbugs</name>
+				</property>
+			</activation>
+			<properties>
+				<maven.javadoc.plugin.quiet>false</maven.javadoc.plugin.quiet>
+				<maven.compiler.showDeprecation>true</maven.compiler.showDeprecation>
+				<maven.compiler.showWarnings>false</maven.compiler.showWarnings>
+			</properties>
+		</profile>
     </profiles>
 
 </project>

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();

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;
     }
 }