Skip to content

Commit

Permalink
validate put object content length (#2690)
Browse files Browse the repository at this point in the history
  • Loading branch information
lucix-aws committed Jun 19, 2024
1 parent 2d43b81 commit 2f18ba5
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 0 deletions.
8 changes: 8 additions & 0 deletions .changelog/d32ee107754148dba31cd1944f3ffcbf.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"id": "d32ee107-7541-48db-a31c-d1944f3ffcbf",
"type": "bugfix",
"description": "Add client-side validation to ensure PutObject requests have a derivable content length.",
"modules": [
"service/s3"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package software.amazon.smithy.aws.go.codegen.customization.s3;

import software.amazon.smithy.go.codegen.integration.GoIntegration;
import software.amazon.smithy.go.codegen.integration.MiddlewareRegistrar;
import software.amazon.smithy.go.codegen.integration.RuntimeClientPlugin;
import software.amazon.smithy.model.shapes.ShapeId;

import java.util.List;

import static software.amazon.smithy.go.codegen.SymbolUtils.buildPackageSymbol;

/**
* Adds validation to PutObject to ensure that content-length is derivable _somehow_ (either through the body being
* seekable or a length being set) - which is required for the operation to function since the service doesn't support
* chunked transfer-encoding.
*/
public class ValidatePutObjectContentLength implements GoIntegration {
private static final ShapeId PUT_OBJECT_SHAPE_ID = ShapeId.from("com.amazonaws.s3#PutObject");

private static final MiddlewareRegistrar MIDDLEWARE = MiddlewareRegistrar.builder()
.resolvedFunction(buildPackageSymbol("addValidatePutObjectContentLength"))
.build();

@Override
public List<RuntimeClientPlugin> getClientPlugins() {
return List.of(
RuntimeClientPlugin.builder()
.operationPredicate((model, service, operation) -> operation.getId().equals(PUT_OBJECT_SHAPE_ID))
.registerMiddleware(MIDDLEWARE)
.build()
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,4 @@ software.amazon.smithy.aws.go.codegen.customization.AccountIDEndpointRouting
software.amazon.smithy.aws.go.codegen.customization.RetryModeUserAgent
software.amazon.smithy.aws.go.codegen.customization.RequestCompressionUserAgent
software.amazon.smithy.aws.go.codegen.customization.s3.ExpressUserAgent
software.amazon.smithy.aws.go.codegen.customization.s3.ValidatePutObjectContentLength
3 changes: 3 additions & 0 deletions service/s3/api_op_PutObject.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

55 changes: 55 additions & 0 deletions service/s3/put_object_content_length.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package s3

import (
"context"
"errors"
"fmt"
"io"

presignedurl "github.com/aws/aws-sdk-go-v2/service/internal/presigned-url"
"github.com/aws/smithy-go/middleware"
)

var errNoContentLength = errors.New(
"The operation input had an undefined content length. PutObject MUST have a " +
"derivable content length from either (1) an explicit value for the " +
"ContentLength input member (2) the Body input member implementing io.Seeker " +
"such that the SDK can derive a value.",
)

// PutObject MUST have a derivable content length for the body in some form,
// since the service does not implement chunked transfer-encoding (and
// aws-chunked encoding requires the length anyway).
//
// We gate this constraint at the client level through additional validation
// rather than letting the request through, which would fail with a 501.
type validatePutObjectContentLength struct{}

func (*validatePutObjectContentLength) ID() string {
return "validatePutObjectContentLength"
}

func (*validatePutObjectContentLength) HandleInitialize(
ctx context.Context, in middleware.InitializeInput, next middleware.InitializeHandler,
) (
out middleware.InitializeOutput, metadata middleware.Metadata, err error,
) {
if presignedurl.GetIsPresigning(ctx) { // won't have a body
return next.HandleInitialize(ctx, in)
}

input, ok := in.Parameters.(*PutObjectInput)
if !ok {
return out, metadata, fmt.Errorf("unknown input parameters type %T", in.Parameters)
}

_, ok = input.Body.(io.Seeker)
if !ok && input.ContentLength == nil {
return out, metadata, errNoContentLength
}
return next.HandleInitialize(ctx, in)
}

func addValidatePutObjectContentLength(stack *middleware.Stack) error {
return stack.Initialize.Add(&validatePutObjectContentLength{}, middleware.After)
}
73 changes: 73 additions & 0 deletions service/s3/put_object_content_length_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package s3

import (
"bytes"
"context"
"errors"
"io"
"testing"

"github.com/aws/aws-sdk-go-v2/aws"
)

type reader struct {
p []byte
read bool
}

func (r *reader) Read(p []byte) (int, error) {
if r.read {
return 0, io.EOF
}

r.read = true
copy(p, r.p)
return len(r.p), nil
}

func TestValidatePutObjectContentLength(t *testing.T) {
for name, cs := range map[string]struct {
Input *PutObjectInput
ExpectErr bool
}{
"noseek,nolen": {
Input: &PutObjectInput{
Bucket: aws.String("bucket"),
Key: aws.String("key"),
Body: &reader{p: []byte("foo")},
},
ExpectErr: true,
},
"noseek,len": {
Input: &PutObjectInput{
Bucket: aws.String("bucket"),
Key: aws.String("key"),
Body: &reader{p: []byte("foo")},
ContentLength: aws.Int64(3),
},
ExpectErr: false,
},
"seek,nolen": {
Input: &PutObjectInput{
Bucket: aws.String("bucket"),
Key: aws.String("key"),
Body: bytes.NewReader([]byte("foo")),
},
ExpectErr: false,
},
} {
t.Run(name, func(t *testing.T) {
svc := New(Options{
Region: "us-east-1",
})

_, err := svc.PutObject(context.Background(), cs.Input)
if cs.ExpectErr && !errors.Is(err, errNoContentLength) {
t.Errorf("expected errNoContentLength, got %v", err)
}
if !cs.ExpectErr && errors.Is(err, errNoContentLength) {
t.Errorf("expected no errNoContentLength but got it")
}
})
}
}
1 change: 1 addition & 0 deletions service/s3/snapshot/api_op_PutObject.go.snap
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ PutObject
legacyEndpointContextSetter
S3Shared:ARNLookup
SetLogger
validatePutObjectContentLength
OperationInputValidation
Serialize stack step
putBucketContext
Expand Down

0 comments on commit 2f18ba5

Please sign in to comment.