From f47095d56659bca0c98ff4ac3d41a9292b21d662 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 09:20:16 +0200 Subject: [PATCH 01/17] rework the bug/review page substitution avoid using "$1" and use custom replacer function to make sure the matched pattern is properly escaped --- .../java/org/opengrok/indexer/web/Util.java | 45 ++++++++++--------- .../org/opengrok/indexer/web/UtilTest.java | 40 ++++++++--------- opengrok-web/src/main/webapp/history.jsp | 8 ++-- 3 files changed, 47 insertions(+), 46 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index fecbb71c28b..0faa19cd3af 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/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 - * value - * is {@code null}, this method does nothing. + * Append "&name=value" to the given buffer. If the given value 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)); } /** diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java index a5885537845..cb7e9baba1e 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java +++ b/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. " - + "bug3333fff" - + " 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 " - + "bug6478abc" - + " 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 <123456> 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(); diff --git a/opengrok-web/src/main/webapp/history.jsp b/opengrok-web/src/main/webapp/history.jsp index 9f80cff831a..a1c1f04e31d 100644 --- a/opengrok-web/src/main/webapp/history.jsp +++ b/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)); } } From 23b91777906993dffd3c6abcf610bb20383983a6 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 09:34:21 +0200 Subject: [PATCH 02/17] replace URL() constructors with URI() --- .../java/org/opengrok/indexer/web/Util.java | 53 +++++++------------ .../org/opengrok/indexer/web/UtilTest.java | 7 --- 2 files changed, 18 insertions(+), 42 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index 0faa19cd3af..a3e29309365 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java @@ -659,7 +659,7 @@ public static void encode(String s, Appendable dest) throws IOException { // special html characters dest.append("&#").append("" + (int) c).append(";"); } else if (c == ' ') { - // non breaking space + // non-breaking space dest.append(" "); } else if (c == '\t') { dest.append("    "); @@ -672,22 +672,6 @@ public static void encode(String s, Appendable dest) throws IOException { } } - /** - * Encode URL. - * - * @param urlStr string URL - * @return the encoded URL - * @throws URISyntaxException URI syntax - * @throws MalformedURLException URL malformed - */ - public static String encodeURL(String urlStr) throws URISyntaxException, MalformedURLException { - URL url = new URL(urlStr); - URI constructed = new URI(url.getProtocol(), url.getUserInfo(), - url.getHost(), url.getPort(), - url.getPath(), url.getQuery(), url.getRef()); - return constructed.toString(); - } - /** * Write out line information wrt. to the given annotation in the format: * {@code Linenumber Blame Author} incl. appropriate links. @@ -951,8 +935,7 @@ public static void uriEncode(String str, Appendable dest) throws IOException { * * @param buf where to append the query string * @param key the name of the parameter to add. Append as is! - * @param value the value for the given parameter. Gets automatically UTF-8 - * URL encoded. + * @param value the value for the given parameter. Gets automatically UTF-8 URL encoded. * @throws NullPointerException if the given buffer is {@code null}. * @see #uriEncode(String) */ @@ -1454,16 +1437,16 @@ private static String generatePageLink(int page, int offset, int limit, long siz /** - * Check if the string is a HTTP URL. + * Check if the string is an HTTP URL. * * @param string the string to check - * @return true if it is http URL, false otherwise + * @return true if it is HTTP URL, false otherwise */ public static boolean isHttpUri(String string) { URL url; try { - url = new URL(string); - } catch (MalformedURLException ex) { + url = new URI(string).toURL(); + } catch (MalformedURLException | URISyntaxException ex) { return false; } return url.getProtocol().equals("http") || url.getProtocol().equals("https"); @@ -1474,26 +1457,25 @@ public static boolean isHttpUri(String string) { /** * If given path is a URL, return the string representation with the user-info part filtered out. * @param path path to object - * @return either the original string or string representation of URL with the user-info part removed + * @return either the original string (if the URL is not valid) + * or string representation of the URL with the user-info part removed */ public static String redactUrl(String path) { URL url; try { - url = new URL(path); - } catch (MalformedURLException e) { - // not an URL + url = new URI(path).toURL(); + } catch (MalformedURLException | URISyntaxException e) { return path; } if (url.getUserInfo() != null) { - return url.toString().replace(url.getUserInfo(), - REDACTED_USER_INFO); + return url.toString().replace(url.getUserInfo(), REDACTED_USER_INFO); } else { return path; } } /** - * Build a HTML link to the given HTTP URL. If the URL is not an http URL + * Build an HTML link to the given HTTP URL. If the URL is not a HTTP URL * then it is returned as it was received. This has the same effect as * invoking linkify(url, true). * @@ -1507,7 +1489,7 @@ public static String linkify(String url) { } /** - * Build a html link to the given http URL. If the URL is not an http URL + * Build an HTML link to the given http URL. If the URL is not a HTTP URL * then it is returned as it was received. * * @param url the HTTP URL @@ -1554,7 +1536,7 @@ public static String buildLink(String name, Map attrs) buffer.append("=\""); String value = attr.getValue(); if (attr.getKey().equals("href")) { - value = Util.encodeURL(value); + value = new URI(value).toString(); } buffer.append(value); buffer.append("\""); @@ -1609,11 +1591,12 @@ public static String buildLink(String name, String url, boolean newTab) } private static String buildLinkReplacer(MatchResult result, String text, String url) { - final String appendedUrl = url + result.group(1); + final String group1 = result.group(1); + final String appendedUrl = url + uriEncode(group1); try { StringBuilder stringBuilder = new StringBuilder(); stringBuilder.append(text.substring(result.start(0), result.start(1))); - stringBuilder.append(buildLink(appendedUrl, appendedUrl, true)); + stringBuilder.append(buildLink(group1, appendedUrl, true)); stringBuilder.append(text.substring(result.end(1), result.end(0))); return stringBuilder.toString(); } catch (URISyntaxException|MalformedURLException e) { @@ -1670,7 +1653,7 @@ public static String completeUrl(String url, HttpServletRequest req) { } return url; } catch (URISyntaxException ex) { - LOGGER.log(Level.INFO, + LOGGER.log(Level.WARNING, String.format("Unable to convert given URL part '%s' to complete URL", url), ex); return url; diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java index cb7e9baba1e..e86059ce156 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java @@ -409,13 +409,6 @@ void testLinkify() throws URISyntaxException, MalformedURLException { assertEquals("smtp://example.com/OpenGrok/OpenGrok", Util.linkify("smtp://example.com/OpenGrok/OpenGrok")); assertEquals("just some crazy text", Util.linkify("just some crazy text")); - // escaping url - assertTrue(Util.linkify("http://www.example.com/\"quotation\"/else") - .contains("href=\"" + Util.encodeURL("http://www.example.com/\"quotation\"/else") + "\"")); - assertTrue(Util.linkify("https://example.com/><\"") - .contains("href=\"" + Util.encodeURL("https://example.com/><\"") + "\"")); - assertTrue(Util.linkify("http://www.example.com?param=1¶m2=2¶m3=\"quoted>\"") - .contains("href=\"" + Util.encodeURL("http://www.example.com?param=1¶m2=2¶m3=\"quoted>\"") + "\"")); // escaping titles assertTrue(Util.linkify("http://www.example.com/\"quotation\"/else") .contains("title=\"Link to " + Util.encode("http://www.example.com/\"quotation\"/else") + "\"")); From 6079a45e63d35ff4bf7096d9703d030de5c37caa Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 09:46:24 +0200 Subject: [PATCH 03/17] fix tests need to allow for relative identifiers --- .../java/org/opengrok/indexer/web/Util.java | 34 ++++++++----------- .../org/opengrok/indexer/web/UtilTest.java | 10 +----- 2 files changed, 16 insertions(+), 28 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index a3e29309365..208f338d213 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java @@ -46,16 +46,8 @@ import java.nio.charset.StandardCharsets; import java.text.DecimalFormat; import java.text.NumberFormat; -import java.util.Arrays; -import java.util.Collection; -import java.util.HashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Locale; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.Optional; -import java.util.TreeMap; import java.util.function.IntConsumer; import java.util.logging.Level; import java.util.logging.Logger; @@ -1443,13 +1435,17 @@ private static String generatePageLink(int page, int offset, int limit, long siz * @return true if it is HTTP URL, false otherwise */ public static boolean isHttpUri(String string) { - URL url; + URI uri; try { - url = new URI(string).toURL(); - } catch (MalformedURLException | URISyntaxException ex) { + uri = new URI(string); + } catch (URISyntaxException ex) { + return false; + } + String scheme = uri.getScheme(); + if (Objects.isNull(scheme)) { return false; } - return url.getProtocol().equals("http") || url.getProtocol().equals("https"); + return uri.getScheme().equals("http") || uri.getScheme().equals("https"); } protected static final String REDACTED_USER_INFO = "redacted_by_OpenGrok"; @@ -1461,14 +1457,14 @@ public static boolean isHttpUri(String string) { * or string representation of the URL with the user-info part removed */ public static String redactUrl(String path) { - URL url; + URI uri; try { - url = new URI(path).toURL(); - } catch (MalformedURLException | URISyntaxException e) { + uri = new URI(path); + } catch (URISyntaxException e) { return path; } - if (url.getUserInfo() != null) { - return url.toString().replace(url.getUserInfo(), REDACTED_USER_INFO); + if (uri.getUserInfo() != null) { + return uri.toString().replace(uri.getUserInfo(), REDACTED_USER_INFO); } else { return path; } @@ -1536,7 +1532,7 @@ public static String buildLink(String name, Map attrs) buffer.append("=\""); String value = attr.getValue(); if (attr.getKey().equals("href")) { - value = new URI(value).toString(); + value = new URI(value).toURL().toString(); } buffer.append(value); buffer.append("\""); diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java index e86059ce156..14e00a3a13d 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java @@ -408,14 +408,6 @@ void testLinkify() throws URISyntaxException, MalformedURLException { assertEquals("ldap://example.com/OpenGrok/OpenGrok", Util.linkify("ldap://example.com/OpenGrok/OpenGrok")); assertEquals("smtp://example.com/OpenGrok/OpenGrok", Util.linkify("smtp://example.com/OpenGrok/OpenGrok")); assertEquals("just some crazy text", Util.linkify("just some crazy text")); - - // escaping titles - assertTrue(Util.linkify("http://www.example.com/\"quotation\"/else") - .contains("title=\"Link to " + Util.encode("http://www.example.com/\"quotation\"/else") + "\"")); - assertTrue(Util.linkify("https://example.com/><\"") - .contains("title=\"Link to " + Util.encode("https://example.com/><\"") + "\"")); - assertTrue(Util.linkify("http://www.example.com?param=1¶m2=2¶m3=\"quoted>\"") - .contains("title=\"Link to " + Util.encode("http://www.example.com?param=1¶m2=2¶m3=\"quoted>\"") + "\"")); } @Test @@ -445,7 +437,7 @@ void testBuildLink() throws URISyntaxException, MalformedURLException { @Test void testBuildLinkInvalidUrl1() { - assertThrows(MalformedURLException.class, () -> Util.buildLink("link", "www.example.com")); // invalid protocol + assertThrows(IllegalArgumentException.class, () -> Util.buildLink("link", "www.example.com")); // invalid protocol } @Test From 786d8f57cf8a7bfa8b4dcb5bf6a3010034a4b471 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 09:59:01 +0200 Subject: [PATCH 04/17] fix style --- .../main/java/org/opengrok/indexer/web/Util.java | 13 +++++++++++-- .../java/org/opengrok/indexer/web/UtilTest.java | 7 ++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index 208f338d213..23a87a1ab6e 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java @@ -46,8 +46,17 @@ import java.nio.charset.StandardCharsets; import java.text.DecimalFormat; import java.text.NumberFormat; -import java.util.*; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Locale; +import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; +import java.util.Optional; +import java.util.TreeMap; import java.util.function.IntConsumer; import java.util.logging.Level; import java.util.logging.Logger; @@ -1595,7 +1604,7 @@ private static String buildLinkReplacer(MatchResult result, String text, String stringBuilder.append(buildLink(group1, appendedUrl, true)); stringBuilder.append(text.substring(result.end(1), result.end(0))); return stringBuilder.toString(); - } catch (URISyntaxException|MalformedURLException e) { + } catch (URISyntaxException | MalformedURLException e) { LOGGER.log(Level.WARNING, "The given URL ''{0}'' is not valid", appendedUrl); return result.group(0); } diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java index 14e00a3a13d..bc9b31b0a44 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java @@ -53,7 +53,12 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.collection.IsIterableContainingInOrder.contains; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +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.opengrok.indexer.condition.RepositoryInstalled.Type.MERCURIAL; /** From 0a7a75a52ecf4eec8e86d86129b8d79900548bc6 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 10:04:49 +0200 Subject: [PATCH 05/17] add flake8 error suppression --- docker/start.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docker/start.py b/docker/start.py index 7daa890c5e3..bf9c5ce90d8 100755 --- a/docker/start.py +++ b/docker/start.py @@ -115,7 +115,7 @@ def index(): API endpoint for triggering reindex. :return: message describing the outcome """ - global periodic_timer + global periodic_timer # noqa: F824 if periodic_timer: logger = logging.getLogger(__name__) @@ -360,7 +360,7 @@ def indexer_no_projects(logger, uri, config_path, extra_indexer_options): indexer.execute() logger.info("Waiting for reindex to be triggered") - global periodic_timer + global periodic_timer # noqa: F824 periodic_timer.wait_for_event() @@ -420,7 +420,7 @@ def project_syncer(logger, loglevel, uri, config_path, numworkers, env, api_time save_config(logger, uri, config_path, api_timeout) logger.info("Waiting for reindex to be triggered") - global periodic_timer + global periodic_timer # noqa: F824 periodic_timer.wait_for_event() From 9475a77187e8fed556276858528adb8bb062fd6a Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 10:07:06 +0200 Subject: [PATCH 06/17] apply black --- docker/start.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docker/start.py b/docker/start.py index bf9c5ce90d8..48d46e61206 100755 --- a/docker/start.py +++ b/docker/start.py @@ -115,7 +115,7 @@ def index(): API endpoint for triggering reindex. :return: message describing the outcome """ - global periodic_timer # noqa: F824 + global periodic_timer # noqa: F824 if periodic_timer: logger = logging.getLogger(__name__) @@ -360,7 +360,7 @@ def indexer_no_projects(logger, uri, config_path, extra_indexer_options): indexer.execute() logger.info("Waiting for reindex to be triggered") - global periodic_timer # noqa: F824 + global periodic_timer # noqa: F824 periodic_timer.wait_for_event() @@ -420,7 +420,7 @@ def project_syncer(logger, loglevel, uri, config_path, numworkers, env, api_time save_config(logger, uri, config_path, api_timeout) logger.info("Waiting for reindex to be triggered") - global periodic_timer # noqa: F824 + global periodic_timer # noqa: F824 periodic_timer.wait_for_event() From 97b243c93e2e9c59678140ec094237f07476c721 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 10:38:27 +0200 Subject: [PATCH 07/17] weaken visibility of protected symbol in final class --- .../src/main/java/org/opengrok/indexer/web/Util.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index 23a87a1ab6e..bc70a84e9f1 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java @@ -1457,7 +1457,7 @@ public static boolean isHttpUri(String string) { return uri.getScheme().equals("http") || uri.getScheme().equals("https"); } - protected static final String REDACTED_USER_INFO = "redacted_by_OpenGrok"; + static final String REDACTED_USER_INFO = "redacted_by_OpenGrok"; /** * If given path is a URL, return the string representation with the user-info part filtered out. From 2d9dea574460d104327405be85ace287b12a9763 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 10:59:47 +0200 Subject: [PATCH 08/17] improve the test - check multiple matches --- .../src/test/java/org/opengrok/indexer/web/UtilTest.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java index bc9b31b0a44..a5ee0bc02c0 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java @@ -485,12 +485,13 @@ void testLinkifyPattern() { */ @Test void testLinkifyPatternEscape() { - final String text = "foo bug <123456> bar"; + final String text = "foo bug <123456> bar bug 777"; final String expected = "foo bug <123456> bar"; + "rel=\"noreferrer\" target=\"_blank\"><123456> bar " + + "bug 777"; assertEquals(expected, - Util.linkifyPattern(text, Pattern.compile("[ \\t]+([0-9<>]{3,})[ \\t]+"), "http://www.example.com?bug=")); + Util.linkifyPattern(text, Pattern.compile("[ \\t]+([0-9<>]{3,})[ \\t]*"), "http://www.example.com?bug=")); } @Test From 10cd0f0b529a45860f1313d502853bd887221a2e Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 11:00:08 +0200 Subject: [PATCH 09/17] adjust javadocs, rename --- .../java/org/opengrok/indexer/web/Util.java | 43 +++++++++++-------- .../org/opengrok/indexer/web/UtilTest.java | 10 ++--- .../main/webapp/WEB-INF/tags/repository.tag | 2 +- 3 files changed, 31 insertions(+), 24 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index bc70a84e9f1..10b7a5a2235 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java @@ -1438,10 +1438,10 @@ private static String generatePageLink(int page, int offset, int limit, long siz /** - * Check if the string is an HTTP URL. + * Check if the string is an HTTP URI (i.e. allows for relative identifiers). * * @param string the string to check - * @return true if it is HTTP URL, false otherwise + * @return true if it is HTTP URI, false otherwise */ public static boolean isHttpUri(String string) { URI uri; @@ -1460,12 +1460,12 @@ public static boolean isHttpUri(String string) { static final String REDACTED_USER_INFO = "redacted_by_OpenGrok"; /** - * If given path is a URL, return the string representation with the user-info part filtered out. + * If given path is a URI, return the string representation with the user-info part filtered out. * @param path path to object - * @return either the original string (if the URL is not valid) + * @return either the original string (if the URI is not valid) * or string representation of the URL with the user-info part removed */ - public static String redactUrl(String path) { + public static String redactUri(String path) { URI uri; try { uri = new URI(path); @@ -1520,9 +1520,9 @@ public static String linkify(String url, boolean newTab) { } /** - * Build an anchor with given name and a pack of attributes. Automatically - * escapes href attributes and automatically escapes the name into HTML - * entities. + * Build an anchor with given name and a pack of attributes. + * Assumes the href attribute value is URI encoded. + * Automatically escapes the name into HTML entities. * * @param name displayed name of the anchor * @param attrs map of attributes for the html element @@ -1553,9 +1553,9 @@ public static String buildLink(String name, Map attrs) } /** - * Build an anchor with given name and a pack of attributes. Automatically - * escapes href attributes and automatically escapes the name into HTML - * entities. + * Build an anchor with given name and a pack of attributes. + * Assumes the href attribute value is URI encoded. + * Automatically escapes the name into HTML entities. * * @param name displayed name of the anchor * @param url anchor's URL @@ -1564,17 +1564,16 @@ public static String buildLink(String name, Map attrs) * @throws URISyntaxException URI syntax * @throws MalformedURLException bad URL */ - public static String buildLink(String name, String url) - throws URISyntaxException, MalformedURLException { + public static String buildLink(String name, String url) throws URISyntaxException, MalformedURLException { Map attrs = new TreeMap<>(); attrs.put("href", url); return buildLink(name, attrs); } /** - * Build an anchor with given name and a pack of attributes. Automatically - * escapes href attributes and automatically escapes the name into HTML - * entities. + * Build an anchor with given name and a pack of attributes. + * Assumes the href attribute value is URI encoded. + * Automatically escapes the name into HTML entities. * * @param name displayed name of the anchor * @param url anchor's URL @@ -1595,6 +1594,15 @@ public static String buildLink(String name, String url, boolean newTab) return buildLink(name, attrs); } + /** + * Callback function to provide replacement value for pattern match. + * It replaces the first group of the pattern and retains the rest of the pattern. + * + * @param result {@link MatchResult} object representing pattern match + * @param text original text containing the match + * @param url URL to be used for the replacement + * @return replacement for the first group in the pattern + */ private static String buildLinkReplacer(MatchResult result, String text, String url) { final String group1 = result.group(1); final String appendedUrl = url + uriEncode(group1); @@ -1612,8 +1620,7 @@ private static String buildLinkReplacer(MatchResult result, String text, String /** * Replace all occurrences of pattern in the incoming text with the link - * 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. + * named name pointing to a URL. * * @param text text to replace all patterns * @param pattern the pattern to match diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java index a5ee0bc02c0..b8bc06723b1 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java @@ -376,13 +376,13 @@ void testIsUrl() { } @Test - void testRedactUrl() { - assertEquals("/foo/bar", Util.redactUrl("/foo/bar")); - assertEquals("http://foo/bar?r=xxx", Util.redactUrl("http://foo/bar?r=xxx")); + void testRedactUri() { + assertEquals("/foo/bar", Util.redactUri("/foo/bar")); + assertEquals("http://foo/bar?r=xxx", Util.redactUri("http://foo/bar?r=xxx")); assertEquals("http://" + Util.REDACTED_USER_INFO + "@foo/bar?r=xxx", - Util.redactUrl("http://user@foo/bar?r=xxx")); + Util.redactUri("http://user@foo/bar?r=xxx")); assertEquals("http://" + Util.REDACTED_USER_INFO + "@foo/bar?r=xxx", - Util.redactUrl("http://user:pass@foo/bar?r=xxx")); + Util.redactUri("http://user:pass@foo/bar?r=xxx")); } @Test diff --git a/opengrok-web/src/main/webapp/WEB-INF/tags/repository.tag b/opengrok-web/src/main/webapp/WEB-INF/tags/repository.tag index 96099813e1a..f3479e45104 100644 --- a/opengrok-web/src/main/webapp/WEB-INF/tags/repository.tag +++ b/opengrok-web/src/main/webapp/WEB-INF/tags/repository.tag @@ -69,7 +69,7 @@ Portions Copyright (c) 2019, Krystof Tulinger . ${Util.htmlize(ObjectUtils.defaultIfNull(repositoryInfo.type, "N/A"))}: - ${Util.linkify(ObjectUtils.defaultIfNull(Util.redactUrl(repositoryInfo.parent), "N/A"))} + ${Util.linkify(ObjectUtils.defaultIfNull(Util.redactUri(repositoryInfo.parent), "N/A"))} (${Util.htmlize(ObjectUtils.defaultIfNull(repositoryInfo.branch, "N/A"))}) From 01c0709eecef8f064e931d161c784221da9b2a2c Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 11:00:53 +0200 Subject: [PATCH 10/17] remove blank --- .../src/main/java/org/opengrok/indexer/web/Util.java | 1 - 1 file changed, 1 deletion(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index 10b7a5a2235..d11a9595fcd 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java @@ -1436,7 +1436,6 @@ private static String generatePageLink(int page, int offset, int limit, long siz } - /** * Check if the string is an HTTP URI (i.e. allows for relative identifiers). * From a64ff6e2dd1427097efbd67550b20f76d6dd174f Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 11:01:51 +0200 Subject: [PATCH 11/17] improve javadoc --- .../src/main/java/org/opengrok/indexer/web/Util.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index d11a9595fcd..35977711139 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java @@ -1437,10 +1437,10 @@ private static String generatePageLink(int page, int offset, int limit, long siz } /** - * Check if the string is an HTTP URI (i.e. allows for relative identifiers). + * Check if the string is an HTTP(S) URI (i.e. allows for relative identifiers). * * @param string the string to check - * @return true if it is HTTP URI, false otherwise + * @return true if it is HTTP(S) URI, false otherwise */ public static boolean isHttpUri(String string) { URI uri; From 94877d7eda387da26f92b22cfb9c046ef10d0732 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 11:04:15 +0200 Subject: [PATCH 12/17] improve wording, wrap line --- .../src/main/java/org/opengrok/indexer/web/Util.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index 35977711139..b5e241d7377 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java @@ -1479,7 +1479,7 @@ public static String redactUri(String path) { } /** - * Build an HTML link to the given HTTP URL. If the URL is not a HTTP URL + * Build an HTML link to the given HTTP URL. If the URL is not an HTTP URL * then it is returned as it was received. This has the same effect as * invoking linkify(url, true). * @@ -1493,7 +1493,7 @@ public static String linkify(String url) { } /** - * Build an HTML link to the given http URL. If the URL is not a HTTP URL + * Build an HTML link to the given http URL. If the URL is not an HTTP URL * then it is returned as it was received. * * @param url the HTTP URL @@ -1631,8 +1631,7 @@ public static String linkifyPattern(String text, Pattern pattern, String url) { } /** - * Try to complete the given URL part into full URL with server name, port, - * scheme, ... + * Try to complete the given URL part into full URL with server name, port, scheme, ... *
*
for request http://localhost:8080/source/xref/xxx and part * /cgi-bin/user=
@@ -1654,7 +1653,8 @@ public static String completeUrl(String url, HttpServletRequest req) { try { if (!isHttpUri(url)) { if (url.startsWith("/")) { - return new URI(req.getScheme(), null, req.getServerName(), req.getServerPort(), url, null, null).toString(); + return new URI(req.getScheme(), null, req.getServerName(), req.getServerPort(), url, + null, null).toString(); } var prepUrl = req.getRequestURL(); if (!url.isEmpty()) { From 94285aba6cced59aebe864fec100c5d1f0bf27e0 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 11:28:15 +0200 Subject: [PATCH 13/17] use upper caps --- .../src/main/java/org/opengrok/indexer/web/Util.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index b5e241d7377..22daba6b2b2 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java @@ -1493,7 +1493,7 @@ public static String linkify(String url) { } /** - * Build an HTML link to the given http URL. If the URL is not an HTTP URL + * Build an HTML link to the given HTTP URL. If the URL is not an HTTP URL * then it is returned as it was received. * * @param url the HTTP URL From e0b2576863ac442bc63e778543a4aa39836c27bf Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 11:30:22 +0200 Subject: [PATCH 14/17] fix javadoc --- .../src/main/java/org/opengrok/indexer/web/Util.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index 22daba6b2b2..eca0e70662b 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java @@ -1462,7 +1462,7 @@ public static boolean isHttpUri(String string) { * If given path is a URI, return the string representation with the user-info part filtered out. * @param path path to object * @return either the original string (if the URI is not valid) - * or string representation of the URL with the user-info part removed + * or string representation of the URI with the user-info part removed */ public static String redactUri(String path) { URI uri; From 96cc02bd3e5d3714bcfd4b3534f3294211c250cd Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 11:31:51 +0200 Subject: [PATCH 15/17] check the bounds --- .../src/main/java/org/opengrok/indexer/web/Util.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index eca0e70662b..8872266d670 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java @@ -1607,9 +1607,13 @@ private static String buildLinkReplacer(MatchResult result, String text, String final String appendedUrl = url + uriEncode(group1); try { StringBuilder stringBuilder = new StringBuilder(); - stringBuilder.append(text.substring(result.start(0), result.start(1))); + if (result.start(0) < result.start(1)) { + stringBuilder.append(text.substring(result.start(0), result.start(1))); + } stringBuilder.append(buildLink(group1, appendedUrl, true)); - stringBuilder.append(text.substring(result.end(1), result.end(0))); + if (result.end(1) < result.end(0)) { + 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); From b9e64347ec1ab0cc0eeb8f2345576a7ce7743ba0 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 11:56:28 +0200 Subject: [PATCH 16/17] add some robustness --- .../org/opengrok/indexer/configuration/Configuration.java | 8 ++++++++ .../src/main/java/org/opengrok/indexer/web/Util.java | 3 +++ .../src/test/java/org/opengrok/indexer/web/UtilTest.java | 8 ++++++++ 3 files changed, 19 insertions(+) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Configuration.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Configuration.java index 8dceda0346e..b3e8076a5ef 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Configuration.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Configuration.java @@ -59,6 +59,7 @@ import org.opengrok.indexer.authorization.AuthorizationStack; import org.opengrok.indexer.history.RepositoryInfo; import org.opengrok.indexer.logger.LoggerFactory; +import org.opengrok.indexer.web.Util; import static org.opengrok.indexer.configuration.PatternUtil.compilePattern; @@ -1642,5 +1643,12 @@ public void checkConfiguration() throws ConfigurationException { LOGGER.log(Level.INFO, "History based reindex is on, however history cache is off. " + "History cache has to be enabled for history based reindex."); } + + if (!Objects.isNull(getBugPage()) && !Util.isHttpUri(getBugPage())) { + throw new ConfigurationException("Bug page must be valid HTTP(S) URI"); + } + if (!Objects.isNull(getReviewPage()) && !Util.isHttpUri(getReviewPage())) { + throw new ConfigurationException("Review page must be valid HTTP(S) URI"); + } } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index 8872266d670..26318d75193 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java @@ -1603,6 +1603,9 @@ public static String buildLink(String name, String url, boolean newTab) * @return replacement for the first group in the pattern */ private static String buildLinkReplacer(MatchResult result, String text, String url) { + if (result.groupCount() < 1) { + return result.group(0); + } final String group1 = result.group(1); final String appendedUrl = url + uriEncode(group1); try { diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java index b8bc06723b1..44eb30dca4c 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java @@ -494,6 +494,14 @@ void testLinkifyPatternEscape() { Util.linkifyPattern(text, Pattern.compile("[ \\t]+([0-9<>]{3,})[ \\t]*"), "http://www.example.com?bug=")); } + @Test + void testLinkifyPatternNoGroup() { + final String text = "foo bug <123456> bar bug 777"; + + assertEquals(text, + Util.linkifyPattern(text, Pattern.compile("[0-9]{3,}"), "http://www.example.com?bug=")); + } + @Test void testCompleteUrl() { HttpServletRequest req = new DummyHttpServletRequest() { From 7b24de1f626af7f217ef7d31c319078b339c1355 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 1 Apr 2025 15:45:07 +0200 Subject: [PATCH 17/17] add tests for checkConfiguration() --- .../configuration/ConfigurationTest.java | 38 ++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/configuration/ConfigurationTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/configuration/ConfigurationTest.java index 520459221e2..27136d307f5 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/configuration/ConfigurationTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/configuration/ConfigurationTest.java @@ -18,7 +18,7 @@ */ /* - * Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 2025, Oracle and/or its affiliates. All rights reserved. * Portions Copyright (c) 2020, Chris Fraire . */ package org.opengrok.indexer.configuration; @@ -33,17 +33,23 @@ import java.io.InputStreamReader; import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.LinkedList; import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.xml.XMLConstants; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import org.opengrok.indexer.util.ClassUtil; +import org.opengrok.indexer.util.IOUtils; import org.xml.sax.Attributes; import org.xml.sax.ext.DefaultHandler2; @@ -234,4 +240,34 @@ void testLoadingValidConfiguration() throws IOException { } } + private static Stream getArgsForTestCheckConfigurationBugPage() { + return Stream.of( + Arguments.of(true, true), + Arguments.of(true, false), + Arguments.of(false, true), + Arguments.of(false, false) + ); + } + + @ParameterizedTest + @MethodSource("getArgsForTestCheckConfigurationBugPage") + void testCheckConfigurationBugPage(boolean valid, boolean bugPage) throws IOException { + Configuration cfg = new Configuration(); + Path tmpSourceRoot = Files.createTempDirectory("sourceRoot"); + cfg.setSourceRoot(tmpSourceRoot.toString()); + Path tmpDataRoot = Files.createTempDirectory("dataRoot"); + cfg.setDataRoot(tmpDataRoot.toString()); + if (bugPage) { + cfg.setBugPage("http://example.org/bug?" + (valid ? "" : "\"")); + } else { + cfg.setReviewPage("http://example.org/review?" + (valid ? "" : "\"")); + } + if (!valid) { + assertThrows(Configuration.ConfigurationException.class, cfg::checkConfiguration); + } else { + assertDoesNotThrow(cfg::checkConfiguration); + } + IOUtils.removeRecursive(tmpSourceRoot); + IOUtils.removeRecursive(tmpDataRoot); + } }