Merge branch 'jetty-9.4.x' of github.com:eclipse/jetty.project into jetty-9.4.x

jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java

@@ -35,6 +35,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.Stream;
 
 import org.eclipse.jetty.util.ClassLoadingObjectInputStream;
 import org.eclipse.jetty.util.MultiException;
@@ -153,7 +154,7 @@ public class FileSessionDataStore extends AbstractSessionDataStore
     public Set<String> doGetExpired(final Set<String> candidates)
     {
         final long now = System.currentTimeMillis();
-        HashSet<String> expired = new HashSet<String>();
+        HashSet<String> expired = new HashSet<>();
 
         //iterate over the files and work out which have expired
         for (String filename : _sessionFileMap.values())
@@ -206,23 +207,12 @@ public class FileSessionDataStore extends AbstractSessionDataStore
         long now = System.currentTimeMillis();
         if (LOG.isDebugEnabled())
             LOG.debug("Sweeping {} for old session files", _storeDir);
-        try
+        try (Stream<Path> stream = Files.walk(_storeDir.toPath(), 1, FileVisitOption.FOLLOW_LINKS))
         {
-            Files.walk(_storeDir.toPath(), 1, FileVisitOption.FOLLOW_LINKS)
+            stream
                 .filter(p -> !Files.isDirectory(p)).filter(p -> !isOurContextSessionFilename(p.getFileName().toString()))
                 .filter(p -> isSessionFilename(p.getFileName().toString()))
-                .forEach(p ->
+                .forEach(p -> sweepFile(now, p));
-                {
-
-                    try
-                    {
-                        sweepFile(now, p);
-                    }
-                    catch (Exception e)
-                    {
-                        LOG.warn(e);
-                    }
-                });
         }
         catch (Exception e)
         {
@@ -237,29 +227,35 @@ public class FileSessionDataStore extends AbstractSessionDataStore
      *
      * @param now the time now in msec
      * @param p the file to check
-     * @throws Exception indicating error in sweep
      */
     public void sweepFile(long now, Path p)
-        throws Exception
     {
-        if (p == null)
+        if (p != null)
-            return;
-
-        try
         {
-            long expiry = getExpiryFromFilename(p.getFileName().toString());
+            try
-            //files with 0 expiry never expire
-            if (expiry > 0 && ((now - expiry) >= (5 * TimeUnit.SECONDS.toMillis(_gracePeriodSec))))
             {
-                Files.deleteIfExists(p);
+                long expiry = getExpiryFromFilename(p.getFileName().toString());
-                if (LOG.isDebugEnabled())
+                //files with 0 expiry never expire
-                    LOG.debug("Sweep deleted {}", p.getFileName());
+                if (expiry > 0 && ((now - expiry) >= (5 * TimeUnit.SECONDS.toMillis(_gracePeriodSec))))
+                {
+                    try
+                    {
+                        if (!Files.deleteIfExists(p))
+                            LOG.warn("Could not delete {}", p.getFileName());
+                        else if (LOG.isDebugEnabled())
+                            LOG.debug("Deleted {}", p.getFileName());
+                    }
+                    catch (IOException e)
+                    {
+                        LOG.warn("Could not delete {}", p.getFileName(), e);
+                    }
+                }
+            }
+            catch (NumberFormatException e)
+            {
+                LOG.warn("Not valid session filename {}", p.getFileName());
+                LOG.warn(e);
             }
-        }
-        catch (NumberFormatException e)
-        {
-            LOG.warn("Not valid session filename {}", p.getFileName());
-            LOG.warn(e);
         }
     }
 
@@ -311,7 +307,7 @@ public class FileSessionDataStore extends AbstractSessionDataStore
     @Override
     public void doStore(String id, SessionData data, long lastSaveTime) throws Exception
     {
-        File file = null;
+        File file;
         if (_storeDir != null)
         {
             delete(id);
@@ -328,8 +324,9 @@ public class FileSessionDataStore extends AbstractSessionDataStore
             }
             catch (Exception e)
             {
-                if (file != null)
+                // No point keeping the file if we didn't save the whole session
-                    file.delete(); // No point keeping the file if we didn't save the whole session
+                if (!file.delete())
+                    e.addSuppressed(new IOException("Could not delete " + file));
                 throw new UnwriteableSessionDataException(id, _context, e);
             }
         }
@@ -353,7 +350,10 @@ public class FileSessionDataStore extends AbstractSessionDataStore
             throw new IllegalStateException("No file store specified");
 
         if (!_storeDir.exists())
-            _storeDir.mkdirs();
+        {
+            if (!_storeDir.mkdirs())
+                throw new IllegalStateException("Could not create " + _storeDir);
+        }
         else
         {
             if (!(_storeDir.isDirectory() && _storeDir.canWrite() && _storeDir.canRead()))
@@ -365,70 +365,67 @@ public class FileSessionDataStore extends AbstractSessionDataStore
             MultiException me = new MultiException();
             long now = System.currentTimeMillis();
 
-            Files.walk(_storeDir.toPath(), 1, FileVisitOption.FOLLOW_LINKS)
+            // Build session file map by walking directory
-                .filter(p -> !Files.isDirectory(p)).filter(p -> isSessionFilename(p.getFileName().toString()))
+            try (Stream<Path> stream = Files.walk(_storeDir.toPath(), 1, FileVisitOption.FOLLOW_LINKS))
-                .forEach(p ->
+            {
-                {
+                stream
-                    //first get rid of all ancient files, regardless of which
+                    .filter(p -> !Files.isDirectory(p))
-                    //context they are for
+                    .filter(p -> isSessionFilename(p.getFileName().toString()))
-                    try
+                    .forEach(p ->
                     {
+                        // first get rid of all ancient files
                         sweepFile(now, p);
-                    }
-                    catch (Exception x)
-                    {
-                        me.add(x);
-                    }
 
-                    String filename = p.getFileName().toString();
+                        String filename = p.getFileName().toString();
-                    String context = getContextFromFilename(filename);
+                        String context = getContextFromFilename(filename);
-                    //now process it if it wasn't deleted, and it is for our context
+                        //now process it if it wasn't deleted, and it is for our context
-                    if (Files.exists(p) && _contextString.equals(context))
+                        if (Files.exists(p) && _contextString.equals(context))
-                    {
-                        //the session is for our context, populate the map with it
-                        String sessionIdWithContext = getIdWithContextFromFilename(filename);
-                        if (sessionIdWithContext != null)
                         {
-                            //handle multiple session files existing for the same session: remove all
+                            //the session is for our context, populate the map with it
-                            //but the file with the most recent expiry time
+                            String sessionIdWithContext = getIdWithContextFromFilename(filename);
-                            String existing = _sessionFileMap.putIfAbsent(sessionIdWithContext, filename);
+                            if (sessionIdWithContext != null)
-                            if (existing != null)
                             {
-                                //if there was a prior filename, work out which has the most
+                                //handle multiple session files existing for the same session: remove all
-                                //recent modify time
+                                //but the file with the most recent expiry time
-                                try
+                                String existing = _sessionFileMap.putIfAbsent(sessionIdWithContext, filename);
+                                if (existing != null)
                                 {
-                                    long existingExpiry = getExpiryFromFilename(existing);
+                                    //if there was a prior filename, work out which has the most
-                                    long thisExpiry = getExpiryFromFilename(filename);
+                                    //recent modify time
+                                    try
+                                    {
+                                        long existingExpiry = getExpiryFromFilename(existing);
+                                        long thisExpiry = getExpiryFromFilename(filename);
 
-                                    if (thisExpiry > existingExpiry)
+                                        if (thisExpiry > existingExpiry)
-                                    {
+                                        {
-                                        //replace with more recent file
+                                            //replace with more recent file
-                                        Path existingPath = _storeDir.toPath().resolve(existing);
+                                            Path existingPath = _storeDir.toPath().resolve(existing);
-                                        //update the file we're keeping
+                                            //update the file we're keeping
-                                        _sessionFileMap.put(sessionIdWithContext, filename);
+                                            _sessionFileMap.put(sessionIdWithContext, filename);
-                                        //delete the old file
+                                            //delete the old file
-                                        Files.delete(existingPath);
+                                            Files.delete(existingPath);
-                                        if (LOG.isDebugEnabled())
+                                            if (LOG.isDebugEnabled())
-                                            LOG.debug("Replaced {} with {}", existing, filename);
+                                                LOG.debug("Replaced {} with {}", existing, filename);
+                                        }
+                                        else
+                                        {
+                                            //we found an older file, delete it
+                                            Files.delete(p);
+                                            if (LOG.isDebugEnabled())
+                                                LOG.debug("Deleted expired session file {}", filename);
+                                        }
                                     }
-                                    else
+                                    catch (IOException e)
                                     {
-                                        //we found an older file, delete it
+                                        me.add(e);
-                                        Files.delete(p);
-                                        if (LOG.isDebugEnabled())
-                                            LOG.debug("Deleted expired session file {}", filename);
                                     }
                                 }
-                                catch (IOException e)
-                                {
-                                    me.add(e);
-                                }
                             }
                         }
-                    }
+                    });
-                });
+                me.ifExceptionThrow();
-            me.ifExceptionThrow();
+            }
         }
     }
 
@@ -511,11 +508,11 @@ public class FileSessionDataStore extends AbstractSessionDataStore
 
     protected long getExpiryFromFilename(String filename)
     {
-        if (StringUtil.isBlank(filename) || filename.indexOf("_") < 0)
+        if (StringUtil.isBlank(filename) || !filename.contains("_"))
             throw new IllegalStateException("Invalid or missing filename");
 
         String s = filename.substring(0, filename.indexOf('_'));
-        return (s == null ? 0 : Long.parseLong(s));
+        return Long.parseLong(s);
     }
 
     protected String getContextFromFilename(String filename)
@@ -592,11 +589,11 @@ public class FileSessionDataStore extends AbstractSessionDataStore
     protected SessionData load(InputStream is, String expectedId)
         throws Exception
     {
-        String id = null; //the actual id from inside the file
+        String id; //the actual id from inside the file
 
         try
         {
-            SessionData data = null;
+            SessionData data;
             DataInputStream di = new DataInputStream(is);
 
             id = di.readUTF();

jetty-server/src/test/java/org/eclipse/jetty/server/CustomResourcesMonitorTest.java

@@ -146,9 +146,8 @@ public class CustomResourcesMonitorTest
         @Override
         public boolean isLowOnResources()
         {
-            try
+            try (Stream<Path> paths = Files.list(_pathToMonitor))
             {
-                Stream<Path> paths = Files.list(_pathToMonitor);
                 List<Path> content = paths.collect(Collectors.toList());
                 if (!content.isEmpty())
                 {

jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java

@@ -36,6 +36,7 @@ import java.util.List;
 import java.util.Locale;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Stream;
 import javax.servlet.DispatcherType;
 import javax.servlet.MultipartConfigElement;
 import javax.servlet.ServletException;
@@ -1837,9 +1838,9 @@ public class RequestTest
 
     private static long getFileCount(Path path)
     {
-        try
+        try (Stream<Path> s = Files.list(path))
         {
-            return Files.list(path).count();
+            return s.count();
         }
         catch (IOException e)
         {

jetty-util/src/test/java/org/eclipse/jetty/util/RolloverFileOutputStreamTest.java

@@ -31,7 +31,7 @@ import java.time.temporal.TemporalAccessor;
 import java.util.Arrays;
 import java.util.TimeZone;
 import java.util.concurrent.TimeUnit;
-import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import org.eclipse.jetty.toolchain.test.FS;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
@@ -379,6 +379,9 @@ public class RolloverFileOutputStreamTest
 
     private String[] ls(Path path) throws IOException
     {
-        return Files.list(path).map(p -> p.getFileName().toString()).collect(Collectors.toList()).toArray(new String[0]);
+        try (Stream<Path> s = Files.list(path))
+        {
+            return s.map(p -> p.getFileName().toString()).toArray(String[]::new);
+        }
     }
 }

jetty-util/src/test/java/org/eclipse/jetty/util/resource/JarResourceTest.java

@@ -30,6 +30,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import java.util.zip.ZipFile;
 
 import org.eclipse.jetty.toolchain.test.FS;
@@ -264,7 +265,10 @@ public class JarResourceTest
 
     private List<Path> listFiles(Path dir) throws IOException
     {
-        return Files.list(dir).collect(Collectors.toList());
+        try (Stream<Path> s = Files.list(dir))
+        {
+            return s.collect(Collectors.toList());
+        }
     }
 
     private List<Path> listFiles(Path dir, DirectoryStream.Filter<? super Path> filter) throws IOException

jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java

@@ -365,10 +365,14 @@ public class WebAppContextTest
         WebAppClassLoader webAppClassLoader = (WebAppClassLoader)contextClassLoader;
         Path extLibsDir = MavenTestingUtils.getTestResourcePathDir("ext");
         extLibsDir = extLibsDir.toAbsolutePath();
-        List<Path> expectedPaths = Files.list(extLibsDir)
+        List<Path> expectedPaths;
-            .filter(Files::isRegularFile)
+        try (Stream<Path> s = Files.list(extLibsDir))
-            .filter((path) -> path.toString().endsWith(".jar"))
+        {
-            .collect(Collectors.toList());
+            expectedPaths = s
+                .filter(Files::isRegularFile)
+                .filter((path) -> path.toString().endsWith(".jar"))
+                .collect(Collectors.toList());
+        }
         List<Path> actualPaths = new ArrayList<>();
         for (URL url : webAppClassLoader.getURLs())
         {

tests/test-sessions/test-file-sessions/src/test/java/org/eclipse/jetty/server/session/FileTestHelper.java

@@ -27,10 +27,10 @@ import java.io.ObjectOutputStream;
 import java.io.OutputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import org.eclipse.jetty.toolchain.test.FS;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
@@ -49,23 +49,28 @@ public class FileTestHelper
 
     public static File getFile(WorkDir workDir, String sessionId) throws IOException
     {
-        Optional<Path> sessionPath = Files.list(workDir.getPath())
+        try (Stream<Path> s = Files.list(workDir.getPath()))
-            .filter((path) -> path.getFileName().toString().contains(sessionId))
+        {
-            .findFirst();
+            return s
-        if (sessionPath.isPresent())
+                .filter((path) -> path.getFileName().toString().contains(sessionId))
-            return sessionPath.get().toFile();
+                .findFirst()
-        return null;
+                .map(Path::toFile)
+                .orElse(null);
+        }
     }
 
     public static void assertSessionExists(WorkDir workDir, String sessionId, boolean exists) throws IOException
     {
-        Optional<Path> sessionPath = Files.list(workDir.getPath())
+        try (Stream<Path> s = Files.list(workDir.getPath()))
-            .filter((path) -> path.getFileName().toString().contains(sessionId))
+        {
-            .findFirst();
+            Optional<Path> sessionPath = s
-        if (exists)
+                .filter((path) -> path.getFileName().toString().contains(sessionId))
-            assertTrue(sessionPath.isPresent());
+                .findFirst();
-        else
+            if (exists)
-            assertFalse(sessionPath.isPresent());
+                assertTrue(sessionPath.isPresent());
+            else
+                assertFalse(sessionPath.isPresent());
+        }
     }
 
     public static void assertFileExists(WorkDir workDir, String filename, boolean exists)
@@ -170,14 +175,11 @@ public class FileTestHelper
     public static void deleteFile(WorkDir workDir, String sessionId) throws IOException
     {
         // Collect
-        List<Path> matches = Files.list(workDir.getPath())
+        try (Stream<Path> s = Files.list(workDir.getPath()))
-            .filter((path) -> path.getFileName().toString().contains(sessionId))
-            .collect(Collectors.toList());
-
-        // Delete outside of lambda
-        for (Path path : matches)
         {
-            FS.deleteFile(path);
+            s.filter((path) -> path.getFileName().toString().contains(sessionId))
+                .collect(Collectors.toList()) // Delete outside of list stream
+                .forEach(FS::deleteFile);
         }
     }