diff --git a/pom.xml b/pom.xml index c5799993..6c4bbc87 100644 --- a/pom.xml +++ b/pom.xml @@ -59,6 +59,11 @@ + + com.googlecode.owasp-java-html-sanitizer + owasp-java-html-sanitizer + 20220608.1 + com.pivovarit parallel-collectors diff --git a/src/main/java/hudson/tasks/test/TestResult.java b/src/main/java/hudson/tasks/test/TestResult.java index b3850cd7..136dc277 100644 --- a/src/main/java/hudson/tasks/test/TestResult.java +++ b/src/main/java/hudson/tasks/test/TestResult.java @@ -32,6 +32,10 @@ import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.owasp.html.HtmlPolicyBuilder; +import org.owasp.html.PolicyFactory; /** * A class that represents a general concept of a test result, without any @@ -43,6 +47,16 @@ public abstract class TestResult extends TestObject { private static final Logger LOGGER = Logger.getLogger(TestResult.class.getName()); + private static final PolicyFactory POLICY_DEFINITION = new HtmlPolicyBuilder() + .allowElements("a") + .allowUrlProtocols("https") + .allowUrlProtocols("http") + .allowAttributes("href") + .onElements("a") + .toFactory(); + + private static final Pattern LINK_REGEX_PATTERN = Pattern.compile("\\b(https?://[^\\s)<>\"]+)"); + /** * If the concept of a parent action is important to a subclass, then it should * provide a non-noop implementation of this method. @@ -284,16 +298,76 @@ public String toPrettyString() { } /** - * Annotate some text -- what does this do? - * @param text Text to use to annotate the actions. + * Escapes "&" and "<" characters in the provided text, with the goal of preventing any HTML tags present + * in the text from being rendered / interpreted as HTML. * + * @param text The text to sanitize + * @return The sanitized text + */ + private static String naiveHtmlSanitize(String text) { + return text.replace("&", "&").replace("<", "<"); + } + + private static String escapeHtmlAndMakeLinksClickable(String text) { + if (text == null) { + return null; + } + + StringBuilder annotatedTxtBuilder = new StringBuilder(); + + Matcher linkMatcher = LINK_REGEX_PATTERN.matcher(text); + + // Goal: Find all the things that look like URLs in the text, and convert them to clickable + // tags so they render as clickable links when viewing test output. + + int lastMatchEndIdxExclusive = 0; + while (linkMatcher.find()) { + // Group 1 in the regex is just the URL + String linkUrl = linkMatcher.group(1); + + // Sanitize the final HTML tag we produce to make sure there's nothing malicious in there + String sanitizedLinkHtmlTag = + POLICY_DEFINITION.sanitize(String.format("%s", linkUrl, linkUrl)); + + annotatedTxtBuilder + // Append all the chars in-between the last link and this current one, and run that substring + // through naive HTML sanitization since we don't want anything other than the tags we're + // spawning to actually render as HTML. + .append(naiveHtmlSanitize(text.substring(lastMatchEndIdxExclusive, linkMatcher.start()))) + // Append our clickable tag + .append(sanitizedLinkHtmlTag); + + lastMatchEndIdxExclusive = linkMatcher.end(); + } + + // Finish up by sanitizing + appending all the remaining text after the last URL we found + annotatedTxtBuilder.append(naiveHtmlSanitize(text.substring(lastMatchEndIdxExclusive))); + + return annotatedTxtBuilder.toString(); + } + + /** + * All JUnit test output (error message, stack trace, std out, std err) shown on the single test case result page + * is passed through this method when the page is rendered, which processes / sanitizes the text to make it + * suitable for HTML rendering. Attempts to auto-detect URLs in the test output and convert them to clickable + * links for convenience. All other HTML will be escaped. + *

+ * Additionally, passes the test output through all the TestActions associated with this result, giving them a + * chance to apply their own custom rendering / transformations. + *

+ * Note: The test output shown when expanding cases on the full "testReport" page is *not* passed through this + * method, and instead relies on the Jelly "escape-by-default" flag to escape test output for HTML rendering, + * which is why links are not clickable in that context. + * + * @param text Text to use to annotate the actions. * @return the provided text HTML-escaped. */ public String annotate(String text) { if (text == null) { return null; } - text = text.replace("&", "&").replace("<", "<"); + + text = escapeHtmlAndMakeLinksClickable(text); for (TestAction action : getTestActions()) { text = action.annotate(text); diff --git a/src/test/java/hudson/tasks/junit/CaseResultTest.java b/src/test/java/hudson/tasks/junit/CaseResultTest.java index ccd985e1..9783b94e 100644 --- a/src/test/java/hudson/tasks/junit/CaseResultTest.java +++ b/src/test/java/hudson/tasks/junit/CaseResultTest.java @@ -94,19 +94,39 @@ public void testIssue20090516() throws Exception { // piggy back tests for annotate methods assertOutput(cr, "plain text", "plain text"); - assertOutput(cr, "line #1\nhttp://nowhere.net/\nline #2\n", "line #1\nhttp://nowhere.net/\nline #2\n"); - assertOutput(cr, "failed; see http://nowhere.net/", "failed; see http://nowhere.net/"); - assertOutput(cr, "failed (see http://nowhere.net/)", "failed (see http://nowhere.net/)"); + assertOutput( + cr, + "line #1\nhttp://nowhere.net/\nline #2\n", + "line #1\n" + "http://nowhere.net/\n" + "line #2\n"); + assertOutput( + cr, + "failed; see http://nowhere.net/", + "failed; see http://nowhere.net/"); + assertOutput( + cr, + "failed (see http://nowhere.net/)", + "failed (see http://nowhere.net/)"); assertOutput( cr, "http://nowhere.net/ - failed: http://elsewhere.net/", - "http://nowhere.net/ - failed: " + "http://elsewhere.net/"); - assertOutput(cr, "https://nowhere.net/", "https://nowhere.net/"); + "http://nowhere.net/ - failed: http://elsewhere.net/"); + assertOutput(cr, "https://nowhere.net/", "https://nowhere.net/"); assertOutput(cr, "stuffhttp://nowhere.net/", "stuffhttp://nowhere.net/"); assertOutput(cr, "a < b && c < d", "a < b && c < d"); - assertOutput(cr, "see ", "see <http://nowhere.net/>"); - assertOutput(cr, "http://google.com/?q=stuff&lang=en", "http://google.com/?q=stuff&lang=en"); - assertOutput(cr, "http://localhost:8080/stuff/", "http://localhost:8080/stuff/"); + assertOutput( + cr, "see ", "see <http://nowhere.net/>"); + assertOutput( + cr, + "http://google.com/?q=stuff&lang=en", + "http://google.com/?q=stuff&lang=en"); + assertOutput( + cr, + "http://localhost:8080/stuff/", + "http://localhost:8080/stuff/"); + assertOutput( + cr, + "https://google.com\"onclick=alert(1)\"", + "https://google.com\"onclick=alert(1)\""); } /**