amadan

carlos / platform Public

leasetest: a failed upload stores nothing; a failed listing returns no entries (Plan 2 review)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 71ef0c261ad01f2cf6b1e8d9a9eeb87920a620fe parent 04df4e3
3 files changed, +45 −3
  • internal/lease/leasetest/contract.go +2 −2
  • internal/lease/leasetest/s3server.go +5 −1
  • internal/lease/leasetest/s3server_test.go +38 −0

CI passed — run details

diff --git a/internal/lease/leasetest/contract.go b/internal/lease/leasetest/contract.go
index 26a8847ca..d1a823408 100644
--- a/internal/lease/leasetest/contract.go
+++ b/internal/lease/leasetest/contract.go
@@ -70,8 +70,8 @@ func RunVersionedStoreContract(t *testing.T, mk Maker) {
m.Put(b, fmt.Sprintf("p/k%d", i), []byte{byte(i)})
}
m.FailListAfterPages = 2
- if vs, err := st.ListVersions(ctx, b, "p/"); err == nil {
- t.Fatalf("got %d entries and no error", len(vs))
+ if vs, err := st.ListVersions(ctx, b, "p/"); err == nil || vs != nil {
+ t.Fatalf("got %d entries and error %v; want no entries and an error", len(vs), err)
}
})
diff --git a/internal/lease/leasetest/s3server.go b/internal/lease/leasetest/s3server.go
index 8a2cefd62..f0877aeb5 100644
--- a/internal/lease/leasetest/s3server.go
+++ b/internal/lease/leasetest/s3server.go
@@ -114,7 +114,11 @@ func NewS3Server(f *VersionedBucket, serves ...string) (http.Handler, func() int
case r.Method == http.MethodPut && r.Header.Get("x-amz-copy-source") != "":
f.serveCopy(w, r, bucket, key)
case r.Method == http.MethodPut:
- body, _ := io.ReadAll(r.Body)
+ body, err := io.ReadAll(r.Body)
+ if err != nil {
+ s3err(w, http.StatusBadRequest, "IncompleteBody") // a failed upload stores nothing
+ return
+ }
if r.Header.Get("If-None-Match") != "*" {
s3err(w, http.StatusNotImplemented, "NotImplemented") // VersionedStore only puts create-only
return
diff --git a/internal/lease/leasetest/s3server_test.go b/internal/lease/leasetest/s3server_test.go
new file mode 100644
index 000000000..7ea3b6785
--- /dev/null
+++ b/internal/lease/leasetest/s3server_test.go
@@ -0,0 +1,38 @@
+package leasetest
+
+import (
+ "errors"
+ "io"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+)
+
+// failingReader yields some bytes and then an error, like a dropped upload.
+type failingReader struct{ r io.Reader }
+
+func (f *failingReader) Read(p []byte) (int, error) {
+ n, err := f.r.Read(p)
+ if err == io.EOF {
+ return n, errors.New("connection reset")
+ }
+ return n, err
+}
+
+func TestS3ServerFailedUploadStoresNothing(t *testing.T) {
+ m := NewVersionedBucket()
+ m.AddBucket("data")
+ h, _ := NewS3Server(m)
+ before := m.Snapshot()
+ req := httptest.NewRequest(http.MethodPut, "/data/k", &failingReader{strings.NewReader("partial")})
+ req.Header.Set("If-None-Match", "*")
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ if rec.Code < 400 {
+ t.Fatalf("failed upload answered %d", rec.Code)
+ }
+ if m.Snapshot() != before {
+ t.Fatalf("failed upload changed the bucket:\n%s", m.Snapshot())
+ }
+}