Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validate put object content length #2690

Merged
merged 3 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice error message

"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
Loading