aws-sdk-go-v2 icon indicating copy to clipboard operation
aws-sdk-go-v2 copied to clipboard

Presigned URL for PUT with ContentType doesn't include ContentType in the signed headers

Open vangent opened this issue 2 years ago • 1 comments

Confirm by changing [ ] to [x] below to ensure that it's a bug:

Describe the bug Signing a PUT URL with a ContentType results in a URL without Content-Type included in the signed headers, so the ContentType is not enforced.

Version of AWS SDK for Go? github.com/aws/aws-sdk-go-v2/service/s3 v1.17.0

Version of Go (go version)? latest

To Reproduce (observed behavior)

  const contentType = "text/plain"
  in := &s3v2.PutObjectInput{
          Bucket: aws.String(bucket),
          Key:    aws.String(key),
          ContentType: aws.String(contentType),
  }
  p, _ := s3v2.NewPresignClient(client).PresignPutObject(ctx, in)
  fmt.Println(p.URL)

Expected behavior

V1 produced a signed URL that included content-type in the X-Amz-SignedHeaders. V2 does not.

As a result, using the signed header from V2 doesn't enforce the content type.

Example URL from V1 (some of the URL elided):

https://go-cloud-testing.s3.us-west-1.amazonaws.com/blob-for-signing?...X-Amz-SignedHeaders=content-type%3Bhost&X-Amz-Signature=cf1572571f8ebd5f5f47b0487d2fea44e6ffc33766fc3719bf5ee61dad9c0e3b     

Example URL from V2:

https://go-cloud-testing.s3.us-west-1.amazonaws.com/blob-for-signing?...X-Amz-SignedHeaders=host&x-id=PutObject&X-Amz-Signature=10df84e7fc6715cd212c53d7cb85248a41e3714e0602fc8075183587c90857e6       

Additional context V1 code is here: https://github.com/google/go-cloud/blob/master/blob/s3blob/s3blob.go#L750

vangent avatar Oct 28 '21 18:10 vangent

hi @vangent , Thanks for pointing this out to us, yea this is definitely a bug, pretty sure it's happening somewhere here... We'll get this fixed as fast as we can.

KaibaLopez avatar Nov 05 '21 22:11 KaibaLopez

We have noticed this issue has not received attention in 1 year. We will close this issue for now. If you think this is in error, please feel free to comment and reopen the issue.

github-actions[bot] avatar Nov 06 '22 00:11 github-actions[bot]

Please fix this.

vangent avatar Nov 07 '22 18:11 vangent

Hi @vangent ,

First of all I'd like to apologize for the long wait. The person who assigned it to themselves left the company and this issue fell between the cracks. This is not the kind of experience we want users to have with any AWS product, so for that I am sincerely sorry.

To answer your question Content-Type will not be set unless Content-Length is also set, or is not 0. (Its 0 by default) There is a specific utility function that omits that header if Content length is not present in the params.

// RemoveContentTypeHeader removes content-type header if
// content length is unset or equal to zero.
func RemoveContentTypeHeader(stack *middleware.Stack) error {
	return stack.Build.Add(&removeContentTypeHeader{}, middleware.After)
}

Here is how I created the presigned URL:

func main() {

	cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRegion("us-east-1"), config.WithClientLogMode(aws.LogResponseWithBody|aws.LogRequestWithBody))
	if err != nil {
		panic(err)
	}

	presigner := s3.NewPresignClient(client)

	presignPutObject, err := presigner.PresignPutObject(context.TODO(), &s3.PutObjectInput{
		Bucket:        aws.String(myBucket),
		Key:           aws.String(myKey),
		ContentType:   aws.String(MyContentType),
		ContentLength: 1,
	})
	if err != nil {
		panic(err)
	}
	fmt.Println(presignPutObject.URL)
	
	// output: 
	// https://REDACTED.s3.us-east-1.amazonaws.com/REDACTED?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=REDACTED&X-Amz-Date=20221107T212750Z&X-Amz-Expires=900&X-Amz-SignedHeaders=content-length%3Bcontent-type%3Bhost&x-id=PutObject&X-Amz-Signature=REDACTED

Let me know if this helps. All the best, Ran~

RanVaknin avatar Nov 07 '22 21:11 RanVaknin

Thanks for getting back to this!

So, I think that explains why the implementation does what it does, but it doesn't explain why. Maybe there's a good reason, but can you explain why the ContentType should not be enforced unless there is a non-zero ContentLength?

vangent avatar Nov 08 '22 01:11 vangent

BTW, https://github.com/aws/aws-sdk-java-v2/issues/2520 is a similar bug with a regression from V1.

vangent avatar Nov 08 '22 01:11 vangent

@vangent ,

Unfortunately I'm not entirely sure. My best guess would be that this is related to some newer S3 related requirements specifying this behavior.

I take it that my previous answer solved the issue at hand so I feel confident we can close this thread. If you have any other issues please feel free to open another issue and I'll do my best to address it as soon as possible.

Thanks again! Ran~

RanVaknin avatar Nov 10 '22 00:11 RanVaknin

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

github-actions[bot] avatar Nov 10 '22 00:11 github-actions[bot]

I have a similar problem. I get a CORS error when adding Content-Type to the signed URL PUT request. Followed your advice to add ContentLength to the PutObjectCommand when generating the signed URL and I can see that header in the browser, but the Content-Type is still missing in X-Amz-SignedHeaders. I run nodejs on the server with @aws-sdk/client-s3 (v3)

` import { getSignedUrl } from '@aws-sdk/s3-request-presigner' import config from "../config/config"

const client = new S3({ region: "eu-north-1", endpoint: config.myUrl, credentials: { accessKeyId: config.myAccess, secretAccessKey: config.mySecret, } })

const signedUrl = async (args: any) => { const fileId = cuid() const filetype = args.filetype const filename = args.filename .replace(/([Ì]|[^0-9a-öA-Ö.\s])/g, '') .normalize('NFKD') .replace(/([\u0300-\u036f]|[^0-9a-zA-Z.\s])/g, '') const command = new PutObjectCommand({ Key: fileId + "/" + filename, Bucket: config.myBucket, ContentType: filetype, ContentLength: 1 }) const url = await getSignedUrl(client, command, { expiresIn: 15 * 60 }) return { id: fileId, name: filename, url, type: filetype } } `

Any idea?

Cheers, Josef

Hi @vangent ,

First of all I'd like to apologize for the long wait. The person who assigned it to themselves left the company and this issue fell between the cracks. This is not the kind of experience we want users to have with any AWS product, so for that I am sincerely sorry.

To answer your question Content-Type will not be set unless Content-Length is also set, or is not 0. (Its 0 by default) There is a specific utility function that omits that header if Content length is not present in the params.

henryson avatar Sep 27 '23 16:09 henryson

Pinging @RanVaknin based on the github action notification and that it doesn't appear, anyone will see @henryson comment otherwise

TheRedSpy15 avatar Nov 01 '23 04:11 TheRedSpy15