sanitize revision parameter

Author: vladak

opengrok-indexer/src/main/java/org/opengrok/indexer/web/Laundromat.java

@@ -51,6 +51,16 @@ public static String launderInput(String value) {
         return replaceAll(value, ESC_N_R_T_F, "_");
     }
 
+    /**
+     * Sanitize {@code value} where it will be used in subsequent OpenGrok
+     * (non-logging) processing.
+     * @return {@code null} if null or else {@code value} with anything besides
+     * alphanumeric or {@code :} characters removed.
+     */
+    public static String launderRevision(String value) {
+        return replaceAll(value, "[^a-zA-Z0-9:]", "");
+    }
+
     /**
      * Sanitize {@code value} where it will be used in a Lucene query.
      * @return {@code null} if null or else {@code value} with "pattern-breaking

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

@@ -711,13 +711,17 @@ public String getDefineTagsIndex() {
 
     /**
      * Get the revision parameter {@code r} from the request.
+     * Anything besides alphanumeric and {@code :} is removed via {@link Laundromat#launderInput(String)}.
      *
      * @return revision if found, an empty string otherwise.
      */
     public String getRequestedRevision() {
         if (rev == null) {
-            String tmp = Laundromat.launderInput(req.getParameter(QueryParameters.REVISION_PARAM));
-            rev = (tmp != null && !tmp.isEmpty()) ? tmp : "";
+            String tmp = Laundromat.launderRevision(req.getParameter(QueryParameters.REVISION_PARAM));
+            rev = Optional.ofNullable(tmp)
+                    .filter(s -> !s.isEmpty())
+                    .orElse("");
+
         }
         return rev;
     }

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

@@ -26,6 +26,7 @@
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.net.URL;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -34,13 +35,17 @@
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import jakarta.servlet.http.HttpServletRequest;
+import org.apache.commons.lang3.tuple.Pair;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.condition.EnabledOnOs;
 import org.junit.jupiter.api.condition.OS;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.opengrok.indexer.authorization.AuthControlFlag;
 import org.opengrok.indexer.authorization.AuthorizationFramework;
 import org.opengrok.indexer.authorization.AuthorizationPlugin;
@@ -79,7 +84,9 @@ public static void setUpClass() throws Exception {
         env.setHistoryEnabled(true);
 
         repository = new TestRepository();
-        repository.create(PageConfigTest.class.getResource("/repositories"));
+        URL repositoryURL = PageConfigTest.class.getResource("/repositories");
+        assertNotNull(repositoryURL);
+        repository.create(repositoryURL);
         env.setRepositories(repository.getSourceRoot());
     }
 
@@ -425,30 +432,36 @@ public String getPathInfo() {
         assertNull(rev);
     }
 
-    @Test
-    void testGetRequestedRevision() {
-        final String[] revisions = {"6c5588de", "", "6c5588de", "6c5588de", "6c5588de"};
-        for (int i = 0; i < revisions.length; i++) {
-            final int index = i;
-            DummyHttpServletRequest req = new DummyHttpServletRequest() {
-                @Override
-                public String getParameter(String name) {
-                    if (name.equals("r")) {
-                        return revisions[index];
-                    }
-                    return null;
+    private static Stream<Pair<String, String>> getParamsForTestGetRequestedRevision() {
+        return Stream.of(Pair.of("6c5588de", "6c5588de"),
+                Pair.of("10013:cb02e4e3d492", "10013:cb02e4e3d492"),
+                Pair.of("", ""),
+                Pair.of("(foo)\n", "foo"));
+    }
+
+    @MethodSource("getParamsForTestGetRequestedRevision")
+    @ParameterizedTest
+    void testGetRequestedRevision(Pair<String, String> revisionParam) {
+        final String actualRevision = revisionParam.getLeft();
+        final String expectedRevision = revisionParam.getRight();
+        DummyHttpServletRequest req = new DummyHttpServletRequest() {
+            @Override
+            public String getParameter(String name) {
+                if (name.equals("r")) {
+                    return actualRevision;
                 }
-            };
+                return null;
+            }
+        };
 
-            PageConfig cfg = PageConfig.get(req);
-            String rev = cfg.getRequestedRevision();
+        PageConfig cfg = PageConfig.get(req);
+        String rev = cfg.getRequestedRevision();
 
-            assertNotNull(rev);
-            assertEquals(revisions[i], rev);
-            assertFalse(rev.contains("r="));
+        assertNotNull(rev);
+        assertEquals(expectedRevision, rev);
+        assertFalse(rev.contains("r="));
 
-            PageConfig.cleanup(req);
-        }
+        PageConfig.cleanup(req);
     }
 
     @Test