From 6f34546752d7df5b0df5d6c32518100be050efde Mon Sep 17 00:00:00 2001
From: Tim Jacomb <t.jacomb@kainos.com>
Date: Sat, 15 Dec 2018 16:50:47 +0000
Subject: [PATCH] Make slack response safer (#439)

* Make slack response safer

* Move error message to Messages.properties
---
 .../plugins/slack/workflow/SlackResponse.java   | 17 ++++++++++++-----
 .../plugins/slack/workflow/SlackSendStep.java   | 14 +++++++++++++-
 .../jenkins/plugins/slack/Messages.properties   |  1 +
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/src/main/java/jenkins/plugins/slack/workflow/SlackResponse.java b/src/main/java/jenkins/plugins/slack/workflow/SlackResponse.java
index 2ffccd82..6be27d4b 100644
--- 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
index c3093d62..5320d388 100644
--- 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
index cc64e5da..8f2f7f20 100644
--- 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)