From f73157dd8f607a1b9c94a91e38d5575c56928dc2 Mon Sep 17 00:00:00 2001
From: Shota Gvinepadze <wineson@gmail.com>
Date: Mon, 28 Sep 2020 17:59:04 +0400
Subject: [PATCH] Fix command url validation (#15239)

* Fix command url validation

* Add Host and Scheme check in IsValidHttpUrl func

* Add unit tests

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
---
 model/command_test.go |  3 ++
 model/utils.go        |  2 +-
 model/utils_test.go   | 90 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/model/command_test.go b/model/command_test.go
index 3bb0432e07..1995c4572e 100644
--- a/model/command_test.go
+++ b/model/command_test.go
@@ -86,6 +86,9 @@ func TestCommandIsValid(t *testing.T) {
 	o.URL = "1234"
 	require.NotNil(t, o.IsValid(), "should be invalid")
 
+	o.URL = "https:////example.com"
+	require.NotNil(t, o.IsValid(), "should be invalid")
+
 	o.URL = "https://example.com"
 	require.Nil(t, o.IsValid())
 
diff --git a/model/utils.go b/model/utils.go
index 2ab71090d4..47dd8c36b8 100644
--- a/model/utils.go
+++ b/model/utils.go
@@ -496,7 +496,7 @@ func IsValidHttpUrl(rawUrl string) bool {
 		return false
 	}
 
-	if _, err := url.ParseRequestURI(rawUrl); err != nil {
+	if u, err := url.ParseRequestURI(rawUrl); err != nil || u.Scheme == "" || u.Host == "" {
 		return false
 	}
 
diff --git a/model/utils_test.go b/model/utils_test.go
index 6e93eff001..38ff68d092 100644
--- a/model/utils_test.go
+++ b/model/utils_test.go
@@ -776,3 +776,93 @@ func TestSanitizeUnicode(t *testing.T) {
 		})
 	}
 }
+
+func TestIsValidHttpUrl(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		Description string
+		Value       string
+		Expected    bool
+	}{
+		{
+			"empty url",
+			"",
+			false,
+		},
+		{
+			"bad url",
+			"bad url",
+			false,
+		},
+		{
+			"relative url",
+			"/api/test",
+			false,
+		},
+		{
+			"relative url ending with slash",
+			"/some/url/",
+			false,
+		},
+		{
+			"url with invalid scheme",
+			"htp://mattermost.com",
+			false,
+		},
+		{
+			"url with just http",
+			"http://",
+			false,
+		},
+		{
+			"url with just https",
+			"https://",
+			false,
+		},
+		{
+			"url with extra slashes",
+			"https:///mattermost.com",
+			false,
+		},
+		{
+			"correct url with http scheme",
+			"http://mattemost.com",
+			true,
+		},
+		{
+			"correct url with https scheme",
+			"https://mattermost.com/api/test",
+			true,
+		},
+		{
+			"correct url with port",
+			"https://localhost:8080/test",
+			true,
+		},
+		{
+			"correct url without scheme",
+			"mattermost.com/some/url/",
+			false,
+		},
+		{
+			"correct url with extra slashes",
+			"https://mattermost.com/some//url",
+			true,
+		},
+	}
+
+	for _, testCase := range testCases {
+		testCase := testCase
+		t.Run(testCase.Description, func(t *testing.T) {
+			defer func() {
+				if r := recover(); r != nil {
+					t.Errorf("panic: %v", r)
+				}
+			}()
+
+			t.Parallel()
+			require.Equal(t, testCase.Expected, IsValidHttpUrl(testCase.Value))
+		})
+	}
+}