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 diff --git a/handlers_test.go b/handlers_test.go index e0defc5..e5931d7 100644 --- a/handlers_test.go +++ b/handlers_test.go @@ -4,7 +4,6 @@ import ( "bytes" "crypto/rand" "encoding/hex" - "errors" "fmt" "io" "io/ioutil" @@ -398,7 +397,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/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 e27d56f..a030d48 100644 --- a/repo/repo.go +++ b/repo/repo.go @@ -3,6 +3,7 @@ package repo import ( "encoding/hex" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -13,6 +14,7 @@ import ( "regexp" "runtime" "strings" + "syscall" "time" "github.com/minio/sha256-simd" @@ -83,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})?$`) @@ -592,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 } } @@ -603,7 +609,23 @@ 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) { + // 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 { + // Otherwise we have a different internal error, reply with + // server-side HTTP status + h.internalServerError(w, err) + } return }