deal with symlinks when sorting directory entries (#3629)

Author: Vladimir Kotal

opengrok-web/src/main/java/org/opengrok/web/PageConfig.java

@@ -65,6 +65,7 @@
 import org.apache.lucene.document.DateTools;
 import org.apache.lucene.document.Document;
 import org.jetbrains.annotations.Nullable;
+import org.jetbrains.annotations.VisibleForTesting;
 import org.opengrok.indexer.Info;
 import org.opengrok.indexer.analysis.AbstractAnalyzer;
 import org.opengrok.indexer.analysis.AnalyzerGuru;
@@ -446,28 +447,11 @@ public List<String> getResourceFileList() {
             if (isDir() && getResourcePath().length() > 1) {
                 files = getResourceFile().listFiles();
             }
+
             if (files == null) {
                 dirFileList = Collections.emptyList();
             } else {
-                List<String> listOfFiles;
-                if (env.getListDirsFirst()) {
-                    Arrays.sort(files, (f1, f2) -> {
-                        if (f1.isDirectory() && f2.isDirectory()) {
-                            return f1.getName().compareTo(f2.getName());
-                        } else if (f1.isFile() && f2.isFile()) {
-                            return f1.getName().compareTo(f2.getName());
-                        } else {
-                            if (f1.isFile() && f2.isDirectory()) {
-                                return 1;
-                            } else {
-                                return -1;
-                            }
-                        }
-                    });
-                } else {
-                    Arrays.sort(files, Comparator.comparing(File::getName));
-                }
-                listOfFiles = Arrays.stream(files).map(File::getName).collect(Collectors.toList());
+                List<String> listOfFiles = getSortedFiles(files);
 
                 if (env.hasProjects() && getPath().isEmpty()) {
                     /**
@@ -494,6 +478,27 @@ public List<String> getResourceFileList() {
         return dirFileList;
     }
 
+    private Comparator<File> getFileComparator() {
+        if (getEnv().getListDirsFirst()) {
+            return (f1, f2) -> {
+                if (f1.isDirectory() && !f2.isDirectory()) {
+                    return -1;
+                } else if (!f1.isDirectory() && f2.isDirectory()) {
+                    return 1;
+                } else {
+                    return f1.getName().compareTo(f2.getName());
+                }
+            };
+        } else {
+            return Comparator.comparing(File::getName);
+        }
+    }
+
+    @VisibleForTesting
+    List<String> getSortedFiles(File[] files) {
+        return Arrays.stream(files).sorted(getFileComparator()).map(File::getName).collect(Collectors.toList());
+    }
+
     /**
      * Get the time of last modification of the related file or directory.
      *

opengrok-web/src/test/java/org/opengrok/web/PageConfigTest.java

@@ -27,10 +27,13 @@
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 
 import jakarta.servlet.http.HttpServletRequest;
 import org.junit.jupiter.api.AfterAll;
@@ -219,6 +222,50 @@ public boolean isAllowed(HttpServletRequest request, Project project) {
         env.setProjects(oldProjects);
     }
 
+    @SuppressWarnings("ResultOfMethodCallIgnored")
+    @EnabledOnOs({OS.LINUX, OS.MAC, OS.SOLARIS, OS.AIX, OS.OTHER})
+    @Test
+    void testGetSortedFilesDirsFirst() throws IOException {
+        RuntimeEnvironment env = RuntimeEnvironment.getInstance();
+        env.setListDirsFirst(true);
+        // Cannot spy/mock final class.
+        HttpServletRequest req = createRequest("/source", "/xref", "");
+        PageConfig pageConfig = PageConfig.get(req);
+
+        // Make sure the source root has just directories.
+        File sourceRootFile = new File(repository.getSourceRoot());
+        assertTrue(Arrays.stream(sourceRootFile.listFiles()).filter(File::isFile).
+                collect(Collectors.toSet()).isEmpty());
+
+        // Create regular file under source root.
+        File file = new File(sourceRootFile, "foo.txt");
+        assertTrue(file.createNewFile());
+        assertTrue(file.isFile());
+
+        // Make sure the regular file is last.
+        List<String> entries = pageConfig.getSortedFiles(sourceRootFile.listFiles());
+        assertNotNull(entries);
+        assertFalse(entries.isEmpty());
+        int numEntries = entries.size();
+        assertEquals("foo.txt", entries.get(entries.size() - 1));
+
+        // Create symbolic link to non-existent target.
+        Path link = Path.of(sourceRootFile.getCanonicalPath(), "link");
+        Path target = Paths.get("/nonexistent");
+        Files.createSymbolicLink(link, target);
+
+        // Check the symlink was sorted as file.
+        entries = pageConfig.getSortedFiles(sourceRootFile.listFiles());
+        assertNotNull(entries);
+        assertFalse(entries.isEmpty());
+        assertEquals(numEntries + 1, entries.size());
+        assertEquals("link", entries.get(entries.size() - 1));
+
+        // Cleanup.
+        file.delete();
+        link.toFile().delete();
+    }
+
     @Test
     public void testGetIntParam() {
         String[] attrs = {"a", "b", "c", "d", "e", "f", "g", "h"};