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

SignatureDoesNotMatch while uploading an object to S3 with ContentType #1800

Closed
Tietew opened this issue Dec 17, 2020 · 8 comments · Fixed by #1892 or smithy-lang/smithy-typescript#261
Assignees
Labels
bug This issue is a bug.

Comments

@Tietew
Copy link

Tietew commented Dec 17, 2020

Describe the bug
SignatureDoesNotMatch occures while uploading an object to S3 with ContentType.

SDK version number
@aws-sdk/[email protected]

Is the issue in the browser/Node.js/ReactNative?
Node.js

Details of the browser/Node.js/ReactNative version

$ node -v
v12.20.0

To Reproduce (observed behavior)

Run following script

import { S3 } from '@aws-sdk/client-s3';

const s3 = new S3({ region: 'ap-northeast-1' });
s3.putObject({
  Bucket: 'mybucketname',
  Key: 'blah.json',
  ContentType: 'application/json',
  Body: Buffer.from(JSON.stringify({ dead: 'beef' })),
}).then(console.log, console.error);

Got

SignatureDoesNotMatch: The request signature we calculated does not match the signature you provided. Check your key and signing method.

Expected behavior
An object s3://mybucketname/blah.json is created with its Content-Type is application/json.

Additional context
With no ContentType, object is successfully created but its Content-Type is application/octet-stream.

@joain946
Copy link

I just ran into this issue as well.
The problems seems to be that there are two content type headers being added. The one specified in PutObject ends up as "Content-Type" then the other header that is being added behind the scene is added as "content-type".
The only workaround I have found so far is to create your own "signer" object when creating the S3 client and that class can then filter out the "content-type" header and then delegate to the "SignatureV4.ts" class.

Here is what the headers for the signing looks like when getting into the SignatureV4.sign(...) method:
image

@AllanZhengYP AllanZhengYP added the bug This issue is a bug. label Dec 17, 2020
@AllanZhengYP
Copy link
Contributor

Hi @Tietew @joain946

I can confirm this is a bug. We will fix it soon. Thanks a lot for reporting and investigating in this.

@apostolos
Copy link

Adding "content-type": true in ALWAYS_UNSIGNABLE_HEADERS seems to fix the problem:
https://github.com/aws/aws-sdk-js-v3/blob/master/packages/signature-v4/src/constants.ts#L18-L34

@adamdottv
Copy link

Hi @Tietew @joain946

I can confirm this is a bug. We will fix it soon. Thanks a lot for reporting and investigating in this.

Hi @AllanZhengYP, any chance you have an ETA on this fix? Or are there any known workarounds?

@AllanZhengYP AllanZhengYP self-assigned this Dec 21, 2020
@adamdottv
Copy link

adamdottv commented Dec 22, 2020

I'm actually working around it with middleware until this is fixed; in case this is helpful to anyone else:

const s3 = new S3()

s3.middlewareStack.add(
  (next) => async (args) => {
    delete args.request.headers['content-type']
    return next(args)
  },
  {step: 'build'},
)

await s3.putObject({
  Bucket: bucket,
  Key: key,
  Body: body,
  ContentType: contentType,
})

@xeaone
Copy link

xeaone commented Dec 24, 2020

Maybe this is assumed but I wanted to add that this issue also plagues the v3 modular interface. Also @adamelmore middleware workaround gets past the issue for the modular interface.

joshuaauerbachwatson added a commit to nimbella/nimbella-cli that referenced this issue Dec 30, 2020
1.  One needs MetadataDirective: 'REPLACE' when using object copying to
update metadata.  2.  One currently needs the workaround described in
aws/aws-sdk-js-v3#1800 (comment)
until the bug in the s3 SDK v3 is fixed.
@gswebspace
Copy link

Ran into this as well. Adding ContentType results in SignatureDoesNotMatch.

joshuaauerbachwatson added a commit to nimbella/nimbella-cli that referenced this issue Jan 6, 2021
1.  One needs MetadataDirective: 'REPLACE' when using object copying to
update metadata.  2.  One currently needs the workaround described in
aws/aws-sdk-js-v3#1800 (comment)
until the bug in the s3 SDK v3 is fixed.
joshuaauerbachwatson added a commit to nimbella/nimbella-cli that referenced this issue Jan 8, 2021
* WIP for storage abstraction

* Improvements to the storage client abstraction

* WIP for storage abstraction.  Likely not buildable

* Switch over to published storage providers

* Minor lint fixes

* The short deployer test suite is passing now.

Storage provider at 0.0.2, GCS provider at 0.0.3.

* Add provider field to storage key, dispatch on it

* Fix omitted await operations from recent revision

* Initial cut on s3 storage provider.

Only some operations supported (WIP)

* Improvements to storage provider, now 0.0.7

* Adjust to latest storage-provider interface

Storage provider now at 0.0.7, storage-gcs now at 0.0.5

* Complete functionality for awsprovider

Test needs updating to cover the added functionality.

* Adjust to latest storage provider interface

* Bug fix: don't destructure what might be undefined

* Start using/testing the s3 storage provider

* Deal with missing Contents on vacuous ListObjects

* Need @types/node

We were accidentally borrowing it from the parent node_modules, which
won't necessarily exist.

* Need @types/node, url-parse, and tslib dependents

We were accidentally borrowing these from the parent node_modules but
that won't always be populated.

* Need @types/node, debug, and memory-streams

We were accidentally borrowing these from the parent node_modules which
won't always exist.

* Fix things revealed by testing (incl. workaround)

1.  One needs MetadataDirective: 'REPLACE' when using object copying to
update metadata.  2.  One currently needs the workaround described in
aws/aws-sdk-js-v3#1800 (comment)
until the bug in the s3 SDK v3 is fixed.

* Numerous changes to get initial test working

Works on both scality and AWS.
Includes a workaround for the problem with content-type metadata.
Publishes new versions 8 of base, 6 of gcs, 3 of aws.
Removes secrets from test.

* Ensure that gcs provider is statically required

This is needed for correct webpacking.  Getting other storage providers
to be webpacked is for another day.

* s3 storage provider acl and url improvements

* Latest SDK, which works with s3
AllanZhengYP added a commit to AllanZhengYP/aws-sdk-js-v3 that referenced this issue Jan 8, 2021
The HTTP request class doesn't have any implementations to insure
case-insensitivity of the http headers. So we need to be mindful
when populating these headers. Otherwise, the reqeust signature
will be messed up. This change will ensure the protocol-specific
default headers like content-type will be overriden by the serialized
header if exists.

For other headers added through middleware stack either by
customization or users, it wouldn't affect signing or sending as long
as the request doesn't contain same header names in different casing.
All the internal headers will be consistent. But users should be
careful when they are adding their own headers.

We don't add middleware to lowercase all headers to prevent
alternating the users' customizations.

Ref: aws#1800
AllanZhengYP added a commit to AllanZhengYP/smithy-typescript that referenced this issue Jan 8, 2021
The HTTP request class doesn't have any implementations to insure
case-insensitivity of the http headers. So we need to be mindful
when populating these headers. Otherwise, the reqeust signature
will be messed up. This change will ensure the protocol-specific
default headers like content-type will be overriden by the serialized
header if exists.

For other headers added through middleware stack either by
customization or users, it wouldn't affect signing or sending as long
as the request doesn't contain same header names in different casing.
All the internal headers will be consistent. But users should be
careful when they are adding their own headers.

We don't add middleware to lowercase all headers to prevent
alternating the users' customizations.

Ref: aws/aws-sdk-js-v3#1800
coderbyheart added a commit to bifravst/app that referenced this issue Jan 11, 2021
AllanZhengYP added a commit to AllanZhengYP/aws-sdk-js-v3 that referenced this issue Jan 11, 2021
The HTTP request class doesn't have any implementations to insure
case-insensitivity of the http headers. So we need to be mindful
when populating these headers. Otherwise, the reqeust signature
will be messed up. This change will ensure the protocol-specific
default headers like content-type will be overriden by the serialized
header if exists.

For other headers added through middleware stack either by
customization or users, it wouldn't affect signing or sending as long
as the request doesn't contain same header names in different casing.
All the internal headers will be consistent. But users should be
careful when they are adding their own headers.

We don't add middleware to lowercase all headers to prevent
alternating the users' customizations.

Ref: aws#1800
AllanZhengYP added a commit to AllanZhengYP/smithy-typescript that referenced this issue Jan 11, 2021
The HTTP request class doesn't have any implementations to insure
case-insensitivity of the http headers. So we need to be mindful
when populating these headers. Otherwise, the reqeust signature
will be messed up. This change will ensure the protocol-specific
default headers like content-type will be overriden by the serialized
header if exists.

For other headers added through middleware stack either by
customization or users, it wouldn't affect signing or sending as long
as the request doesn't contain same header names in different casing.
All the internal headers will be consistent. But users should be
careful when they are adding their own headers.

We don't add middleware to lowercase all headers to prevent
alternating the users' customizations.

Ref: aws/aws-sdk-js-v3#1800
AllanZhengYP added a commit to AllanZhengYP/smithy-typescript that referenced this issue Jan 11, 2021
The HTTP request class doesn't have any implementations to insure
case-insensitivity of the http headers. So we need to be mindful
when populating these headers. Otherwise, the reqeust signature
will be messed up. This change will ensure the protocol-specific
default headers like content-type will be overriden by the serialized
header if exists.

For other headers added through middleware stack either by
customization or users, it wouldn't affect signing or sending as long
as the request doesn't contain same header names in different casing.
All the internal headers will be consistent. But users should be
careful when they are adding their own headers.

We don't add middleware to lowercase all headers to prevent
alternating the users' customizations.

Ref: aws/aws-sdk-js-v3#1800
AllanZhengYP added a commit to smithy-lang/smithy-typescript that referenced this issue Jan 13, 2021
The HTTP request class doesn't have any implementations to insure
case-insensitivity of the http headers. So we need to be mindful
when populating these headers. Otherwise, the reqeust signature
will be messed up. This change will ensure the protocol-specific
default headers like content-type will be overriden by the serialized
header if exists.

For other headers added through middleware stack either by
customization or users, it wouldn't affect signing or sending as long
as the request doesn't contain same header names in different casing.
All the internal headers will be consistent. But users should be
careful when they are adding their own headers.

We don't add middleware to lowercase all headers to prevent
alternating the users' customizations.

Ref: aws/aws-sdk-js-v3#1800
AllanZhengYP added a commit that referenced this issue Jan 13, 2021
The HTTP request class doesn't have any implementations to insure
case-insensitivity of the http headers. So we need to be mindful
when populating these headers. Otherwise, the reqeust signature
will be messed up. This change will ensure the protocol-specific
default headers like content-type will be overriden by the serialized
header if exists.

For other headers added through middleware stack either by
customization or users, it wouldn't affect signing or sending as long
as the request doesn't contain same header names in different casing.
All the internal headers will be consistent. But users should be
careful when they are adding their own headers.

We don't add middleware to lowercase all headers to prevent
alternating the users' customizations.

Ref: #1800
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue is a bug.
Projects
None yet
7 participants