From 2c6f74f5d004288d49c2112c0a029ee5e1c893ef Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 21 Oct 2026 18:23:42 +0100 Subject: [PATCH 1/2] feat(s3): carry an optional body on the forwarded request Hilt decides some requests on their body: the bucket policy PUT already travels with its document, and DeleteObjects needs the version permission when an entry names a version. The shared request type gains an optional body for the operations Hilt names, omitted from the encoding otherwise, so a request without one encodes as before. The codec caps it at 2 MiB; a DeleteObjects body is at most about 1.2 MiB. Co-Authored-By: Claude Fable 5.1 --- commands/s3/cbor_gen.go | 58 ++++++++++++++++++++++++++++++++++++++- commands/s3/codec_test.go | 36 ++++++++++++++++++++++++ commands/s3/json_gen.go | 45 +++++++++++++++++++++++++++++- commands/s3/types.go | 5 ++++ 4 files changed, 142 insertions(+), 2 deletions(-) diff --git a/commands/s3/cbor_gen.go b/commands/s3/cbor_gen.go index 926e231..1e1912b 100644 --- a/commands/s3/cbor_gen.go +++ b/commands/s3/cbor_gen.go @@ -27,8 +27,13 @@ func (t *Request) MarshalCBOR(w io.Writer) error { } cw := cbg.NewCborWriter(w) + fieldCount := 4 - if _, err := cw.Write([]byte{163}); err != nil { + if t.Body == nil { + fieldCount-- + } + + if _, err := cw.Write(cbg.CborEncodeMajorType(cbg.MajMap, uint64(fieldCount))); err != nil { return err } @@ -55,6 +60,34 @@ func (t *Request) MarshalCBOR(w io.Writer) error { return err } + // t.Body ([]uint8) (slice) + if t.Body != nil { + + if len("body") > 8192 { + return xerrors.Errorf("Value in field \"body\" was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("body"))); err != nil { + return err + } + if _, err := cw.WriteString(string("body")); err != nil { + return err + } + + if len(t.Body) > 2097152 { + return xerrors.Errorf("Byte array in field t.Body was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajByteString, uint64(len(t.Body))); err != nil { + return err + } + + if _, err := cw.Write(t.Body); err != nil { + return err + } + + } + // t.Method (string) (string) if len("method") > 8192 { return xerrors.Errorf("Value in field \"method\" was too long") @@ -186,6 +219,29 @@ func (t *Request) UnmarshalCBOR(r io.Reader) (err error) { t.URL = string(sval) } + // t.Body ([]uint8) (slice) + case "body": + + maj, extra, err = cr.ReadHeader() + if err != nil { + return err + } + + if extra > 2097152 { + return fmt.Errorf("t.Body: byte array too large (%d)", extra) + } + if maj != cbg.MajByteString { + return fmt.Errorf("expected byte array") + } + + if extra > 0 { + t.Body = make([]uint8, extra) + } + + if _, err := io.ReadFull(cr, t.Body); err != nil { + return err + } + // t.Method (string) (string) case "method": diff --git a/commands/s3/codec_test.go b/commands/s3/codec_test.go index 55998ee..577fefb 100644 --- a/commands/s3/codec_test.go +++ b/commands/s3/codec_test.go @@ -68,6 +68,42 @@ func TestRequestRoundTrip(t *testing.T) { } } +// TestRequestBodyRoundTrip covers the optional body: carried when set, in +// both codecs, and absent from the encoding when not, so a request without +// one encodes exactly as it did before the field existed. +func TestRequestBodyRoundTrip(t *testing.T) { + in := &s3.Request{ + Method: "POST", + Headers: map[string]string{"host": "region.s3.fil.one"}, + URL: "https://region.s3.fil.one/bucket?delete", + Body: []byte("kv"), + } + + var cb bytes.Buffer + require.NoError(t, in.MarshalCBOR(&cb)) + var outCBOR s3.Request + require.NoError(t, outCBOR.UnmarshalCBOR(bytes.NewReader(cb.Bytes()))) + require.Equal(t, *in, outCBOR) + + var jb bytes.Buffer + require.NoError(t, in.MarshalDagJSON(&jb)) + var outJSON s3.Request + require.NoError(t, outJSON.UnmarshalDagJSON(bytes.NewReader(jb.Bytes()))) + require.Equal(t, *in, outJSON) + + // Without a body neither encoding names the field. + bare := &s3.Request{Method: "GET", Headers: map[string]string{"host": "h"}, URL: "https://h/b/k"} + cb.Reset() + require.NoError(t, bare.MarshalCBOR(&cb)) + require.NotContains(t, cb.String(), "body") + jb.Reset() + require.NoError(t, bare.MarshalDagJSON(&jb)) + require.NotContains(t, jb.String(), "body") + var outBare s3.Request + require.NoError(t, outBare.UnmarshalCBOR(bytes.NewReader(cb.Bytes()))) + require.Nil(t, outBare.Body) +} + func TestAuthorizeOKRoundTrip(t *testing.T) { access := did.MustParse("did:key:z6MkjFRxLLGdBqQSLkZbVnuwUFiomK8eGBkPtim9ETvP7vec") delCid := mustCid(t, "bafyreienos3cw7hcga5vwani3pberioe2qscnz5jk2um5jajo4v7bwmvvm") diff --git a/commands/s3/json_gen.go b/commands/s3/json_gen.go index 1c56355..61ee423 100644 --- a/commands/s3/json_gen.go +++ b/commands/s3/json_gen.go @@ -31,6 +31,33 @@ func (t *Request) MarshalDagJSON(w io.Writer) error { } written := false + // t.Body ([]uint8) (slice) + if t.Body != nil { + if len("body") > 8192 { + return fmt.Errorf("string in field \"body\" was too long") + } + if err := jw.WriteString(string("body")); err != nil { + return fmt.Errorf("writing string for field \"body\": %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return err + } + if len(t.Body) > 2097152 { + return fmt.Errorf("byte array in field t.Body was too long") + } + + if err := jw.WriteBytes(t.Body); err != nil { + return fmt.Errorf("writing bytes for field t.Body: %w", err) + } + + written = true + } + if written { + if err := jw.WriteComma(); err != nil { + return err + } + } + // t.Headers (map[string]string) (map) if len("headers") > 8192 { return fmt.Errorf("string in field \"headers\" was too long") @@ -169,7 +196,23 @@ func (t *Request) UnmarshalDagJSON(r io.Reader) (err error) { } switch name { - // t.Headers (map[string]string) (map) + // t.Body ([]uint8) (slice) + case "body": + + { + bval, err := jr.ReadBytes(2097152) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading bytes for field t.Body: byte array too large") + } + return fmt.Errorf("reading bytes for field t.Body: %w", err) + } + if len(bval) > 0 { + t.Body = []uint8(bval) + } + } + + // t.Headers (map[string]string) (map) case "headers": if err := jr.ReadObjectOpen(); err != nil { return fmt.Errorf("reading object open for field t.Headers: %w", err) diff --git a/commands/s3/types.go b/commands/s3/types.go index 36f17bd..7ad1d9c 100644 --- a/commands/s3/types.go +++ b/commands/s3/types.go @@ -28,6 +28,11 @@ type Request struct { Method string `cborgen:"method" dagjsongen:"method"` Headers map[string]string `cborgen:"headers" dagjsongen:"headers"` URL string `cborgen:"url" dagjsongen:"url"` + // Body is the request body, carried only for the operations whose body + // decides what the request needs authorized (Hilt names them); every + // other request is sent without one. It is bound to the signature by + // the authorizer before it is read. + Body []byte `cborgen:"body,omitempty" dagjsongen:"body,omitempty"` } // VerificationKey is a SigV4/SigV4a derived signing key returned by Hilt so From 6f109fba74b7aa0fe4794eb8c9f84b81fccb5f5b Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 21 Oct 2026 19:05:20 +0100 Subject: [PATCH 2/2] fix(s3): size the body for the largest valid DeleteObjects document XML escaping expands a key up to sixfold, and an entry carries a version id and conditional fields beside it, so a thousand entries can run to about 7.3 MB. The field gets its own 8 MiB limit; everything else keeps the codecs' 2 MiB. Co-Authored-By: Claude Fable 5.1 --- commands/s3/cbor_gen.go | 4 ++-- commands/s3/codec_test.go | 28 ++++++++++++++++++++++++++++ commands/s3/json_gen.go | 4 ++-- commands/s3/types.go | 7 +++++-- 4 files changed, 37 insertions(+), 6 deletions(-) diff --git a/commands/s3/cbor_gen.go b/commands/s3/cbor_gen.go index 1e1912b..f2678fb 100644 --- a/commands/s3/cbor_gen.go +++ b/commands/s3/cbor_gen.go @@ -74,7 +74,7 @@ func (t *Request) MarshalCBOR(w io.Writer) error { return err } - if len(t.Body) > 2097152 { + if len(t.Body) > 8388608 { return xerrors.Errorf("Byte array in field t.Body was too long") } @@ -227,7 +227,7 @@ func (t *Request) UnmarshalCBOR(r io.Reader) (err error) { return err } - if extra > 2097152 { + if extra > 8388608 { return fmt.Errorf("t.Body: byte array too large (%d)", extra) } if maj != cbg.MajByteString { diff --git a/commands/s3/codec_test.go b/commands/s3/codec_test.go index 577fefb..b0fb422 100644 --- a/commands/s3/codec_test.go +++ b/commands/s3/codec_test.go @@ -2,6 +2,7 @@ package s3_test import ( "bytes" + "fmt" "reflect" "strings" "testing" @@ -91,6 +92,33 @@ func TestRequestBodyRoundTrip(t *testing.T) { require.NoError(t, outJSON.UnmarshalDagJSON(bytes.NewReader(jb.Bytes()))) require.Equal(t, *in, outJSON) + // The limit holds the largest valid DeleteObjects document, which XML + // escaping takes well past the codecs' usual 2 MiB: a thousand distinct + // keys of 1021 ampersands and a three-digit suffix is over 5 MB. + var big strings.Builder + big.WriteString("") + for i := range 1000 { + fmt.Fprintf(&big, "%s%03dv", strings.Repeat("&", 1021), i) + } + big.WriteString("") + require.Greater(t, big.Len(), 5_000_000) + in.Body = []byte(big.String()) + cb.Reset() + require.NoError(t, in.MarshalCBOR(&cb)) + require.NoError(t, outCBOR.UnmarshalCBOR(bytes.NewReader(cb.Bytes()))) + require.Equal(t, in.Body, outCBOR.Body) + jb.Reset() + require.NoError(t, in.MarshalDagJSON(&jb)) + require.NoError(t, outJSON.UnmarshalDagJSON(bytes.NewReader(jb.Bytes()))) + require.Equal(t, in.Body, outJSON.Body) + + // Over the limit, both encoders refuse. + in.Body = make([]byte, 8<<20+1) + cb.Reset() + require.Error(t, in.MarshalCBOR(&cb)) + jb.Reset() + require.Error(t, in.MarshalDagJSON(&jb)) + // Without a body neither encoding names the field. bare := &s3.Request{Method: "GET", Headers: map[string]string{"host": "h"}, URL: "https://h/b/k"} cb.Reset() diff --git a/commands/s3/json_gen.go b/commands/s3/json_gen.go index 61ee423..69823b7 100644 --- a/commands/s3/json_gen.go +++ b/commands/s3/json_gen.go @@ -42,7 +42,7 @@ func (t *Request) MarshalDagJSON(w io.Writer) error { if err := jw.WriteObjectColon(); err != nil { return err } - if len(t.Body) > 2097152 { + if len(t.Body) > 8388608 { return fmt.Errorf("byte array in field t.Body was too long") } @@ -200,7 +200,7 @@ func (t *Request) UnmarshalDagJSON(r io.Reader) (err error) { case "body": { - bval, err := jr.ReadBytes(2097152) + bval, err := jr.ReadBytes(8388608) if err != nil { if errors.Is(err, jsg.ErrLimitExceeded) { return fmt.Errorf("reading bytes for field t.Body: byte array too large") diff --git a/commands/s3/types.go b/commands/s3/types.go index 7ad1d9c..a3c5838 100644 --- a/commands/s3/types.go +++ b/commands/s3/types.go @@ -31,8 +31,11 @@ type Request struct { // Body is the request body, carried only for the operations whose body // decides what the request needs authorized (Hilt names them); every // other request is sent without one. It is bound to the signature by - // the authorizer before it is read. - Body []byte `cborgen:"body,omitempty" dagjsongen:"body,omitempty"` + // the authorizer before it is read. The limit holds the largest valid + // DeleteObjects document: a thousand entries of a kilobyte key expanded + // up to sixfold by XML escaping, a version id as long, and the + // conditional fields, about 7.3 MB. + Body []byte `cborgen:"body,omitempty,maxlen=8388608" dagjsongen:"body,omitempty,maxlen=8388608"` } // VerificationKey is a SigV4/SigV4a derived signing key returned by Hilt so