From 5b369d377704325011a2ee6ad4d0037a704499d5 Mon Sep 17 00:00:00 2001
From: Lev <1187448+levb@users.noreply.github.com>
Date: Fri, 4 Jan 2019 10:57:13 -0700
Subject: [PATCH] MM-13598 Added missing error check to DoActionRequest
 (#10058)

* MM-13598 Added missing error check in DoActionRequest

* Added a test
---
 app/integration_action.go      |  5 +++-
 app/integration_action_test.go | 49 ++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/app/integration_action.go b/app/integration_action.go
index 9f40a2f808..b9c4a515ed 100644
--- a/app/integration_action.go
+++ b/app/integration_action.go
@@ -126,7 +126,10 @@ func (a *App) DoPostAction(postId, actionId, userId, selectedOption string) (str
 // Perform an HTTP POST request to an integration's action endpoint.
 // Caller must consume and close returned http.Response as necessary.
 func (a *App) DoActionRequest(rawURL string, body []byte) (*http.Response, *model.AppError) {
-	req, _ := http.NewRequest("POST", rawURL, bytes.NewReader(body))
+	req, err := http.NewRequest("POST", rawURL, bytes.NewReader(body))
+	if err != nil {
+		return nil, model.NewAppError("DoActionRequest", "api.post.do_action.action_integration.app_error", nil, err.Error(), http.StatusBadRequest)
+	}
 	req.Header.Set("Content-Type", "application/json")
 	req.Header.Set("Accept", "application/json")
 
diff --git a/app/integration_action_test.go b/app/integration_action_test.go
index 070bf3519f..7f76f55e82 100644
--- a/app/integration_action_test.go
+++ b/app/integration_action_test.go
@@ -17,6 +17,55 @@ import (
 	"github.com/mattermost/mattermost-server/model"
 )
 
+// Test for MM-13598 where an invalid integration URL was causing a crash
+func TestPostActionInvalidURL(t *testing.T) {
+	th := Setup().InitBasic()
+	defer th.TearDown()
+
+	th.App.UpdateConfig(func(cfg *model.Config) {
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1"
+	})
+
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		request := model.PostActionIntegrationRequestFromJson(r.Body)
+		assert.NotNil(t, request)
+	}))
+	defer ts.Close()
+
+	interactivePost := model.Post{
+		Message:       "Interactive post",
+		ChannelId:     th.BasicChannel.Id,
+		PendingPostId: model.NewId() + ":" + fmt.Sprint(model.GetMillis()),
+		UserId:        th.BasicUser.Id,
+		Props: model.StringInterface{
+			"attachments": []*model.SlackAttachment{
+				{
+					Text: "hello",
+					Actions: []*model.PostAction{
+						{
+							Integration: &model.PostActionIntegration{
+								URL: ":test",
+							},
+							Name: "action",
+							Type: "some_type",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	post, err := th.App.CreatePostAsUser(&interactivePost, false)
+	require.Nil(t, err)
+	attachments, ok := post.Props["attachments"].([]*model.SlackAttachment)
+	require.True(t, ok)
+	require.NotEmpty(t, attachments[0].Actions)
+	require.NotEmpty(t, attachments[0].Actions[0].Id)
+
+	_, err = th.App.DoPostAction(post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "")
+	require.NotNil(t, err)
+}
+
 func TestPostAction(t *testing.T) {
 	th := Setup().InitBasic()
 	defer th.TearDown()