rework the bug/review page substitution

avoid using "$1" and use custom replacer function to make sure the matched
pattern is properly escaped

Author: vladak

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

@@ -59,6 +59,7 @@
 import java.util.function.IntConsumer;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.regex.MatchResult;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.IntStream;
@@ -367,7 +368,7 @@ public static String breadcrumbPath(String urlPrefix, String path) {
      * @param path the full path from which the breadcrumb path is built
      * @param sep separator to use to crack the given path
      *
-     * @return HTML markup fro the breadcrumb or the path itself.
+     * @return HTML markup for the breadcrumb or the path itself.
      * @see #breadcrumbPath(String, String, char, String, boolean, boolean)
      */
     public static String breadcrumbPath(String urlPrefix, String path, char sep) {
@@ -939,16 +940,14 @@ public static String uriEncode(String q) {
      * @param dest a defined target
      * @throws IOException I/O
      */
-    public static void uriEncode(String str, Appendable dest)
-            throws IOException {
+    public static void uriEncode(String str, Appendable dest) throws IOException {
         String uenc = uriEncode(str);
         dest.append(uenc);
     }
 
     /**
-     * Append '&name=value" to the given buffer. If the given
-     * <var>value</var>
-     * is {@code null}, this method does nothing.
+     * Append "&amp;name=value" to the given buffer. If the given <var>value</var> is {@code null},
+     * this method does nothing.
      *
      * @param buf where to append the query string
      * @param key the name of the parameter to add. Append as is!
@@ -957,8 +956,7 @@ public static void uriEncode(String str, Appendable dest)
      * @throws NullPointerException if the given buffer is {@code null}.
      * @see #uriEncode(String)
      */
-    public static void appendQuery(StringBuilder buf, String key,
-            String value) {
+    public static void appendQuery(StringBuilder buf, String key, String value) {
 
         if (value != null) {
             buf.append(AMP).append(key).append('=').append(uriEncode(value));
@@ -1610,25 +1608,32 @@ public static String buildLink(String name, String url, boolean newTab)
         return buildLink(name, attrs);
     }
 
+    private static String buildLinkReplacer(MatchResult result, String text, String url) {
+        final String appendedUrl = url + result.group(1);
+        try {
+            StringBuilder stringBuilder = new StringBuilder();
+            stringBuilder.append(text.substring(result.start(0), result.start(1)));
+            stringBuilder.append(buildLink(appendedUrl, appendedUrl, true));
+            stringBuilder.append(text.substring(result.end(1), result.end(0)));
+            return stringBuilder.toString();
+        } catch (URISyntaxException|MalformedURLException e) {
+            LOGGER.log(Level.WARNING, "The given URL ''{0}'' is not valid", appendedUrl);
+            return result.group(0);
+        }
+    }
+
     /**
      * Replace all occurrences of pattern in the incoming text with the link
-     * named name pointing to an URL. It is possible to use the regexp pattern
+     * named name pointing to a URL. It is possible to use the regexp pattern
      * groups in name and URL when they are specified in the pattern.
      *
-     * @param text text to replace all patterns
+     * @param text    text to replace all patterns
      * @param pattern the pattern to match
-     * @param name link display name
-     * @param url link URL
+     * @param url     link URL
      * @return the text with replaced links
      */
-    public static String linkifyPattern(String text, Pattern pattern, String name, String url) {
-        try {
-            String buildLink = buildLink(name, url, true);
-            return pattern.matcher(text).replaceAll(buildLink);
-        } catch (URISyntaxException | MalformedURLException ex) {
-            LOGGER.log(Level.WARNING, "The given URL ''{0}'' is not valid", url);
-            return text;
-        }
+    public static String linkifyPattern(String text, Pattern pattern, String url) {
+        return pattern.matcher(text).replaceAll(match -> buildLinkReplacer(match, text, url));
     }
 
     /**

opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java

@@ -53,11 +53,7 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.collection.IsIterableContainingInOrder.contains;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertNull;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 import static org.opengrok.indexer.condition.RepositoryInstalled.Type.MERCURIAL;
 
 /**
@@ -490,23 +486,21 @@ void testLinkifyPattern() {
                 + " fugiat nulla pariatur. Excepteur sint "
                 + "occaecat bug6478abc cupidatat non proident, sunt in culpa qui officia "
                 + "deserunt mollit anim id est laborum.";
-        String expected2
-                = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, "
-                + "sed do eiusmod tempor incididunt as per 12345698 ut labore et dolore magna "
-                + "aliqua. "
-                + "<a href=\"http://www.other-example.com?bug=3333\" rel=\"noreferrer\" target=\"_blank\">bug3333fff</a>"
-                + " Ut enim ad minim veniam, quis nostrud exercitation "
-                + "ullamco laboris nisi ut aliquip ex ea introduced in 9791216541 commodo consequat. "
-                + "Duis aute irure dolor in reprehenderit in voluptate velit "
-                + "esse cillum dolore eu fixes 132469187 fugiat nulla pariatur. Excepteur sint "
-                + "occaecat "
-                + "<a href=\"http://www.other-example.com?bug=6478\" rel=\"noreferrer\" target=\"_blank\">bug6478abc</a>"
-                + " cupidatat non proident, sunt in culpa qui officia "
-                + "deserunt mollit anim id est laborum.";
 
-        assertEquals(expected, Util.linkifyPattern(text, Pattern.compile("\\b([0-9]{8,})\\b"), "$1", "http://www.example.com?bug=$1"));
-        assertEquals(expected2, Util.linkifyPattern(text, Pattern.compile("\\b(bug([0-9]{4})\\w{3})\\b"), "$1",
-                "http://www.other-example.com?bug=$2"));
+        assertEquals(expected, Util.linkifyPattern(text, Pattern.compile("\\b([0-9]{8,})\\b"), "http://www.example.com?bug="));
+    }
+
+    /**
+     * Matched pattern should be properly encoded in the resulting HTML.
+     */
+    @Test
+    void testLinkifyPatternEscape() {
+        final String text = "foo bug <123456> bar";
+        final String expected = "foo bug <a href=\"http://www.example.com?bug=%3C123456%3E\" " +
+                "rel=\"noreferrer\" target=\"_blank\">&lt;123456&gt;</a> bar";
+
+        assertEquals(expected,
+                Util.linkifyPattern(text, Pattern.compile("[ \\t]+([0-9<>]{3,})[ \\t]+"), "http://www.example.com?bug="));
     }
 
     @Test
@@ -652,7 +646,9 @@ void testWriteHADNonexistentFile() throws Exception {
     @Test
     void testWriteHAD() throws Exception {
         TestRepository repository = new TestRepository();
-        repository.create(UtilTest.class.getResource("/repositories"));
+        URL repositoryURL = UtilTest.class.getResource("/repositories");
+        assertNotNull(repositoryURL);
+        repository.create(repositoryURL);
 
         RuntimeEnvironment env = RuntimeEnvironment.getInstance();
 

opengrok-web/src/main/webapp/history.jsp

@@ -369,10 +369,10 @@ document.domReady.push(function() {domReadyHistory();});
                 String cout = Util.htmlize(entry.getMessage());
 
                 if (bugPage != null && !bugPage.isEmpty() && bugPattern != null) {
-                    cout = Util.linkifyPattern(cout, bugPattern, "$1", Util.completeUrl(bugPage + "$1", request));
+                    cout = Util.linkifyPattern(cout, bugPattern, Util.completeUrl(bugPage, request));
                 }
                 if (reviewPage != null && !reviewPage.isEmpty() && reviewPattern != null) {
-                    cout = Util.linkifyPattern(cout, reviewPattern, "$1", Util.completeUrl(reviewPage + "$1", request));
+                    cout = Util.linkifyPattern(cout, reviewPattern, Util.completeUrl(reviewPage, request));
                 }
                 
                 boolean showSummary = false;
@@ -382,10 +382,10 @@ document.domReady.push(function() {domReadyHistory();});
                     coutSummary = coutSummary.substring(0, summaryLength - 1);
                     coutSummary = Util.htmlize(coutSummary);
                     if (bugPage != null && !bugPage.isEmpty() && bugPattern != null) {
-                        coutSummary = Util.linkifyPattern(coutSummary, bugPattern, "$1", Util.completeUrl(bugPage + "$1", request));
+                        coutSummary = Util.linkifyPattern(coutSummary, bugPattern, Util.completeUrl(bugPage, request));
                     }
                     if (reviewPage != null && !reviewPage.isEmpty() && reviewPattern != null) {
-                        coutSummary = Util.linkifyPattern(coutSummary, reviewPattern, "$1", Util.completeUrl(reviewPage + "$1", request));
+                        coutSummary = Util.linkifyPattern(coutSummary, reviewPattern, Util.completeUrl(reviewPage, request));
                     }
                 }