From 173bfb537146e2a44e26c1302bf41319b530d59a Mon Sep 17 00:00:00 2001 From: Enrico204 Date: Sat, 21 Aug 2021 13:43:51 +0200 Subject: [PATCH 1/3] Reply "insufficient storage" on disk full or over-quota This commit will change the current behavior on disk-related errors: * HTTP 507 "Insufficient storage" is the status on disk full or over-quota * HTTP 500 "Internal server error" on other disk-related errors previously both were 400 "Bad request" --- quota/quota.go | 2 +- repo/repo.go | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/quota/quota.go b/quota/quota.go index c36244e..0b1020e 100644 --- a/quota/quota.go +++ b/quota/quota.go @@ -75,7 +75,7 @@ func (m *Manager) WrapWriter(req *http.Request, w io.Writer) (io.Writer, int, er if currentSize+contentLen > m.maxRepoSize { err := fmt.Errorf("incoming blob (%d bytes) would exceed maximum size of repository (%d bytes)", contentLen, m.maxRepoSize) - return nil, http.StatusRequestEntityTooLarge, err + return nil, http.StatusInsufficientStorage, err } } diff --git a/repo/repo.go b/repo/repo.go index 01fe1d8..e8b4b5c 100644 --- a/repo/repo.go +++ b/repo/repo.go @@ -4,6 +4,7 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -14,6 +15,7 @@ import ( "regexp" "runtime" "strings" + "syscall" "time" "github.com/miolini/datacounter" @@ -603,7 +605,12 @@ func (h *Handler) saveBlob(w http.ResponseWriter, r *http.Request) { if h.opt.Debug { log.Print(err) } - httpDefaultError(w, http.StatusBadRequest) + var pathError *os.PathError + if errors.As(err, &pathError) && (pathError.Err == syscall.ENOSPC || pathError.Err == syscall.EDQUOT) { + httpDefaultError(w, http.StatusInsufficientStorage) + } else { + httpDefaultError(w, http.StatusInternalServerError) + } return } From 9b31f17188190b435ebb3595f286bae78e9f582b Mon Sep 17 00:00:00 2001 From: Enrico204 Date: Mon, 23 Aug 2021 12:50:08 +0200 Subject: [PATCH 2/3] Add unreleased changelog entry for pull request 160 --- changelog/unreleased/pull-160 | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 changelog/unreleased/pull-160 diff --git a/changelog/unreleased/pull-160 b/changelog/unreleased/pull-160 new file mode 100644 index 0000000..cc413f6 --- /dev/null +++ b/changelog/unreleased/pull-160 @@ -0,0 +1,13 @@ +Bugfix: Reply "insufficient storage" on disk full or over-quota + +When there was no space left on disk, or any other write-related error, +rest-server was replying with HTTP status code 400 (Bad request). This is +misleading (restic client will dump the status code to the user). + +This has been fixed changing the behaviour so two different statuses are used: +* HTTP 507 "Insufficient storage" is the status on disk full or repository + over-quota +* HTTP 500 "Internal server error" on other disk-related errors + +https://github.com/restic/rest-server/issues/155 +https://github.com/restic/rest-server/pull/160 \ No newline at end of file From fb5d63435afd7db8b4fd03e9f4be5fbc6e593371 Mon Sep 17 00:00:00 2001 From: Enrico204 Date: Mon, 6 Sep 2021 22:13:33 +0200 Subject: [PATCH 3/3] Fix tests for: reply "insufficient storage" on disk full or over-quota --- handlers_test.go | 3 +-- repo/repo.go | 21 ++++++++++++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/handlers_test.go b/handlers_test.go index 3471a79..8da45af 100644 --- a/handlers_test.go +++ b/handlers_test.go @@ -5,7 +5,6 @@ import ( "crypto/rand" "crypto/sha256" "encoding/hex" - "errors" "fmt" "io" "io/ioutil" @@ -397,7 +396,7 @@ func TestAbortedRequest(t *testing.T) { // the first request is an upload to a file which blocks while reading the // body and then after some data returns an error - rd := newDelayedErrorReader(errors.New("injected")) + rd := newDelayedErrorReader(io.ErrUnexpectedEOF) wg.Add(1) go func() { diff --git a/repo/repo.go b/repo/repo.go index e8b4b5c..b80278f 100644 --- a/repo/repo.go +++ b/repo/repo.go @@ -85,6 +85,10 @@ func httpMethodNotAllowed(w http.ResponseWriter, allowed []string) { httpDefaultError(w, http.StatusMethodNotAllowed) } +// errFileContentDoesntMatchHash is the error raised when the file content hash +// doesn't match the hash provided in the URL +var errFileContentDoesntMatchHash = errors.New("file content does not match hash") + // BlobPathRE matches valid blob URI paths with optional object IDs var BlobPathRE = regexp.MustCompile(`^/(data|index|keys|locks|snapshots)/([0-9a-f]{64})?$`) @@ -594,7 +598,7 @@ func (h *Handler) saveBlob(w http.ResponseWriter, r *http.Request) { // reject if file content doesn't match file name if err == nil && hex.EncodeToString(hasher.Sum(nil)) != objectID { - err = fmt.Errorf("file content does not match hash") + err = errFileContentDoesntMatchHash } } @@ -606,10 +610,21 @@ func (h *Handler) saveBlob(w http.ResponseWriter, r *http.Request) { log.Print(err) } var pathError *os.PathError - if errors.As(err, &pathError) && (pathError.Err == syscall.ENOSPC || pathError.Err == syscall.EDQUOT) { + if errors.As(err, &pathError) && (pathError.Err == syscall.ENOSPC || + pathError.Err == syscall.EDQUOT) { + // The error is disk-related (no space left, no quota left), + // notify the client using the correct HTTP status httpDefaultError(w, http.StatusInsufficientStorage) + } else if errors.Is(err, errFileContentDoesntMatchHash) || + errors.Is(err, io.ErrUnexpectedEOF) || + errors.Is(err, http.ErrMissingBoundary) || + errors.Is(err, http.ErrNotMultipart) { + // The error is connection-related, send a client-side HTTP status + httpDefaultError(w, http.StatusBadRequest) } else { - httpDefaultError(w, http.StatusInternalServerError) + // Otherwise we have a different internal error, reply with + // server-side HTTP status + h.internalServerError(w, err) } return }