Add escapeHtml parameter to all DataBoundTokenMacro implementations (#…

…118)
jenkinsci · Apr 20, 2022 · d1aaca2 · d1aaca2
1 parent 361b095
commit d1aaca2
Show file tree

Hide file tree

Showing 11 changed files with 206 additions and 14 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/tokenmacro/DataBoundTokenMacro.java b/src/main/java/org/jenkinsci/plugins/tokenmacro/DataBoundTokenMacro.java
@@ -24,7 +24,6 @@
 package org.jenkinsci.plugins.tokenmacro;
 
 import com.google.common.collect.ListMultimap;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.FilePath;
 import hudson.model.AbstractBuild;
 import hudson.model.Run;
@@ -45,6 +44,8 @@
 
 import static java.lang.annotation.ElementType.*;
 import static java.lang.annotation.RetentionPolicy.*;
+
+import org.apache.commons.lang.StringEscapeUtils;
 import org.apache.commons.lang.StringUtils;
 
 /**
@@ -85,6 +86,8 @@ private interface Setter {
     }
 
     private Map<String,Setter> setters;
+    @Parameter
+    public boolean escapeHtml = false;
 
     public DataBoundTokenMacro() {
         buildMap();
@@ -199,13 +202,36 @@ private DataBoundTokenMacro prepare(String macroName, Map<String, String> argume
     @Override
     public String evaluate(AbstractBuild<?, ?> build, TaskListener listener, String macroName, Map<String, String> arguments, ListMultimap<String, String> argumentMultimap) throws MacroEvaluationException, IOException, InterruptedException {
         DataBoundTokenMacro copy = prepare(macroName,arguments,argumentMultimap);
-        return copy.evaluate(build,listener,macroName);
+        String res = copy.evaluate(build, listener, macroName);
+        if (copy.escapeHtml && !copy.handlesHtmlEscapeInternally()) {
+            res = StringEscapeUtils.escapeHtml(res);
+        }
+        return res;
     }
 
     @Override
     public String evaluate(Run<?,?> run, FilePath workspace, TaskListener listener, String macroName, Map<String, String> arguments, ListMultimap<String, String> argumentMultimap) throws MacroEvaluationException, IOException, InterruptedException {
         DataBoundTokenMacro copy = prepare(macroName,arguments,argumentMultimap);
-        return copy.evaluate(run, workspace, listener, macroName);
+        String res = copy.evaluate(run, workspace, listener, macroName);
+        if (copy.escapeHtml && !copy.handlesHtmlEscapeInternally()) {
+            res = StringEscapeUtils.escapeHtml(res);
+        }
+        return res;
+    }
+
+    /**
+     * Indicates whether this macro handles {@link #escapeHtml} on its own inside the <code>evaluate</code> methods.
+     *
+     * If this method returns <code>false</code> and {@link #escapeHtml} is <code>true</code> then the returned value from
+     * {@link #evaluate(AbstractBuild, TaskListener, String)} and {@link #evaluate(Run, FilePath, TaskListener, String)}
+     * will be escaped. If this method returns <code>true</code> no escaping will be performed,
+     * and it is assumed the escaping will be handled internally by the implementing class. It is then also assumed that
+     * the <code>help.jelly</code> file for that class mentions the {@link #escapeHtml} parameter.
+     *
+     * @return true if the implementing class handles its own html escaping.
+     */
+    public boolean handlesHtmlEscapeInternally() {
+        return false;
     }
 
     public abstract String evaluate(AbstractBuild<?, ?> context, TaskListener listener, String macroName) throws MacroEvaluationException, IOException, InterruptedException;

diff --git a/src/main/java/org/jenkinsci/plugins/tokenmacro/impl/BuildLogMacro.java b/src/main/java/org/jenkinsci/plugins/tokenmacro/impl/BuildLogMacro.java
@@ -6,7 +6,6 @@
 import hudson.model.Run;
 import hudson.model.TaskListener;
 import org.apache.commons.lang.StringEscapeUtils;
-import org.apache.tools.ant.taskdefs.Parallel;
 import org.jenkinsci.plugins.tokenmacro.DataBoundTokenMacro;
 import org.jenkinsci.plugins.tokenmacro.MacroEvaluationException;
 
@@ -32,9 +31,6 @@ public class BuildLogMacro extends DataBoundTokenMacro {
     @Parameter
     public int truncTailLines = 0;
 
-    @Parameter
-    public boolean escapeHtml = false;
-
     @Parameter
     public int maxLineLength = MAX_LINE_LENGTH_DEFAULT_VALUE;
 
@@ -85,4 +81,9 @@ public String evaluate(Run<?,?> run, FilePath workspace, TaskListener listener,
 
         return buffer.toString();
     }
+
+    @Override
+    public boolean handlesHtmlEscapeInternally() {
+        return true;
+    }
 }
diff --git a/src/main/java/org/jenkinsci/plugins/tokenmacro/impl/BuildLogMultilineRegexMacro.java b/src/main/java/org/jenkinsci/plugins/tokenmacro/impl/BuildLogMultilineRegexMacro.java
@@ -6,14 +6,15 @@
 import hudson.model.AbstractBuild;
 import hudson.model.Run;
 import hudson.model.TaskListener;
-import org.apache.commons.lang.StringEscapeUtils;
 
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+
+import org.apache.commons.lang.StringEscapeUtils;
 import org.jenkinsci.plugins.tokenmacro.DataBoundTokenMacro;
 import org.jenkinsci.plugins.tokenmacro.MacroEvaluationException;
 
@@ -40,8 +41,6 @@ public class BuildLogMultilineRegexMacro extends DataBoundTokenMacro {
     @Parameter
     public String substText = null; // insert entire segment
     @Parameter
-    public boolean escapeHtml = false;
-    @Parameter
     public String matchedSegmentHtmlStyle = null;
 
     private static final Pattern LINE_TERMINATOR_PATTERN = Pattern.compile("(?<=.)\\r?\\n");
@@ -209,5 +208,10 @@ private int countLineTerminators(CharSequence charSequence) {
         }
         return lineTerminatorCount;
     }
+
+    @Override
+    public boolean handlesHtmlEscapeInternally() {
+        return true;
+    }
 }
 
diff --git a/src/main/java/org/jenkinsci/plugins/tokenmacro/impl/BuildLogRegexMacro.java b/src/main/java/org/jenkinsci/plugins/tokenmacro/impl/BuildLogRegexMacro.java
@@ -53,8 +53,6 @@ public class BuildLogRegexMacro extends DataBoundTokenMacro {
     @Parameter
     public String substText = null; // insert entire line
     @Parameter
-    public boolean escapeHtml = false;
-    @Parameter
     public String matchedLineHtmlStyle = null;
     @Parameter
     public boolean addNewline = true;
@@ -323,4 +321,9 @@ private static class Pair<K,V> extends AbstractMap.SimpleEntry<K,V>
             super(key, val);
         }
     }
+
+    @Override
+    public boolean handlesHtmlEscapeInternally() {
+        return true;
+    }
 }
diff --git a/src/main/resources/lib/token-macro/help.groovy b/src/main/resources/lib/token-macro/help.groovy
@@ -1,8 +1,18 @@
+import org.jenkinsci.plugins.tokenmacro.DataBoundTokenMacro
+
 st = namespace("jelly:stapler")
 
 
 org.jenkinsci.plugins.tokenmacro.TokenMacro.all().each { tm ->
     st.include(it:tm, page: "help", optional: true)
+    if (tm instanceof DataBoundTokenMacro && !tm.handlesHtmlEscapeInternally()) {
+        dd() {
+            dl() {
+                dt("escapeHtml")
+                dd(_("If true, any HTML specific code will be escaped. Defaults to false."))
+            }
+        }
+    }
     br()
 }
 

diff --git a/...ain/resources/org/jenkinsci/plugins/tokenmacro/impl/AbstractChangesSinceMacro/help.groovy b/...ain/resources/org/jenkinsci/plugins/tokenmacro/impl/AbstractChangesSinceMacro/help.groovy
@@ -23,6 +23,6 @@ dd {
     }
   }
   span("Following Parameters are also supported: " +
-          "showPaths, pathFormat, showDependencies, dateFormat, regex, replace, default. " +
+          "showPaths, pathFormat, showDependencies, dateFormat, regex, replace, default, escapeHtml. " +
           "See \${CHANGES_SINCE_LAST_BUILD} details.")
 }
diff --git a/src/main/resources/org/jenkinsci/plugins/tokenmacro/impl/JsonFileMacro/help.jelly b/src/main/resources/org/jenkinsci/plugins/tokenmacro/impl/JsonFileMacro/help.jelly
@@ -0,0 +1,9 @@
+<?jelly escape-by-default='true'?>
+<j:jelly xmlns:j="jelly:core">
+  <dt>$${JSON,file="FILE",path="JSON/PATH",expr="EXPRESSION"}</dt>
+  <dd>
+    Expands to the result(s) of a JSON path or expression run against the given JSON file.<br/>
+    If the path/expr evaluates to more than one value, then a semicolon-separated string is returned.<br/>
+    The file path is relative to the build workspace root.
+  </dd>
+</j:jelly>
diff --git a/src/test/java/org/jenkinsci/plugins/tokenmacro/impl/BuildLogRegexMacroTest.java b/src/test/java/org/jenkinsci/plugins/tokenmacro/impl/BuildLogRegexMacroTest.java
@@ -1,5 +1,6 @@
 package org.jenkinsci.plugins.tokenmacro.impl;
 
+import com.google.common.collect.ArrayListMultimap;
 import hudson.console.ConsoleNote;
 import hudson.model.AbstractBuild;
 import hudson.model.TaskListener;
@@ -11,6 +12,7 @@
 
 import java.io.InputStreamReader;
 import java.io.StringReader;
+import java.util.Collections;
 import java.util.Map;
 import java.util.HashMap;
 
@@ -436,6 +438,24 @@ public void testGetContent_matchedLineHtmlStyle()
         assertEquals("<pre>\n<b style=\"color: red\">error</b>\n</pre>\n", result);
     }
 
+    @Test
+    public void testGetContent_matchedLineHtmlStyleWithHtmlEscape()
+            throws Exception {
+        when(build.getLogReader()).thenReturn(
+                new StringReader("<error>"));
+        final Map<String, String> arguments = new HashMap<>();
+        arguments.put("escapeHtml", "true");
+        arguments.put("showTruncatedLines", "false");
+        arguments.put("matchedLineHtmlStyle", "color: red");
+        final ArrayListMultimap<String, String> listMultimap = ArrayListMultimap.create();
+        listMultimap.put("escapeHtml", "true");
+        listMultimap.put("showTruncatedLines", "false");
+        listMultimap.put("matchedLineHtmlStyle", "color: red");
+        final String result = buildLogRegexMacro.evaluate(build, null, listener, BuildLogRegexMacro.MACRO_NAME, arguments, listMultimap);
+
+        assertEquals("<pre>\n<b style=\"color: red\">&lt;error&gt;</b>\n</pre>\n", result);
+    }
+
     @Test
     public void testGetContent_shouldStripOutConsoleNotes()
             throws Exception {

diff --git a/src/test/java/org/jenkinsci/plugins/tokenmacro/impl/ChangesSinceLastBuildMacroTest.java b/src/test/java/org/jenkinsci/plugins/tokenmacro/impl/ChangesSinceLastBuildMacroTest.java
@@ -1,5 +1,6 @@
 package org.jenkinsci.plugins.tokenmacro.impl;
 
+import com.google.common.collect.ArrayListMultimap;
 import hudson.model.AbstractBuild;
 import hudson.model.Result;
 import hudson.model.TaskListener;
@@ -17,10 +18,12 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.TimeZone;
 
 import static junit.framework.Assert.assertEquals;
 import static junit.framework.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -185,6 +188,29 @@ public void testShouldPrintMessageWhenNoChanges()
         assertEquals("another default message\n", content);
     }
 
+    @Test
+    public void testShouldDefaultToNotEscapeHtml()
+            throws Exception {
+        AbstractBuild currentBuild = createBuild(Result.SUCCESS, 42, "<b>bold</b>");
+
+        String content = changesSinceLastBuildMacro.evaluate(currentBuild, listener, ChangesSinceLastBuildMacro.MACRO_NAME);
+
+        assertEquals("[Ash Lux] <b>bold</b>\n\n", content);
+    }
+
+    @Test
+    public void testShouldEscapeHtmlWhenArgumentEscapeHtmlSetToTrue()
+            throws Exception {
+        AbstractBuild currentBuild = createBuild(Result.SUCCESS, 42, "<b>bold</b>");
+
+        final Map<String, String> arguments = Collections.singletonMap("escapeHtml", "true");
+        final ArrayListMultimap<String, String> listMultimap = ArrayListMultimap.create();
+        listMultimap.put("escapeHtml", "true");
+        String content = changesSinceLastBuildMacro.evaluate(currentBuild, null, listener, ChangesSinceLastBuildMacro.MACRO_NAME, arguments, listMultimap);
+
+        assertEquals("[Ash Lux] &lt;b&gt;bold&lt;/b&gt;\n\n", content);
+    }
+
     private AbstractBuild createBuild(Result result, int buildNumber, String message) {
         AbstractBuild build = mock(AbstractBuild.class);
         when(build.getResult()).thenReturn(result);

diff --git a/.../java/org/jenkinsci/plugins/tokenmacro/impl/ChangesSinceLastSuccessfulBuildMacroTest.java b/.../java/org/jenkinsci/plugins/tokenmacro/impl/ChangesSinceLastSuccessfulBuildMacroTest.java
@@ -1,5 +1,6 @@
 package org.jenkinsci.plugins.tokenmacro.impl;
 
+import com.google.common.collect.ArrayListMultimap;
 import hudson.model.AbstractBuild;
 import hudson.model.Result;
 import hudson.model.TaskListener;
@@ -16,6 +17,7 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.TimeZone;
 import static junit.framework.Assert.assertEquals;
 
@@ -278,7 +280,7 @@ public void testShouldPrintMessageWhenNoChanges()
 
         String contentStr = content.evaluate(currentBuild, listener, ChangesSinceLastSuccessfulBuildMacro.MACRO_NAME);
 
-        Assert.assertEquals("Changes for Build #41\n"
+        assertEquals("Changes for Build #41\n"
                 + "[Ash Lux] [DEFECT-666] Changes for a failed build.\n"
                 + "\n"
                 + "\n"
@@ -287,6 +289,49 @@ public void testShouldPrintMessageWhenNoChanges()
                 + "\n", contentStr);
     }
 
+    @Test
+    public void testShouldDefaultToNotEscapeHtml()
+            throws Exception {
+        AbstractBuild failureBuild = createBuild(Result.FAILURE, 41, "[DEFECT-666] Changes for a failed build. <b>bold</b>");
+
+        AbstractBuild currentBuild = createBuildWithNoChanges(Result.SUCCESS, 42);
+        when(currentBuild.getPreviousBuild()).thenReturn(failureBuild);
+        when(failureBuild.getNextBuild()).thenReturn(currentBuild);
+
+        String contentStr = content.evaluate(currentBuild, listener, ChangesSinceLastSuccessfulBuildMacro.MACRO_NAME);
+
+        Assert.assertEquals("Changes for Build #41\n"
+                + "[Ash Lux] [DEFECT-666] Changes for a failed build. <b>bold</b>\n"
+                + "\n"
+                + "\n"
+                + "Changes for Build #42\n"
+                + "No changes\n"
+                + "\n", contentStr);
+    }
+
+    @Test
+    public void testShouldEscapeHtmlWhenArgumentEscapeHtmlSetToTrue()
+            throws Exception {
+        AbstractBuild failureBuild = createBuild(Result.FAILURE, 41, "[DEFECT-666] Changes for a failed build. <b>bold</b>");
+
+        AbstractBuild currentBuild = createBuildWithNoChanges(Result.SUCCESS, 42);
+        when(currentBuild.getPreviousBuild()).thenReturn(failureBuild);
+        when(failureBuild.getNextBuild()).thenReturn(currentBuild);
+
+        final Map<String, String> arguments = Collections.singletonMap("escapeHtml", "true");
+        final ArrayListMultimap<String, String> listMultimap = ArrayListMultimap.create();
+        listMultimap.put("escapeHtml", "true");
+        String contentStr = content.evaluate(currentBuild, null, listener, ChangesSinceLastSuccessfulBuildMacro.MACRO_NAME, arguments, listMultimap);
+
+        assertEquals("Changes for Build #41\n"
+                + "[Ash Lux] [DEFECT-666] Changes for a failed build. &lt;b&gt;bold&lt;/b&gt;\n"
+                + "\n"
+                + "\n"
+                + "Changes for Build #42\n"
+                + "No changes\n"
+                + "\n", contentStr);
+    }
+
     private AbstractBuild createBuildWithNoChanges(Result result, int buildNumber) {
         AbstractBuild build = mock(AbstractBuild.class);
         when(build.getResult()).thenReturn(result);