Browse Source

WebClient: return proper status code for http.MaxBytesError

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>
Nicola Murino 2 years ago
parent
commit
561976bcd0
5 changed files with 27 additions and 20 deletions
  1. 1 1
      go.mod
  2. 2 2
      go.sum
  3. 16 16
      internal/httpd/api_http_user.go
  4. 5 1
      internal/httpd/api_utils.go
  5. 3 0
      internal/httpd/internal_test.go

+ 1 - 1
go.mod

@@ -37,7 +37,7 @@ require (
 	github.com/hashicorp/go-retryablehttp v0.7.2
 	github.com/hashicorp/go-retryablehttp v0.7.2
 	github.com/jackc/pgx/v5 v5.3.0
 	github.com/jackc/pgx/v5 v5.3.0
 	github.com/jlaffaye/ftp v0.0.0-20201112195030-9aae4d151126
 	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/lestrrat-go/jwx/v2 v2.0.8
 	github.com/lithammer/shortuuid/v3 v3.0.7
 	github.com/lithammer/shortuuid/v3 v3.0.7
 	github.com/mattn/go-sqlite3 v1.14.16
 	github.com/mattn/go-sqlite3 v1.14.16

+ 2 - 2
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.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.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.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.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.3/go.mod h1:RVVoqg1df56z8g3pUjL/3lE5UfnlrJX8tyFgg4nqhuY=
 github.com/klauspost/cpuid/v2 v2.2.4 h1:acbojRNwl3o09bUq+yDCtZFc1aiwaAAxtcn8YkZXnvk=
 github.com/klauspost/cpuid/v2 v2.2.4 h1:acbojRNwl3o09bUq+yDCtZFc1aiwaAAxtcn8YkZXnvk=

+ 16 - 16
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
 	connection.User.CheckFsRoot(connection.ID) //nolint:errcheck
 	err = connection.CreateDir(name, true)
 	err = connection.CreateDir(name, true)
 	if err != nil {
 	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
 		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) {
 func deleteUserDir(w http.ResponseWriter, r *http.Request) {
@@ -192,7 +192,7 @@ func getUserFile(w http.ResponseWriter, r *http.Request) {
 		return
 		return
 	}
 	}
 	if info.IsDir() {
 	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
 		return
 	}
 	}
 
 
@@ -239,7 +239,7 @@ func setFileDirMetadata(w http.ResponseWriter, r *http.Request) {
 	}
 	}
 	err = connection.SetStat(name, &attrs)
 	err = connection.SetStat(name, &attrs)
 	if err != nil {
 	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
 		return
 	}
 	}
 	sendAPIResponse(w, r, nil, "OK", http.StatusOK)
 	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
 	connection.User.CheckFsRoot(connection.ID) //nolint:errcheck
 	writer, err := connection.getFileWriter(filePath)
 	writer, err := connection.getFileWriter(filePath)
 	if err != nil {
 	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
 		return err
 	}
 	}
 	_, err = io.Copy(writer, r.Body)
 	_, err = io.Copy(writer, r.Body)
 	if err != nil {
 	if err != nil {
 		writer.Close() //nolint:errcheck
 		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
 		return err
 	}
 	}
 	err = writer.Close()
 	err = writer.Close()
 	if err != nil {
 	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
 		return err
 	}
 	}
 	setModificationTimeFromHeader(r, connection, filePath)
 	setModificationTimeFromHeader(r, connection, filePath)
@@ -348,7 +348,7 @@ func doUploadFiles(w http.ResponseWriter, r *http.Request, connection *Connectio
 	for _, f := range files {
 	for _, f := range files {
 		file, err := f.Open()
 		file, err := f.Open()
 		if err != nil {
 		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
 			return uploaded
 		}
 		}
 		defer file.Close()
 		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)))
 		filePath := path.Join(parentDir, path.Base(util.CleanPath(f.Filename)))
 		writer, err := connection.getFileWriter(filePath)
 		writer, err := connection.getFileWriter(filePath)
 		if err != nil {
 		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
 			return uploaded
 		}
 		}
 		_, err = io.Copy(writer, file)
 		_, err = io.Copy(writer, file)
 		if err != nil {
 		if err != nil {
 			writer.Close() //nolint:errcheck
 			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
 			return uploaded
 		}
 		}
 		err = writer.Close()
 		err = writer.Close()
 		if err != nil {
 		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
 			return uploaded
 		}
 		}
 		uploaded++
 		uploaded++
@@ -387,15 +387,15 @@ func deleteUserFile(w http.ResponseWriter, r *http.Request) {
 	name := connection.User.GetCleanedPath(r.URL.Query().Get("path"))
 	name := connection.User.GetCleanedPath(r.URL.Query().Get("path"))
 	fs, p, err := connection.GetFsAndResolvedPath(name)
 	fs, p, err := connection.GetFsAndResolvedPath(name)
 	if err != nil {
 	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
 		return
 	}
 	}
 
 
 	var fi os.FileInfo
 	var fi os.FileInfo
 	if fi, err = fs.Lstat(p); err != nil {
 	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)
 		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
 		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))
 		sendAPIResponse(w, r, err, fmt.Sprintf("Unable to delete file %q", name), getMappedStatusCode(err))
 		return
 		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) {
 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),
 				Mtime: util.GetTimeFromMsecSinceEpoch(mTime),
 			}
 			}
 			err = c.SetStat(filePath, &attrs)
 			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)
 				attrs.Mtime, filePath, err)
 		} else {
 		} else {
 			c.Log(logger.LevelInfo, "invalid modification time header was ignored: %v", mTimeString)
 			c.Log(logger.LevelInfo, "invalid modification time header was ignored: %v", mTimeString)

+ 5 - 1
internal/httpd/api_utils.go

@@ -122,7 +122,11 @@ func getMappedStatusCode(err error) int {
 	case errors.Is(err, common.ErrOpUnsupported):
 	case errors.Is(err, common.ErrOpUnsupported):
 		statusCode = http.StatusBadRequest
 		statusCode = http.StatusBadRequest
 	default:
 	default:
-		statusCode = http.StatusInternalServerError
+		if _, ok := err.(*http.MaxBytesError); ok {
+			statusCode = http.StatusRequestEntityTooLarge
+		} else {
+			statusCode = http.StatusInternalServerError
+		}
 	}
 	}
 	return statusCode
 	return statusCode
 }
 }

+ 3 - 0
internal/httpd/internal_test.go

@@ -392,6 +392,9 @@ func TestMappedStatusCode(t *testing.T) {
 	err = os.ErrClosed
 	err = os.ErrClosed
 	code = getMappedStatusCode(err)
 	code = getMappedStatusCode(err)
 	assert.Equal(t, http.StatusInternalServerError, code)
 	assert.Equal(t, http.StatusInternalServerError, code)
+	err = &http.MaxBytesError{}
+	code = getMappedStatusCode(err)
+	assert.Equal(t, http.StatusRequestEntityTooLarge, code)
 }
 }
 
 
 func TestGCSWebInvalidFormFile(t *testing.T) {
 func TestGCSWebInvalidFormFile(t *testing.T) {