implement suggested checks

(cherry picked from commit 6e60d5d74b51732f27515ba14e8180990991f831)
jenkinsci · Dec 21, 2021 · 5ca8ab7 · 5ca8ab7
1 parent 2fa7dd4
commit 5ca8ab7
Show file tree

Hide file tree

Showing 11 changed files with 105 additions and 28 deletions.
diff --git a/pom.xml b/pom.xml
@@ -113,6 +113,11 @@
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-step-api</artifactId>
         </dependency>
+        <dependency>
+            <groupId>commons-validator</groupId>
+            <artifactId>commons-validator</artifactId>
+            <version>1.7</version>
+        </dependency>
 
         <dependency>
             <groupId>org.jenkins-ci.plugins</groupId>

diff --git a/src/main/java/com/jenkinsci/plugins/badge/BadgePlugin.java b/src/main/java/com/jenkinsci/plugins/badge/BadgePlugin.java
@@ -26,6 +26,7 @@
 import com.jenkinsci.plugins.badge.action.BadgeAction;
 import com.jenkinsci.plugins.badge.action.BadgeSummaryAction;
 import hudson.Extension;
+import hudson.markup.RawHtmlMarkupFormatter;
 import hudson.model.Action;
 import hudson.model.Job;
 import hudson.model.Run;
@@ -40,25 +41,32 @@
 @Extension
 public class BadgePlugin extends GlobalConfiguration {
 
-  /** @return the singleton instance */
+  /**
+   * @return the singleton instance
+   */
   public static BadgePlugin get() {
     return GlobalConfiguration.all().get(BadgePlugin.class);
   }
 
   private boolean disableFormatHTML;
 
+  private final RawHtmlMarkupFormatter formatter = new RawHtmlMarkupFormatter(false);
+
   public BadgePlugin() {
     // When Jenkins is restarted, load any saved configuration from disk.
     load();
   }
 
-  /** @return the whether HTML formatting is disabled or not */
+  /**
+   * @return the whether HTML formatting is disabled or not
+   */
   public boolean isDisableFormatHTML() {
     return disableFormatHTML;
   }
 
   /**
    * Together with {@link #isDisableFormatHTML}, binds to entry in {@code config.jelly}.
+   *
    * @param disableFormatHTML the new value of this field
    */
   @DataBoundSetter
@@ -123,4 +131,8 @@ private void removeAction(Class type, StaplerRequest req, StaplerResponse rsp) t
       rsp.sendRedirect(req.getRequestURI().substring(0, req.getRequestURI().indexOf("parent/parent")));
     }
   }
+
+  public String translate(String text) throws IOException {
+    return formatter.translate(text);
+  }
 }
diff --git a/src/main/java/com/jenkinsci/plugins/badge/action/BadgeAction.java b/src/main/java/com/jenkinsci/plugins/badge/action/BadgeAction.java
@@ -32,9 +32,13 @@
 import org.kohsuke.stapler.export.ExportedBean;
 
 import java.io.File;
+import java.io.IOException;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 @ExportedBean(defaultVisibility = 2)
 public class BadgeAction extends AbstractBadgeAction {
+  private static final Logger LOGGER = Logger.getLogger(BadgeSummaryAction.class.getName());
   private static final long serialVersionUID = 1L;
   private final String iconPath;
   private final String text;
@@ -53,9 +57,10 @@ public static BadgeAction createBadge(String icon, String text) {
     return new BadgeAction(getIconPath(icon), text);
   }
 
-  public static BadgeAction createBadge(String icon, String text, String link) {
+  public static BadgeAction createBadge(String icon, String text, String link) throws IllegalArgumentException {
     BadgeAction action = new BadgeAction(getIconPath(icon), text);
     action.link = link;
+    action.validate();
     return action;
   }
 
@@ -78,30 +83,40 @@ public static BadgeAction createShortText(String text, String color, String back
     return action;
   }
 
-  public static BadgeAction createInfoBadge(String text) {
+  public static BadgeAction createInfoBadge(String text) throws IllegalArgumentException {
     return createInfoBadge(text, null);
   }
 
-  public static BadgeAction createInfoBadge(String text, String link) {
+  public static BadgeAction createInfoBadge(String text, String link) throws IllegalArgumentException {
     return createBadge("info.gif", text, link);
   }
 
-  public static BadgeAction createWarningBadge(String text) {
+  public static BadgeAction createWarningBadge(String text) throws IllegalArgumentException {
     return createWarningBadge(text, null);
   }
 
-  public static BadgeAction createWarningBadge(String text, String link) {
+  public static BadgeAction createWarningBadge(String text, String link) throws IllegalArgumentException {
     return createBadge("warning.gif", text, link);
   }
 
-  public static BadgeAction createErrorBadge(String text) {
+  public static BadgeAction createErrorBadge(String text) throws IllegalArgumentException {
     return createErrorBadge(text, null);
   }
 
-  public static BadgeAction createErrorBadge(String text, String link) {
+  public static BadgeAction createErrorBadge(String text, String link) throws IllegalArgumentException {
     return createBadge("error.gif", text, link);
   }
 
+  protected void validate() throws IllegalArgumentException {
+    if (BadgePlugin.get().isDisableFormatHTML()) {
+      return;
+    }
+
+    if (link != null && !link.startsWith("/") && !link.matches("^https?:.*") && !link.matches("^mailto:.*")) {
+      throw new IllegalArgumentException("Invalid link '" + link + "'for badge action with text '" + text + "'");
+    }
+  }
+
   /* Action methods */
   public String getUrlName() {
     return "";
@@ -122,7 +137,7 @@ public boolean isTextOnly() {
 
   @Exported
   public String getIconPath() {
-    // add the context path to the path variable if the umage starts with /
+    // add the context path to the path variable if the image starts with /
     if (iconPath != null && iconPath.startsWith("/")) {
       StaplerRequest currentRequest = Stapler.getCurrentRequest();
       if (currentRequest != null && !iconPath.startsWith(currentRequest.getContextPath())) {
@@ -134,7 +149,15 @@ public String getIconPath() {
 
   @Exported
   public String getText() {
-    return text;
+    if (BadgePlugin.get().isDisableFormatHTML()) {
+      return text;
+    }
+    try {
+      return BadgePlugin.get().translate(text);
+    } catch (IOException e) {
+      LOGGER.log(Level.WARNING, "Error preparing badge text for ui", e);
+      return "<b><font color=\"red\">ERROR</font></b>";
+    }
   }
 
   @Exported
@@ -159,7 +182,16 @@ public String getBorderColor() {
 
   @Exported
   public String getLink() {
-    return link;
+    if (link == null || BadgePlugin.get().isDisableFormatHTML()) {
+      return link;
+    }
+
+    if (link.startsWith("/") || link.matches("^https?:.*") || link.matches("^mailto:.*")) {
+      return link;
+    }
+    LOGGER.log(Level.WARNING, "Error invalid link value: '" + link + "'");
+
+    return null;
   }
 
   public static String getIconPath(String icon) {

diff --git a/src/main/java/com/jenkinsci/plugins/badge/action/BadgeSummaryAction.java b/src/main/java/com/jenkinsci/plugins/badge/action/BadgeSummaryAction.java
@@ -24,7 +24,6 @@
 package com.jenkinsci.plugins.badge.action;
 
 import com.jenkinsci.plugins.badge.BadgePlugin;
-import hudson.markup.RawHtmlMarkupFormatter;
 import org.apache.commons.lang.StringEscapeUtils;
 import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.Whitelisted;
 import org.kohsuke.stapler.export.Exported;
@@ -75,7 +74,7 @@ public String getText() {
       return summaryText;
     }
     try {
-      return new RawHtmlMarkupFormatter(false).translate(summaryText);
+      return BadgePlugin.get().translate(summaryText);
     } catch (IOException e) {
       LOGGER.log(Level.WARNING, "Error preparing summary text for ui", e);
       return "<b><font color=\"red\">ERROR</font></b>";

diff --git a/src/main/java/com/jenkinsci/plugins/badge/action/HtmlBadgeAction.java b/src/main/java/com/jenkinsci/plugins/badge/action/HtmlBadgeAction.java
@@ -24,7 +24,6 @@
 package com.jenkinsci.plugins.badge.action;
 
 import com.jenkinsci.plugins.badge.BadgePlugin;
-import hudson.markup.RawHtmlMarkupFormatter;
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.export.ExportedBean;
 
@@ -70,7 +69,7 @@ public String getHtml() {
       return html;
     }
     try {
-      return new RawHtmlMarkupFormatter(false).translate(html);
+      return BadgePlugin.get().translate(html);
     } catch (IOException e) {
       LOGGER.log(Level.WARNING, "Error preparing html content for ui", e);
       return "<b><font color=\"red\">ERROR</font></b>";

diff --git a/src/main/java/com/jenkinsci/plugins/badge/dsl/AddBadgeStep.java b/src/main/java/com/jenkinsci/plugins/badge/dsl/AddBadgeStep.java
@@ -144,7 +144,7 @@ protected Void run() throws Exception {
       return null;
     }
 
-    protected BadgeAction newBatchAction(Badge badge) {
+    protected BadgeAction newBatchAction(Badge badge) throws IllegalArgumentException {
       return BadgeAction.createBadge(badge.getIcon(), badge.getText(), badge.getLink());
     }
 

diff --git a/src/main/java/com/jenkinsci/plugins/badge/dsl/AddErrorBadgeStep.java b/src/main/java/com/jenkinsci/plugins/badge/dsl/AddErrorBadgeStep.java
@@ -61,7 +61,7 @@ public StepExecution start(StepContext context) {
     return new Execution(getBadge(), getId(), context) {
 
       @Override
-      protected BadgeAction newBatchAction(Badge badge) {
+      protected BadgeAction newBatchAction(Badge badge) throws IllegalArgumentException {
         return BadgeAction.createErrorBadge(badge.getText(), badge.getLink());
       }
     };

diff --git a/src/main/java/com/jenkinsci/plugins/badge/dsl/AddInfoBadgeStep.java b/src/main/java/com/jenkinsci/plugins/badge/dsl/AddInfoBadgeStep.java
@@ -61,7 +61,7 @@ public StepExecution start(StepContext context) {
     return new Execution(getBadge(), getId(), context) {
 
       @Override
-      protected BadgeAction newBatchAction(Badge badge) {
+      protected BadgeAction newBatchAction(Badge badge) throws IllegalArgumentException {
         return BadgeAction.createInfoBadge(badge.getText(), badge.getLink());
       }
     };

diff --git a/src/main/java/com/jenkinsci/plugins/badge/dsl/AddWarningBadgeStep.java b/src/main/java/com/jenkinsci/plugins/badge/dsl/AddWarningBadgeStep.java
@@ -61,7 +61,7 @@ public StepExecution start(StepContext context) {
     return new Execution(getBadge(), getId(), context) {
 
       @Override
-      protected BadgeAction newBatchAction(Badge badge) {
+      protected BadgeAction newBatchAction(Badge badge) throws IllegalArgumentException {
         return BadgeAction.createWarningBadge(badge.getText(), badge.getLink());
       }
     };

diff --git a/src/test/java/com/jenkinsci/plugins/badge/dsl/AddBadgeStepTest.java b/src/test/java/com/jenkinsci/plugins/badge/dsl/AddBadgeStepTest.java
@@ -25,6 +25,7 @@
 
 import com.jenkinsci.plugins.badge.action.BadgeAction;
 import hudson.model.BuildBadgeAction;
+import hudson.model.Result;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
@@ -33,9 +34,7 @@
 import java.util.List;
 import java.util.UUID;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 
 public class AddBadgeStepTest extends AbstractBadgeTest {
 
@@ -52,7 +51,7 @@ public void addBadge_in_node() throws Exception {
   private void addBadge(boolean inNode) throws Exception {
     String icon = UUID.randomUUID().toString();
     String text = UUID.randomUUID().toString();
-    String link = UUID.randomUUID().toString();
+    String link = "https://" + UUID.randomUUID();
     WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
 
     String script = "addBadge(icon:\"" + icon + "\",  text:\"" + text + "\",  link:\"" + link + "\")";
@@ -92,10 +91,10 @@ public void addErrorBadge() throws Exception {
 
   private void addStatusBadge(String functionName, String expectedIcon, boolean withLink) throws Exception {
     String text = UUID.randomUUID().toString();
-    String link = UUID.randomUUID().toString();
+    String link = "mailto://" + UUID.randomUUID();
 
     WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, text);
-    String script = functionName +"(text:\"" + text + "\"";
+    String script = functionName + "(text:\"" + text + "\"";
     if (withLink) {
       script += ",  link:\"" + link + "\"";
     }
@@ -116,4 +115,35 @@ private void addStatusBadge(String functionName, String expectedIcon, boolean wi
       assertNull(action.getLink());
     }
   }
+
+  @Test
+  public void addBadge_invalid_link() throws Exception {
+    String icon = UUID.randomUUID().toString();
+    String text = UUID.randomUUID().toString();
+    String link = "javascript:" + UUID.randomUUID();
+    WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
+
+    String script = "addBadge(icon:\"" + icon + "\",  text:\"" + text + "\",  link:\"" + link + "\")";
+    p.setDefinition(new CpsFlowDefinition(script, true));
+    r.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0));
+  }
+
+  @Test
+  public void addBadge_invalid_text() throws Exception {
+    String icon = UUID.randomUUID().toString();
+    String textPrefix = UUID.randomUUID().toString();
+    String text = textPrefix + "');alert('foo";
+    WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
+
+    String script = "addBadge(icon:\"" + icon + "\",  text:\"" + text + "\")";
+    p.setDefinition(new CpsFlowDefinition(script, true));
+    WorkflowRun b = r.assertBuildStatusSuccess(p.scheduleBuild2(0));
+
+    List<BuildBadgeAction> badgeActions = b.getBadgeActions();
+    assertEquals(1, badgeActions.size());
+
+    BadgeAction action = (BadgeAction) badgeActions.get(0);
+    assertTrue(action.getIconPath().endsWith(icon));
+    assertEquals(textPrefix + "&#39;);alert(&#39;foo", action.getText());
+  }
 }
diff --git a/src/test/java/com/jenkinsci/plugins/badge/dsl/ShortTextStepTest.java b/src/test/java/com/jenkinsci/plugins/badge/dsl/ShortTextStepTest.java
@@ -47,11 +47,11 @@ public void addShortText() throws Exception {
     String background = UUID.randomUUID().toString();
     Integer border = new Random().nextInt();
     String borderColor = UUID.randomUUID().toString();
-    String link = UUID.randomUUID().toString();
+    String link = "http://" + UUID.randomUUID();
 
     WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
     p.setDefinition(new CpsFlowDefinition("addShortText(text:\"" + text + "\",color:\"" + color + "\", background:\"" + background + "\", border:" + border + ", borderColor:\""
-        + borderColor + "\", link:\"" + link + "\")", true));
+            + borderColor + "\", link:\"" + link + "\")", true));
     WorkflowRun b = r.assertBuildStatusSuccess(p.scheduleBuild2(0));
 
     List<? extends Action> allActions = b.getAllActions();