From 7e317c7568699ddf31a43b3177ccb04168e28b8f Mon Sep 17 00:00:00 2001 From: Ben Schumacher Date: Fri, 9 Feb 2024 17:15:03 +0100 Subject: [PATCH] [MM-56455] Handle HTTP error for too large request body in Client4 (#25842) --- server/public/model/utils.go | 26 +++++++------ server/public/model/utils_test.go | 62 +++++++++++++++++++------------ 2 files changed, 53 insertions(+), 35 deletions(-) diff --git a/server/public/model/utils.go b/server/public/model/utils.go index 9c4a81f4b7..3c5812bb21 100644 --- a/server/public/model/utils.go +++ b/server/public/model/utils.go @@ -4,6 +4,7 @@ package model import ( + "bytes" "crypto/rand" "database/sql/driver" "encoding/base32" @@ -11,7 +12,6 @@ import ( "fmt" "io" "net" - "net/http" "net/mail" "net/url" "os" @@ -336,22 +336,24 @@ func (er *AppError) Wrap(err error) *AppError { return er } -// AppErrorFromJSON will decode the input and return an AppError -func AppErrorFromJSON(data io.Reader) *AppError { - str := "" - bytes, rerr := io.ReadAll(data) - if rerr != nil { - str = rerr.Error() - } else { - str = string(bytes) +// AppErrorFromJSON will try to decode the input into an AppError. +func AppErrorFromJSON(r io.Reader) error { + data, err := io.ReadAll(r) + if err != nil { + return err } - decoder := json.NewDecoder(strings.NewReader(str)) var er AppError - err := decoder.Decode(&er) + err = json.NewDecoder(bytes.NewReader(data)).Decode(&er) if err != nil { - return NewAppError("AppErrorFromJSON", "model.utils.decode_json.app_error", nil, "body: "+str, http.StatusInternalServerError).Wrap(err) + // If the request exceeded FileSettings.MaxFileSize a plain error gets returned. Convert it into an AppError. + if string(data) == "http: request body too large\n" { + return errors.New("The request was too large. Consider asking your System Admin to raise the FileSettings.MaxFileSize setting.") + } + + return errors.Wrapf(err, "failed to decode JSON payload into AppError. Body: %s", string(data)) } + return &er } diff --git a/server/public/model/utils_test.go b/server/public/model/utils_test.go index 14d1ef819c..a556b4ea86 100644 --- a/server/public/model/utils_test.go +++ b/server/public/model/utils_test.go @@ -74,25 +74,6 @@ func TestPadDateStringZeros(t *testing.T) { } } -func TestAppError(t *testing.T) { - appErr := NewAppError("TestAppError", "message", nil, "", http.StatusInternalServerError) - json := appErr.ToJSON() - rerr := AppErrorFromJSON(strings.NewReader(json)) - require.Equal(t, appErr.Message, rerr.Message) - - t.Log(appErr.Error()) -} - -func TestAppErrorNoTranslation(t *testing.T) { - appErr := NewAppError("TestAppError", NoTranslation, nil, "test error", http.StatusBadRequest) - require.Equal(t, "TestAppError: test error", appErr.Error()) -} - -func TestAppErrorJunk(t *testing.T) { - rerr := AppErrorFromJSON(strings.NewReader("This is a broken test")) - require.Equal(t, "body: This is a broken test", rerr.DetailedError) -} - func TestAppErrorRender(t *testing.T) { t.Run("Minimal", func(t *testing.T) { aerr := NewAppError("here", "message", nil, "", http.StatusTeapot) @@ -130,13 +111,26 @@ func TestAppErrorRender(t *testing.T) { aerr := NewAppError("id", msg, nil, str, http.StatusTeapot).Wrap(errors.New(str)) assert.Len(t, aerr.Error(), maxErrorLength+len(msg)) }) + + t.Run("No Translation", func(t *testing.T) { + appErr := NewAppError("TestAppError", NoTranslation, nil, "test error", http.StatusBadRequest) + require.Equal(t, "TestAppError: test error", appErr.Error()) + }) } func TestAppErrorSerialize(t *testing.T) { + t.Run("Junk", func(t *testing.T) { + rerr := AppErrorFromJSON(strings.NewReader("This is a broken test")) + require.ErrorContains(t, rerr, "failed to decode JSON payload into AppError") + require.ErrorContains(t, rerr, "This is a broken test") + }) + t.Run("Normal", func(t *testing.T) { aerr := NewAppError("", "message", nil, "", http.StatusTeapot) js := aerr.ToJSON() - berr := AppErrorFromJSON(strings.NewReader(js)) + err := AppErrorFromJSON(strings.NewReader(js)) + berr, ok := err.(*AppError) + require.True(t, ok) require.Equal(t, "message", berr.Id) require.Empty(t, berr.DetailedError) require.Equal(t, http.StatusTeapot, berr.StatusCode) @@ -147,7 +141,9 @@ func TestAppErrorSerialize(t *testing.T) { t.Run("Detailed", func(t *testing.T) { aerr := NewAppError("", "message", nil, "detail", http.StatusTeapot) js := aerr.ToJSON() - berr := AppErrorFromJSON(strings.NewReader(js)) + err := AppErrorFromJSON(strings.NewReader(js)) + berr, ok := err.(*AppError) + require.True(t, ok) require.Equal(t, "message", berr.Id) require.Equal(t, "detail", berr.DetailedError) require.Equal(t, http.StatusTeapot, berr.StatusCode) @@ -158,7 +154,9 @@ func TestAppErrorSerialize(t *testing.T) { t.Run("Wrapped", func(t *testing.T) { aerr := NewAppError("", "message", nil, "", http.StatusTeapot).Wrap(errors.New("wrapped")) js := aerr.ToJSON() - berr := AppErrorFromJSON(strings.NewReader(js)) + err := AppErrorFromJSON(strings.NewReader(js)) + berr, ok := err.(*AppError) + require.True(t, ok) require.Equal(t, "message", berr.Id) require.Equal(t, "wrapped", berr.DetailedError) require.Equal(t, http.StatusTeapot, berr.StatusCode) @@ -169,13 +167,31 @@ func TestAppErrorSerialize(t *testing.T) { t.Run("Detailed + Wrapped", func(t *testing.T) { aerr := NewAppError("", "message", nil, "detail", http.StatusTeapot).Wrap(errors.New("wrapped")) js := aerr.ToJSON() - berr := AppErrorFromJSON(strings.NewReader(js)) + err := AppErrorFromJSON(strings.NewReader(js)) + berr, ok := err.(*AppError) + require.True(t, ok) require.Equal(t, "message", berr.Id) require.Equal(t, "detail, wrapped", berr.DetailedError) require.Equal(t, http.StatusTeapot, berr.StatusCode) require.EqualError(t, berr, aerr.Error()) }) + + t.Run("Where", func(t *testing.T) { + appErr := NewAppError("TestAppError", "message", nil, "", http.StatusInternalServerError) + json := appErr.ToJSON() + err := AppErrorFromJSON(strings.NewReader(json)) + rerr, ok := err.(*AppError) + require.True(t, ok) + require.Equal(t, appErr.Message, rerr.Message) + }) + + t.Run("Returned http.MaxBytesError", func(t *testing.T) { + aerr := (&http.MaxBytesError{}).Error() + "\n" + + err := AppErrorFromJSON(strings.NewReader(aerr)) + require.EqualError(t, err, "The request was too large. Consider asking your System Admin to raise the FileSettings.MaxFileSize setting.") + }) } func TestCopyStringMap(t *testing.T) {