MM-40469: Handle invalid response codes from plugins (#19136)

* MM-40469: Handle invalid response codes from plugins

This was an interesting crash detected via Sentry.

Typically, any HTTP status code outside 100-999 range
will cause a crash in the HTTP server. And `ExecuteCommand`
is the only plugin hook which returns a model.AppError
instead of error.

So an incorrect plugin implementation could return
a status code of 0, and crash the server. We handle that
by rewriting any illegal response code to 500.

https://mattermost.atlassian.net/browse/MM-40469

```release-note
NONE
```

* added warning

```release-note
NONE
```
Этот коммит содержится в:
Agniva De Sarker
2021-12-09 14:17:41 +05:30
коммит произвёл GitHub
родитель 052d99c362
Коммит 0e00afd65f
2 изменённых файлов: 69 добавлений и 0 удалений

Просмотреть файл

@@ -13,6 +13,7 @@ import (
"github.com/mattermost/mattermost-server/v6/app/request"
"github.com/mattermost/mattermost-server/v6/model"
"github.com/mattermost/mattermost-server/v6/shared/mlog"
)
type PluginCommand struct {
@@ -164,5 +165,12 @@ func (a *App) tryExecutePluginCommand(c *request.Context, args *model.CommandArg
errMessage := fmt.Sprintf("err= Plugin %s crashed due to /%s command", matched.PluginId, trigger)
return matched.Command, nil, model.NewAppError("ExecutePluginCommand", "model.plugin_command_crash.error.app_error", map[string]interface{}{"Command": trigger, "PluginId": matched.PluginId}, errMessage, http.StatusInternalServerError)
}
// This is a response from the plugin, which may set an incorrect status code;
// e.g setting a status code of 0 will crash the server. So we always bucket everything under 500.
if appErr != nil && (appErr.StatusCode < 100 || appErr.StatusCode > 999) {
mlog.Warn("Invalid status code returned from plugin. Converting to internal server error.", mlog.String("plugin_id", matched.PluginId), mlog.Int("status_code", appErr.StatusCode))
appErr.StatusCode = http.StatusInternalServerError
}
return matched.Command, response, appErr
}