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

PutObject does not add Content-Length in the headers when it is specified in the parameters and the body is not Seekable #2511

Closed
riftsin opened this issue Feb 21, 2024 · 3 comments

Comments

@riftsin
Copy link

riftsin commented Feb 21, 2024

Describe the bug

Hello,

When you do a PutObject like this:

buf := []byte{"just some random data"}
r, w := io.Pipe()
defer w.Close()
go func() {
                defer r.Close()
                _, err := s3Client.PutObject(context.TODO(), &s3.PutObjectInput{
                    Bucket:        aws.String("bucket"),
                    Key:           aws.String("screenshot.png"),
                    Body:          r,
                    ContentLength: aws.Int64(len(buf)),
                },
                    s3Client.optionFunc,
                    s3.WithAPIOptions(v4.SwapComputePayloadSHA256ForUnsignedPayloadMiddleware),
                )
                if err != nil {
                  log.Println(err)
                }
}()
io.Copy(w, bytes.NewReader(buf))

You get a SignatureDoesMatchError.

The reason for this is that the header Content-Length is not set in the Request as you can see in the following logs (even though it is set in the canonical request)

---[ CANONICAL STRING  ]-----------------------------
PUT
/bucket/screenshot.png
x-id=PutObject
accept-encoding:identity
amz-sdk-invocation-id:471a9f15-9cbc-4ade-a909-e15f847a4741
amz-sdk-request:attempt=1; max=1
content-length:172595
content-type:application/octet-stream
host:s3.eu-west-3.amazonaws.com
x-amz-content-sha256:UNSIGNED-PAYLOAD
x-amz-date:20240221T103126Z

accept-encoding;amz-sdk-invocation-id;amz-sdk-request;content-length;content-type;host;x-amz-content-sha256;x-amz-date
UNSIGNED-PAYLOAD
---[ STRING TO SIGN ]--------------------------------
AWS4-HMAC-SHA256
20240221T103126Z
20240221/eu-west-3/s3/aws4_request
3060f414e9bfb0d526ba52e0e5c5f889f37bb61005017d58804be9d38d0b657c
-----------------------------------------------------
SDK 2024/02/21 11:31:26 DEBUG Request
PUT /bucket/screenshot.png?x-id=PutObject HTTP/1.1
Host: s3.eu-west-3.amazonaws.com
User-Agent: aws-sdk-go-v2/1.21.2 os/macos lang/go#1.22.0 md/GOOS#darwin md/GOARCH#arm64 api/s3#1.38.5
Transfer-Encoding: chunked
Accept-Encoding: identity
Amz-Sdk-Invocation-Id: 471a9f15-9cbc-4ade-a909-e15f847a4741
Amz-Sdk-Request: attempt=1; max=1
Authorization: AWS4-HMAC-SHA256 Credential=AKIA***/20240221/eu-west-3/s3/aws4_request, SignedHeaders=accept-encoding;amz-sdk-invocation-id;amz-sdk-request;content-length;content-type;host;x-amz-content-sha256;x-amz-date, Signature=e0ebe472d82ea11f46d2dd02c74055f5ec81e9ddf66ca69cd4211248649c3a31
Content-Type: application/octet-stream
X-Amz-Content-Sha256: UNSIGNED-PAYLOAD
X-Amz-Date: 20240221T103126Z

Expected Behavior

I would expect the content length to be set when I specifiy it using the Content-Length parameter of PutObject.

Current Behavior

The Content-Length header is not set and therefore the request fails with a SignatureDoesNotMatch error.

Reproduction Steps

Just use the code in the example (left out the boiler plate)

Possible Solution

No response

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

In the example this was used

    github.com/aws/aws-sdk-go-v2 v1.21.2
    github.com/aws/aws-sdk-go-v2/config v1.18.38
    github.com/aws/aws-sdk-go-v2/credentials v1.13.36
    github.com/aws/aws-sdk-go-v2/service/s3 v1.38.5

However I have tested it with aws-sdk-go-v2 v1.25.0 and I get the same behavior

Compiler and Version used

1.22 darwin/arm64

Operating System and version

MacOS Ventura

@riftsin riftsin added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 21, 2024
@lucix-aws
Copy link
Contributor

lucix-aws commented Feb 21, 2024

PutObject does not add Content-Length in the headers when it is specified in the parameters and the body is not Seekable

I don't think this is the case. Consider the following snippet, built with the express purpose of testing this claim (based heavily on what you provided, but removing the asynchronous aspect):

package main

import (
	"context"
	"io"
	"log"

	"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/config"
	"github.com/aws/aws-sdk-go-v2/service/s3"
)

type reader string

var _ io.Reader = reader("")

func (r reader) Read(p []byte) (int, error) {
	copy(p, r)
	return len(r), nil
}

func main() {
	cfg, err := config.LoadDefaultConfig(context.Background())
	if err != nil {
		log.Fatal(err)
	}

	s3Client := s3.NewFromConfig(cfg, func(o *s3.Options) {
		o.ClientLogMode = aws.LogRequest | aws.LogResponse
	})

	str := reader("just some random data")
	_, err = s3Client.PutObject(context.TODO(), &s3.PutObjectInput{
		Bucket:        aws.String("<bucket>"),
		Key:           aws.String("screenshot.png"),
		Body:          str,
		ContentLength: aws.Int64(int64(len(str))),
	}, s3.WithAPIOptions(v4.SwapComputePayloadSHA256ForUnsignedPayloadMiddleware))
	if err != nil {
		log.Println(err)
	}
}

I'm seeing this upload successfully (with the content-length header included in the request) against the latest modules.

I have a few followup asks, related to your snippet as posted:

1. We can't see the source of s3Client.optionFunc, which may very well matter. Given its unexportedness I assume you've wrapped our client with your own structure that has this field.
2. You mentioned you removed some boilerplate, but your snippet as it's presented here isn't really functional because the inner goroutine that calls the operation is basically guaranteed to get dropped before it completes, since the outer main is going to return first. I can only assume there's other things going on in your application surrounding this, which are almost certainly going to be relevant as far as root-causing this is concerned.

@lucix-aws lucix-aws added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Feb 21, 2024
@lucix-aws
Copy link
Contributor

lucix-aws commented Feb 21, 2024

I missed the chunked Transfer-Encoding on the first pass. With some basic WaitGroup ops surrounding your snippet I can reproduce this.

We appear to have a special case internally for io.PipeReader that is causing us to send invalid signatures with this request configuration.

Since we've demonstrated that content-length + non-seekable body does work in the general sense, I'm going to close this issue and open a new one to track the PipeReader issue separately.

@lucix-aws lucix-aws removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 21, 2024
Copy link

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants