Make slack response safer (#439)

* Make slack response safer * Move error message to Messages.properties
jenkinsci · Dec 15, 2018 · 6f34546 · 6f34546
1 parent bdb3b08
commit 6f34546
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 6 deletions.
diff --git a/src/main/java/jenkins/plugins/slack/workflow/SlackResponse.java b/src/main/java/jenkins/plugins/slack/workflow/SlackResponse.java
@@ -7,14 +7,21 @@
 import java.io.Serializable;
 
 public class SlackResponse implements Serializable {
+    private static final String THREAD_ID = "ts";
+    private static final String CHANNEL = "channel";
+
     private String channelId;
     private String ts;
 
-    public SlackResponse(String slackResponseString) {
-        if (!StringUtils.isEmpty(slackResponseString)) {
-            JSONObject result = new JSONObject(slackResponseString);
-            channelId = result.getString("channel");
-            ts = result.getString("ts");
+    public SlackResponse() {
+    }
+
+    public SlackResponse(JSONObject slackResponseObject) {
+        if (slackResponseObject.has(CHANNEL)) {
+            channelId = slackResponseObject.getString(CHANNEL);
+        }
+        if (slackResponseObject.has(THREAD_ID)) {
+            this.ts = slackResponseObject.getString(THREAD_ID);
         }
     }
 

diff --git a/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java b/src/main/java/jenkins/plugins/slack/workflow/SlackSendStep.java
@@ -269,7 +269,19 @@ protected SlackResponse run() throws Exception {
             SlackResponse response = null;
             if (publishSuccess) {
                 String responseString = slackService.getResponseString();
-                response = new SlackResponse(responseString);
+                if (responseString != null) {
+                    try {
+                        org.json.JSONObject result = new org.json.JSONObject(responseString);
+                        response = new SlackResponse(result);
+                    } catch (org.json.JSONException ex) {
+                        listener.error(Messages.FailedToParseSlackResponse());
+                        if (step.failOnError) {
+                            throw ex;
+                        }
+                    }
+                } else {
+                    return new SlackResponse();
+                }
             } else if (step.failOnError) {
                 throw new AbortException(Messages.NotificationFailed());
             } else {

diff --git a/src/main/resources/jenkins/plugins/slack/Messages.properties b/src/main/resources/jenkins/plugins/slack/Messages.properties
@@ -5,3 +5,4 @@ SlackSendStepDisplayName=Send Slack Message
 NotificationFailed=Slack notification failed. See Jenkins logs for details.
 NotificationFailedWithException=Slack notification failed with exception: {0}
 SlackSendStepConfig=Slack Send Pipeline step configured values from global config - baseUrl: {0}, teamDomain: {1}, token: {2}, channel: {3}, color: {4}
+FailedToParseSlackResponse=Could not parse response from slack, potentially because of invalid configuration (botUser: true and baseUrl set)