From 561976bcd009b27b556d2461d39e4a89792ea94f Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Mon, 27 Feb 2023 11:03:05 +0100 Subject: [PATCH] WebClient: return proper status code for http.MaxBytesError Signed-off-by: Nicola Murino --- go.mod | 2 +- go.sum | 4 ++-- internal/httpd/api_http_user.go | 32 ++++++++++++++++---------------- internal/httpd/api_utils.go | 6 +++++- internal/httpd/internal_test.go | 3 +++ 5 files changed, 27 insertions(+), 20 deletions(-) diff --git a/go.mod b/go.mod index 278a6c59..7cf79b92 100644 --- a/go.mod +++ b/go.mod @@ -37,7 +37,7 @@ require ( github.com/hashicorp/go-retryablehttp v0.7.2 github.com/jackc/pgx/v5 v5.3.0 github.com/jlaffaye/ftp v0.0.0-20201112195030-9aae4d151126 - github.com/klauspost/compress v1.15.15 + github.com/klauspost/compress v1.16.0 github.com/lestrrat-go/jwx/v2 v2.0.8 github.com/lithammer/shortuuid/v3 v3.0.7 github.com/mattn/go-sqlite3 v1.14.16 diff --git a/go.sum b/go.sum index 03a6288c..47ec20d0 100644 --- a/go.sum +++ b/go.sum @@ -1400,8 +1400,8 @@ github.com/klauspost/compress v1.11.3/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYs github.com/klauspost/compress v1.11.13/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs= github.com/klauspost/compress v1.13.6/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk= github.com/klauspost/compress v1.15.1/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk= -github.com/klauspost/compress v1.15.15 h1:EF27CXIuDsYJ6mmvtBRlEuB2UVOqHG1tAXgZ7yIO+lw= -github.com/klauspost/compress v1.15.15/go.mod h1:ZcK2JAFqKOpnBlxcLsJzYfrS9X1akm9fHZNnD9+Vo/4= +github.com/klauspost/compress v1.16.0 h1:iULayQNOReoYUe+1qtKOqw9CwJv3aNQu8ivo7lw1HU4= +github.com/klauspost/compress v1.16.0/go.mod h1:ntbaceVETuRiXiv4DpjP66DpAtAGkEQskQzEyD//IeE= github.com/klauspost/cpuid/v2 v2.0.4/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= github.com/klauspost/cpuid/v2 v2.2.3/go.mod h1:RVVoqg1df56z8g3pUjL/3lE5UfnlrJX8tyFgg4nqhuY= github.com/klauspost/cpuid/v2 v2.2.4 h1:acbojRNwl3o09bUq+yDCtZFc1aiwaAAxtcn8YkZXnvk= diff --git a/internal/httpd/api_http_user.go b/internal/httpd/api_http_user.go index 0f14a274..45d9e73b 100644 --- a/internal/httpd/api_http_user.go +++ b/internal/httpd/api_http_user.go @@ -100,10 +100,10 @@ func createUserDir(w http.ResponseWriter, r *http.Request) { connection.User.CheckFsRoot(connection.ID) //nolint:errcheck err = connection.CreateDir(name, true) if err != nil { - sendAPIResponse(w, r, err, fmt.Sprintf("Unable to create directory %#v", name), getMappedStatusCode(err)) + sendAPIResponse(w, r, err, fmt.Sprintf("Unable to create directory %q", name), getMappedStatusCode(err)) return } - sendAPIResponse(w, r, nil, fmt.Sprintf("Directory %#v created", name), http.StatusCreated) + sendAPIResponse(w, r, nil, fmt.Sprintf("Directory %q created", name), http.StatusCreated) } func deleteUserDir(w http.ResponseWriter, r *http.Request) { @@ -192,7 +192,7 @@ func getUserFile(w http.ResponseWriter, r *http.Request) { return } if info.IsDir() { - sendAPIResponse(w, r, nil, fmt.Sprintf("Please set the path to a valid file, %#v is a directory", name), http.StatusBadRequest) + sendAPIResponse(w, r, nil, fmt.Sprintf("Please set the path to a valid file, %q is a directory", name), http.StatusBadRequest) return } @@ -239,7 +239,7 @@ func setFileDirMetadata(w http.ResponseWriter, r *http.Request) { } err = connection.SetStat(name, &attrs) if err != nil { - sendAPIResponse(w, r, err, fmt.Sprintf("Unable to set metadata for path %#v", name), getMappedStatusCode(err)) + sendAPIResponse(w, r, err, fmt.Sprintf("Unable to set metadata for path %q", name), getMappedStatusCode(err)) return } sendAPIResponse(w, r, nil, "OK", http.StatusOK) @@ -275,18 +275,18 @@ func doUploadFile(w http.ResponseWriter, r *http.Request, connection *Connection connection.User.CheckFsRoot(connection.ID) //nolint:errcheck writer, err := connection.getFileWriter(filePath) if err != nil { - sendAPIResponse(w, r, err, fmt.Sprintf("Unable to write file %#v", filePath), getMappedStatusCode(err)) + sendAPIResponse(w, r, err, fmt.Sprintf("Unable to write file %q", filePath), getMappedStatusCode(err)) return err } _, err = io.Copy(writer, r.Body) if err != nil { writer.Close() //nolint:errcheck - sendAPIResponse(w, r, err, fmt.Sprintf("Error saving file %#v", filePath), getMappedStatusCode(err)) + sendAPIResponse(w, r, err, fmt.Sprintf("Error saving file %q", filePath), getMappedStatusCode(err)) return err } err = writer.Close() if err != nil { - sendAPIResponse(w, r, err, fmt.Sprintf("Error closing file %#v", filePath), getMappedStatusCode(err)) + sendAPIResponse(w, r, err, fmt.Sprintf("Error closing file %q", filePath), getMappedStatusCode(err)) return err } setModificationTimeFromHeader(r, connection, filePath) @@ -348,7 +348,7 @@ func doUploadFiles(w http.ResponseWriter, r *http.Request, connection *Connectio for _, f := range files { file, err := f.Open() if err != nil { - sendAPIResponse(w, r, err, fmt.Sprintf("Unable to read uploaded file %#v", f.Filename), getMappedStatusCode(err)) + sendAPIResponse(w, r, err, fmt.Sprintf("Unable to read uploaded file %q", f.Filename), getMappedStatusCode(err)) return uploaded } defer file.Close() @@ -356,18 +356,18 @@ func doUploadFiles(w http.ResponseWriter, r *http.Request, connection *Connectio filePath := path.Join(parentDir, path.Base(util.CleanPath(f.Filename))) writer, err := connection.getFileWriter(filePath) if err != nil { - sendAPIResponse(w, r, err, fmt.Sprintf("Unable to write file %#v", f.Filename), getMappedStatusCode(err)) + sendAPIResponse(w, r, err, fmt.Sprintf("Unable to write file %q", f.Filename), getMappedStatusCode(err)) return uploaded } _, err = io.Copy(writer, file) if err != nil { writer.Close() //nolint:errcheck - sendAPIResponse(w, r, err, fmt.Sprintf("Error saving file %#v", f.Filename), getMappedStatusCode(err)) + sendAPIResponse(w, r, err, fmt.Sprintf("Error saving file %q", f.Filename), getMappedStatusCode(err)) return uploaded } err = writer.Close() if err != nil { - sendAPIResponse(w, r, err, fmt.Sprintf("Error closing file %#v", f.Filename), getMappedStatusCode(err)) + sendAPIResponse(w, r, err, fmt.Sprintf("Error closing file %q", f.Filename), getMappedStatusCode(err)) return uploaded } uploaded++ @@ -387,15 +387,15 @@ func deleteUserFile(w http.ResponseWriter, r *http.Request) { name := connection.User.GetCleanedPath(r.URL.Query().Get("path")) fs, p, err := connection.GetFsAndResolvedPath(name) if err != nil { - sendAPIResponse(w, r, err, fmt.Sprintf("Unable to delete file %#v", name), getMappedStatusCode(err)) + sendAPIResponse(w, r, err, fmt.Sprintf("Unable to delete file %q", name), getMappedStatusCode(err)) return } var fi os.FileInfo if fi, err = fs.Lstat(p); err != nil { - connection.Log(logger.LevelError, "failed to remove file %#v: stat error: %+v", p, err) + connection.Log(logger.LevelError, "failed to remove file %q: stat error: %+v", p, err) err = connection.GetFsError(fs, err) - sendAPIResponse(w, r, err, fmt.Sprintf("Unable to delete file %#v", name), getMappedStatusCode(err)) + sendAPIResponse(w, r, err, fmt.Sprintf("Unable to delete file %q", name), getMappedStatusCode(err)) return } @@ -409,7 +409,7 @@ func deleteUserFile(w http.ResponseWriter, r *http.Request) { sendAPIResponse(w, r, err, fmt.Sprintf("Unable to delete file %q", name), getMappedStatusCode(err)) return } - sendAPIResponse(w, r, nil, fmt.Sprintf("File %#v deleted", name), http.StatusOK) + sendAPIResponse(w, r, nil, fmt.Sprintf("File %q deleted", name), http.StatusOK) } func getUserFilesAsZipStream(w http.ResponseWriter, r *http.Request) { @@ -554,7 +554,7 @@ func setModificationTimeFromHeader(r *http.Request, c *Connection, filePath stri Mtime: util.GetTimeFromMsecSinceEpoch(mTime), } err = c.SetStat(filePath, &attrs) - c.Log(logger.LevelDebug, "requested modification time %v for file %#v, error: %v", + c.Log(logger.LevelDebug, "requested modification time %v for file %q, error: %v", attrs.Mtime, filePath, err) } else { c.Log(logger.LevelInfo, "invalid modification time header was ignored: %v", mTimeString) diff --git a/internal/httpd/api_utils.go b/internal/httpd/api_utils.go index c55707d2..f424b712 100644 --- a/internal/httpd/api_utils.go +++ b/internal/httpd/api_utils.go @@ -122,7 +122,11 @@ func getMappedStatusCode(err error) int { case errors.Is(err, common.ErrOpUnsupported): statusCode = http.StatusBadRequest default: - statusCode = http.StatusInternalServerError + if _, ok := err.(*http.MaxBytesError); ok { + statusCode = http.StatusRequestEntityTooLarge + } else { + statusCode = http.StatusInternalServerError + } } return statusCode } diff --git a/internal/httpd/internal_test.go b/internal/httpd/internal_test.go index f9015f57..6f6312d8 100644 --- a/internal/httpd/internal_test.go +++ b/internal/httpd/internal_test.go @@ -392,6 +392,9 @@ func TestMappedStatusCode(t *testing.T) { err = os.ErrClosed code = getMappedStatusCode(err) assert.Equal(t, http.StatusInternalServerError, code) + err = &http.MaxBytesError{} + code = getMappedStatusCode(err) + assert.Equal(t, http.StatusRequestEntityTooLarge, code) } func TestGCSWebInvalidFormFile(t *testing.T) {