MM-10232, MM-10259: Improve error handling from invalid json (#8668)

* MM-10232: improve error handling from malformed slash command responses

Switch to json.Unmarshal, which doesn't obscure JSON parse failures like
json.Decode. The latter is primarily designed for streams of JSON, not
necessarily unmarshalling just a single object.

* rework HumanizedJsonError to expose Line and Character discretely

* MM-10259: pinpoint line and character where json config error occurs

* tweak HumanizeJsonError to accept err first

2 files changed, 15 insertions, 7 deletions

diff --git a/api4/webhook.go b/api4/webhook.go
index 52c4ea331..a0e7b5785 100644
--- a/api4/webhook.go
+++ b/api4/webhook.go
@@ -509,11 +509,15 @@ func commandWebhook(c *Context, w http.ResponseWriter, r *http.Request) {
 	params := mux.Vars(r)
 	id := params["id"]
 
-	response := model.CommandResponseFromHTTPBody(r.Header.Get("Content-Type"), r.Body)
-
-	err := c.App.HandleCommandWebhook(id, response)
+	response, err := model.CommandResponseFromHTTPBody(r.Header.Get("Content-Type"), r.Body)
 	if err != nil {
-		c.Err = err
+		c.Err = model.NewAppError("commandWebhook", "web.command_webhook.parse.app_error", nil, err.Error(), http.StatusBadRequest)
+		return
+	}
+
+	appErr := c.App.HandleCommandWebhook(id, response)
+	if appErr != nil {
+		c.Err = appErr
 		return
 	}
 
diff --git a/api4/webhook_test.go b/api4/webhook_test.go
index 0a295b4b2..e983b6461 100644
--- a/api4/webhook_test.go
+++ b/api4/webhook_test.go
@@ -917,17 +917,21 @@ func TestCommandWebhooks(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if resp, _ := http.Post(Client.Url+"/hooks/commands/123123123123", "application/json", bytes.NewBufferString("{\"text\":\"this is a test\"}")); resp.StatusCode != http.StatusNotFound {
+	if resp, _ := http.Post(Client.Url+"/hooks/commands/123123123123", "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)); resp.StatusCode != http.StatusNotFound {
 		t.Fatal("expected not-found for non-existent hook")
 	}
 
+	if resp, err := http.Post(Client.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"invalid`)); err != nil || resp.StatusCode != http.StatusBadRequest {
+		t.Fatal(err)
+	}
+
 	for i := 0; i < 5; i++ {
-		if resp, err := http.Post(Client.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString("{\"text\":\"this is a test\"}")); err != nil || resp.StatusCode != http.StatusOK {
+		if resp, err := http.Post(Client.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)); err != nil || resp.StatusCode != http.StatusOK {
 			t.Fatal(err)
 		}
 	}
 
-	if resp, _ := http.Post(Client.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString("{\"text\":\"this is a test\"}")); resp.StatusCode != http.StatusBadRequest {
+	if resp, _ := http.Post(Client.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)); resp.StatusCode != http.StatusBadRequest {
 		t.Fatal("expected error for sixth usage")
 	}
 }