amadan

carlos / platform Public

leasetest: the fake S3 server validates PUT bodies as real S3 does

A PUT (plain, create-only or copy) whose body length differs from
Content-Length answers 400 IncompleteBody, and one whose sha256 differs
from a hex x-amz-content-sha256 answers 400 XAmzContentSHA256Mismatch.
Nothing is stored. Fixes the TestStagingMirrorPutReader flake, where a
transport handed the fake a partial body that real S3 would have refused.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 227e3a3f30ff6b7d97f7e3030dbeeab5dc5b3fa9 parent c65b318
2 files changed, +147 −5
  • internal/lease/leasetest/s3server.go +40 −5
  • internal/lease/leasetest/s3server_test.go +107 −0
diff --git a/internal/lease/leasetest/s3server.go b/internal/lease/leasetest/s3server.go
index 931926ea8..9ccf76364 100644
--- a/internal/lease/leasetest/s3server.go
+++ b/internal/lease/leasetest/s3server.go
@@ -1,6 +1,8 @@
package leasetest
import (
+ "crypto/sha256"
+ "encoding/hex"
"encoding/xml"
"errors"
"fmt"
@@ -54,6 +56,36 @@ func s3errMsg(w http.ResponseWriter, status int, code, msg string) {
io.WriteString(w, "<Error><Code>"+code+"</Code><Message>"+msg+"</Message></Error>")
}
+// readValidatedBody reads a PUT's body and checks it as real S3 does: its
+// length must equal the declared Content-Length (when one is declared), and,
+// when x-amz-content-sha256 is a hex digest, its sha256 must equal it. Any
+// other value of that header (UNSIGNED-PAYLOAD, a streaming marker) or none
+// is not checked. On a mismatch it answers 400 with S3's code and reports
+// false, and the caller stores nothing.
+func readValidatedBody(w http.ResponseWriter, r *http.Request) ([]byte, bool) {
+ body, err := io.ReadAll(r.Body)
+ if err != nil || (r.ContentLength >= 0 && int64(len(body)) != r.ContentLength) {
+ s3errMsg(w, http.StatusBadRequest, "IncompleteBody", "You did not provide the number of bytes specified by the Content-Length HTTP header.")
+ return nil, false
+ }
+ if want := r.Header.Get("x-amz-content-sha256"); isSHA256Hex(want) {
+ sum := sha256.Sum256(body)
+ if !strings.EqualFold(want, hex.EncodeToString(sum[:])) {
+ s3errMsg(w, http.StatusBadRequest, "XAmzContentSHA256Mismatch", "The provided 'x-amz-content-sha256' header does not match what was computed.")
+ return nil, false
+ }
+ }
+ return body, true
+}
+
+func isSHA256Hex(s string) bool {
+ if len(s) != 2*sha256.Size {
+ return false
+ }
+ _, err := hex.DecodeString(s)
+ return err == nil
+}
+
// copyMarkerMsg is S3's answer to a COPY naming a delete marker's version.
const copyMarkerMsg = "The source of a copy request may not specifically refer to a delete marker by version id."
@@ -97,6 +129,13 @@ func NewS3Server(f *VersionedBucket, serves ...string) (http.Handler, func() int
s3err(w, http.StatusMovedPermanently, "PermanentRedirect")
return
}
+ var body []byte
+ if r.Method == http.MethodPut { // every PUT, a copy included, is validated before it is applied
+ var ok bool
+ if body, ok = readValidatedBody(w, r); !ok {
+ return
+ }
+ }
f.mu.Lock()
defer f.mu.Unlock()
if err := f.known(bucket); err != nil {
@@ -148,12 +187,8 @@ 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, err := io.ReadAll(r.Body)
- if err != nil {
- s3err(w, http.StatusBadRequest, "IncompleteBody") // a failed upload stores nothing
- return
- }
var id string
+ var err error
if r.Header.Get("If-None-Match") == "*" {
id, err = f.putIfAbsentLocked(bucket, key, body)
} else {
diff --git a/internal/lease/leasetest/s3server_test.go b/internal/lease/leasetest/s3server_test.go
index 75663ed4c..5defbec0e 100644
--- a/internal/lease/leasetest/s3server_test.go
+++ b/internal/lease/leasetest/s3server_test.go
@@ -2,6 +2,8 @@ package leasetest
import (
"context"
+ "crypto/sha256"
+ "encoding/hex"
"errors"
"io"
"net/http"
@@ -119,3 +121,108 @@ func TestS3ServerCopyOfCurrentVersion(t *testing.T) {
t.Fatal("a failed precondition copied")
}
}
+
+// Real S3 rejects a PUT whose body is not the length the request declares,
+// or whose sha256 is not the one the request signed, and stores nothing. The
+// fake must do the same, or a client that sends a short or altered body looks
+// like it succeeded (a transport can hand the server a partial body before it
+// fails).
+func TestS3ServerValidatesPutBody(t *testing.T) {
+ sum := sha256.Sum256([]byte("abcdefghij"))
+ good := hex.EncodeToString(sum[:])
+ wrong := hex.EncodeToString(make([]byte, 32))
+ put := func(h http.Handler, key, body string, declared int64, sha string, extra map[string]string) *httptest.ResponseRecorder {
+ req := httptest.NewRequest(http.MethodPut, "/data/"+key, strings.NewReader(body))
+ req.ContentLength = declared
+ if sha != "" {
+ req.Header.Set("x-amz-content-sha256", sha)
+ }
+ for k, v := range extra {
+ req.Header.Set(k, v)
+ }
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ return rec
+ }
+ for _, c := range []struct {
+ name string
+ body string
+ declared int64
+ sha string
+ code string // empty: accepted
+ }{
+ {"exact", "abcdefghij", 10, good, ""},
+ {"unsigned payload", "abcdefghij", 10, "UNSIGNED-PAYLOAD", ""},
+ {"no sha header", "abcdefghij", 10, "", ""},
+ {"unknown length", "abcdefghij", -1, good, ""},
+ {"short body", "abc", 10, good, "IncompleteBody"},
+ {"short body unsigned", "abc", 10, "UNSIGNED-PAYLOAD", "IncompleteBody"},
+ {"long body", "abcdefghijkl", 10, "", "IncompleteBody"},
+ {"wrong sha", "abcdefghij", 10, wrong, "XAmzContentSHA256Mismatch"},
+ {"altered body", "abcdefghiX", 10, good, "XAmzContentSHA256Mismatch"},
+ } {
+ for _, createOnly := range []bool{false, true} {
+ m := NewVersionedBucket()
+ m.AddBucket("data")
+ h, _ := NewS3Server(m)
+ extra := map[string]string{}
+ if createOnly {
+ extra["If-None-Match"] = "*"
+ }
+ rec := put(h, "k", c.body, c.declared, c.sha, extra)
+ _, stored := m.Current("data", "k")
+ if c.code == "" {
+ if rec.Code != 200 || !stored {
+ t.Errorf("%s (create-only %v): %d, stored %v", c.name, createOnly, rec.Code, stored)
+ }
+ continue
+ }
+ if rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), "<Code>"+c.code+"</Code>") {
+ t.Errorf("%s (create-only %v): %d %q, want 400 %s", c.name, createOnly, rec.Code, rec.Body.String(), c.code)
+ }
+ if stored {
+ t.Errorf("%s (create-only %v): a rejected PUT stored the body", c.name, createOnly)
+ }
+ }
+ }
+}
+
+// A copy carries no body: its Content-Length must be what arrives and its
+// signed sha256 (when a digest) the hash of nothing. A copy that fails the
+// check stores nothing.
+func TestS3ServerValidatesCopyBody(t *testing.T) {
+ empty := sha256.Sum256(nil)
+ m := NewVersionedBucket()
+ m.AddBucket("src")
+ m.AddBucket("dst")
+ m.Put("src", "a", []byte("body"))
+ h, _ := NewS3Server(m)
+ copyWith := func(dst, body string, declared int64, sha string) *httptest.ResponseRecorder {
+ req := httptest.NewRequest(http.MethodPut, "/dst/"+dst, strings.NewReader(body))
+ req.ContentLength = declared
+ req.Header.Set("x-amz-copy-source", "/src/a")
+ if sha != "" {
+ req.Header.Set("x-amz-content-sha256", sha)
+ }
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ return rec
+ }
+ if rec := copyWith("ok", "", 0, hex.EncodeToString(empty[:])); rec.Code != 200 {
+ t.Fatalf("a well-formed copy: %d %q", rec.Code, rec.Body.String())
+ }
+ if rec := copyWith("ok2", "", 0, "UNSIGNED-PAYLOAD"); rec.Code != 200 {
+ t.Fatalf("an unsigned-payload copy: %d %q", rec.Code, rec.Body.String())
+ }
+ if rec := copyWith("long", "xyz", 0, ""); rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), "<Code>IncompleteBody</Code>") {
+ t.Fatalf("a copy with a body longer than declared: %d %q", rec.Code, rec.Body.String())
+ }
+ if rec := copyWith("sha", "", 0, hex.EncodeToString(make([]byte, 32))); rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), "<Code>XAmzContentSHA256Mismatch</Code>") {
+ t.Fatalf("a copy with the wrong sha256: %d %q", rec.Code, rec.Body.String())
+ }
+ for _, k := range []string{"long", "sha"} {
+ if _, ok := m.Current("dst", k); ok {
+ t.Fatalf("a rejected copy stored %q", k)
+ }
+ }
+}