mirrors/versitygw
1
0
Fork 0
mirror of https://github.com/versity/versitygw.git synced 2026-09-25 09:24:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2420 from versity/ben/mpu-contentlength

Browse Source 
fix: use content-legnth for PutObject and UploadPart allocation sizes
This commit is contained in:
Ben McClelland
2026-09-21 09:16:14 -07:00
committed by GitHub
parent 38f12c85fa 20a5778aac
commit e2b7b33bac
4 changed files with 170 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
s3api/controllers/object-put.go
+28 -34
View File
@@ -246,12 +246,7 @@ func (c S3ApiController) UploadPart(ctx fiber.Ctx) (*Response, error) {
if contentLengthStr == "" {
contentLengthStr = "0"
}
// Use decoded content length if available because the
// middleware will decode the chunked transfer encoding
decodedLength := ctx.Get("X-Amz-Decoded-Content-Length")
if decodedLength != "" {
contentLengthStr = decodedLength
}
err := c.verifyAccess(ctx,
auth.AccessOptions{
@@ -280,7 +275,6 @@ func (c S3ApiController) UploadPart(ctx fiber.Ctx) (*Response, error) {
},
}, s3err.GetInvalidArgumentErr(s3err.InvalidArgPartNumber, ctx.Query("partNumber"))
}
contentLength, err := strconv.ParseInt(contentLengthStr, 10, 64)
if err != nil {
debuglogger.Logf("error parsing content length %q: %v", contentLengthStr, err)
@@ -308,20 +302,23 @@ func (c S3ApiController) UploadPart(ctx fiber.Ctx) (*Response, error) {
} else {
body = bytes.NewReader([]byte{})
}
_, chunked := body.(middlewares.ChecksumReader)
if chunked && decodedLength != "" {
contentLength, err = strconv.ParseInt(decodedLength, 10, 64)
if err != nil {
debuglogger.Logf("error parsing content length %q: %v", decodedLength, err)
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, s3err.GetAPIError(s3err.ErrInvalidRequest)
}
}
// aws-chunked bodies are framed and length-checked by the chunk readers
// (the ones implementing middlewares.ChecksumReader). A plain body has
// nothing but Content-Length to tell a finished upload from an aborted one.
//
// Use the raw Content-Length header, not contentLength: that variable may
// have been replaced by X-Amz-Decoded-Content-Length above, which describes
// the DECODED size. That header only applies to aws-chunked payloads, and
// those skip this wrapper anyway. AWS S3 ignores it on a plain body and
// stores Content-Length bytes, so checking against the decoded value would
// reject a complete upload.
if _, chunked := body.(middlewares.ChecksumReader); !chunked {
if raw, cerr := strconv.ParseInt(ctx.Get("Content-Length"), 10, 64); cerr == nil && raw > 0 {
body = utils.NewContentLengthReader(body, raw)
}
if !chunked && contentLength > 0 {
body = utils.NewContentLengthReader(body, contentLength)
}
res, err := c.be.UploadPart(ctx.RequestCtx(),
@@ -723,12 +720,7 @@ func (c S3ApiController) PutObject(ctx fiber.Ctx) (*Response, error) {
if contentLengthStr == "" {
contentLengthStr = "0"
}
// Use decoded content length if available because the
// middleware will decode the chunked transfer encoding
decodedLength := ctx.Get("X-Amz-Decoded-Content-Length")
if decodedLength != "" {
contentLengthStr = decodedLength
}
actions := []auth.Action{auth.PutObjectAction}
if tagging != "" {
@@ -787,7 +779,6 @@ func (c S3ApiController) PutObject(ctx fiber.Ctx) (*Response, error) {
},
}, err
}
contentLength, err := strconv.ParseInt(contentLengthStr, 10, 64)
if err != nil {
debuglogger.Logf("error parsing content length %q: %v", contentLengthStr, err)
@@ -823,20 +814,23 @@ func (c S3ApiController) PutObject(ctx fiber.Ctx) (*Response, error) {
} else {
body = bytes.NewReader([]byte{})
}
_, chunked := body.(middlewares.ChecksumReader)
if chunked && decodedLength != "" {
contentLength, err = strconv.ParseInt(decodedLength, 10, 64)
if err != nil {
debuglogger.Logf("error parsing content length %q: %v", decodedLength, err)
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, s3err.GetAPIError(s3err.ErrInvalidRequest)
}
}
// aws-chunked bodies are framed and length-checked by the chunk readers
// (the ones implementing middlewares.ChecksumReader). A plain body has
// nothing but Content-Length to tell a finished upload from an aborted one.
//
// Use the raw Content-Length header, not contentLength: that variable may
// have been replaced by X-Amz-Decoded-Content-Length above, which describes
// the DECODED size. That header only applies to aws-chunked payloads, and
// those skip this wrapper anyway. AWS S3 ignores it on a plain body and
// stores Content-Length bytes, so checking against the decoded value would
// reject a complete upload.
if _, chunked := body.(middlewares.ChecksumReader); !chunked {
if raw, cerr := strconv.ParseInt(ctx.Get("Content-Length"), 10, 64); cerr == nil && raw > 0 {
body = utils.NewContentLengthReader(body, raw)
}
if !chunked && contentLength > 0 {
body = utils.NewContentLengthReader(body, contentLength)
}
ifMatch, ifNoneMatch := utils.ParsePreconditionMatchHeaders(ctx)
s3api/controllers/object-put_test.go
+66 -2
View File
@@ -17,6 +17,7 @@ package controllers
import (
"context"
"encoding/xml"
"io"
"strings"
"testing"
"time"
@@ -32,6 +33,13 @@ import (
"github.com/versity/versitygw/s3response"
)
type checksumBodyReader struct {
io.Reader
}
func (checksumBodyReader) Algorithm() string { return "" }
func (checksumBodyReader) Checksum() string { return "" }
func TestS3ApiController_PutObjectTagging(t *testing.T) {
validTaggingBody, err := xml.Marshal(
s3response.Tagging{
@@ -496,7 +504,12 @@ func TestS3ApiController_UploadPart(t *testing.T) {
{
name: "invalid content length",
input: testInput{
locals: defaultLocals,
locals: map[utils.ContextKey]any{
utils.ContextKeyIsRoot: true,
utils.ContextKeyParsedAcl: auth.ACL{Owner: "root"},
utils.ContextKeyAccount: auth.Account{Access: "root", Role: auth.RoleAdmin},
utils.ContextKeyBodyReader: checksumBodyReader{strings.NewReader("")},
},
headers: map[string]string{
"X-Amz-Decoded-Content-Length": "invalid_cLength",
},
@@ -631,6 +644,52 @@ func TestS3ApiController_UploadPart(t *testing.T) {
}
}
func TestS3ApiController_UploadPartPlainBodyUsesContentLength(t *testing.T) {
be := &BackendMock{
UploadPartFunc: func(_ context.Context, input *s3.UploadPartInput) (*s3.UploadPartOutput, error) {
assert.Equal(t, int64(5), *input.ContentLength)
body, err := io.ReadAll(input.Body)
assert.NoError(t, err)
assert.Equal(t, "hello", string(body))
return &s3.UploadPartOutput{}, nil
},
GetBucketPolicyFunc: func(_ context.Context, _ string) ([]byte, error) {
return nil, s3err.GetAPIError(s3err.ErrAccessDenied)
},
}
ctrl := S3ApiController{be: be, mpMaxParts: 10000}
testController(t, ctrl.UploadPart, &Response{
Headers: map[string]*string{
"ETag": nil,
"x-amz-checksum-crc32": nil,
"x-amz-checksum-crc32c": nil,
"x-amz-checksum-crc64nvme": nil,
"x-amz-checksum-sha1": nil,
"x-amz-checksum-sha256": nil,
"x-amz-checksum-sha512": nil,
"x-amz-checksum-md5": nil,
"x-amz-checksum-xxhash64": nil,
"x-amz-checksum-xxhash3": nil,
"x-amz-checksum-xxhash128": nil,
},
MetaOpts: &MetaOptions{BucketOwner: "root", ContentLength: 5},
}, nil, ctxInputs{
body: []byte("hello"),
locals: map[utils.ContextKey]any{
utils.ContextKeyIsRoot: true,
utils.ContextKeyParsedAcl: auth.ACL{Owner: "root"},
utils.ContextKeyAccount: auth.Account{Access: "root", Role: auth.RoleAdmin},
utils.ContextKeyBodyReader: strings.NewReader("hello"),
},
headers: map[string]string{
"Content-Length": "5",
"X-Amz-Decoded-Content-Length": "99999",
},
queries: map[string]string{"partNumber": "1", "uploadId": "upload"},
})
}
func TestS3ApiController_UploadPartCopy(t *testing.T) {
tests := []struct {
name string
@@ -1309,7 +1368,12 @@ func TestS3ApiController_PutObject(t *testing.T) {
{
name: "invalid content length",
input: testInput{
locals: defaultLocals,
locals: map[utils.ContextKey]any{
utils.ContextKeyIsRoot: true,
utils.ContextKeyParsedAcl: auth.ACL{Owner: "root"},
utils.ContextKeyAccount: auth.Account{Access: "root", Role: auth.RoleAdmin},
utils.ContextKeyBodyReader: checksumBodyReader{strings.NewReader("")},
},
extraMockErr: s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotFound),
headers: map[string]string{
"X-Amz-Decoded-Content-Length": "invalid_length",
tests/integration/UploadPart.go
+74
View File
@@ -24,9 +24,12 @@ import (
"hash/crc32"
"hash/crc64"
"math/bits"
"net/http"
"strings"
"time"
"github.com/aws/aws-sdk-go-v2/aws"
v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4"
"github.com/aws/aws-sdk-go-v2/service/s3"
"github.com/aws/aws-sdk-go-v2/service/s3/types"
"github.com/versity/versitygw/s3err"
@@ -502,6 +505,77 @@ func UploadPart_success(s *S3Conf) error {
})
}
// UploadPart_plain_body_with_decoded_length checks that a complete plain part
// uses Content-Length rather than X-Amz-Decoded-Content-Length. The latter is
// only meaningful for aws-chunked uploads and must not preallocate a zero tail.
func UploadPart_plain_body_with_decoded_length(s *S3Conf) error {
testName := "UploadPart_plain_body_with_decoded_length"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "plain-part-with-decoded-length"
upload, err := createMp(s3client, bucket, obj)
if err != nil {
return err
}
data := "hello"
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
req, err := http.NewRequestWithContext(ctx, http.MethodPut,
fmt.Sprintf("%s/%s/%s?uploadId=%s&partNumber=1", s.endpoint, bucket, obj, *upload.UploadId),
strings.NewReader(data))
if err != nil {
cancel()
return fmt.Errorf("create upload part request: %w", err)
}
req.Header.Set("x-amz-content-sha256", "UNSIGNED-PAYLOAD")
req.Header.Set("X-Amz-Decoded-Content-Length", "99999")
signer := v4.NewSigner()
if err := signer.SignHTTP(req.Context(),
aws.Credentials{AccessKeyID: s.awsID, SecretAccessKey: s.awsSecret},
req, "UNSIGNED-PAYLOAD", "s3", s.awsRegion, time.Now()); err != nil {
cancel()
return fmt.Errorf("sign upload part request: %w", err)
}
resp, err := s.httpClient.Do(req)
cancel()
if err != nil {
return fmt.Errorf("send upload part request: %w", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
return fmt.Errorf("expected upload part status %v, got %v", http.StatusOK, resp.StatusCode)
}
partNumber := int32(1)
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.CompleteMultipartUpload(ctx, &s3.CompleteMultipartUploadInput{
Bucket: &bucket,
Key: &obj,
UploadId: upload.UploadId,
MultipartUpload: &types.CompletedMultipartUpload{Parts: []types.CompletedPart{{
ETag: aws.String(resp.Header.Get("ETag")),
PartNumber: &partNumber,
}}},
})
cancel()
if err != nil {
return fmt.Errorf("complete multipart upload: %w", err)
}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
out, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{Bucket: &bucket, Key: &obj})
cancel()
if err != nil {
return err
}
if out.ContentLength == nil || *out.ContentLength != int64(len(data)) {
return fmt.Errorf("expected completed object to be %v bytes, got %v", len(data), out.ContentLength)
}
return nil
})
}
func UploadPart_data_integrity_etag(s *S3Conf) error {
testName := "UploadPart_data_integrity_etag"
partNumber := int32(1)
tests/integration/group-tests.go
+2
View File
@@ -474,6 +474,7 @@ func TestUploadPart(ts *TestState) {
ts.Run(UploadPart_with_checksums_success)
}
ts.Run(UploadPart_success)
ts.Run(UploadPart_plain_body_with_decoded_length)
ts.Run(UploadPart_etag_quoting_consistency)
}
@@ -2873,6 +2874,7 @@ func GetIntTests() IntTests {
"PutObject_special_chars": PutObject_special_chars,
"PutObject_aborted_plain_body": PutObject_aborted_plain_body,
"PutObject_plain_body_with_decoded_length": PutObject_plain_body_with_decoded_length,
"UploadPart_plain_body_with_decoded_length": UploadPart_plain_body_with_decoded_length,
"PutObject_tagging": PutObject_tagging,
"PutObject_success": PutObject_success,
"PutObject_default_content_type": PutObject_default_content_type,