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

The getAwsChunkedEncodingStream function returns a stream which encodes all streams (can be binary streams) as utf-8 strings #4635

Closed
3 tasks done
tommn2 opened this issue Apr 12, 2023 · 3 comments
Assignees
Labels
bug This issue is a bug. p2 This is a standard priority issue

Comments

@tommn2
Copy link

tommn2 commented Apr 12, 2023

Checkboxes for prior research

Describe the bug

The getAwsChunkedEncodingStream() method is returning a stream that is encoding all streams as utf-8 strings. Each chunk is being written out with a chunk length that is the correct buffer length but the chunk data is written as a utf-8 string (the default encoding for the Buffer.toString() method). As a result, the chunk's data length does not equal the length written out at the beginning of the chunk.

Without digging into the code more and looking at everywhere this stream is used, I don't know if there are use-cases where a utf-8 string is desired. If there are then, at a minimum, the chunk length would be incorrect.

In my case I am uploading a binary file to S3 via a PutObjectCommand object; I have a binary stream which is being incorrectly converted to utf-8 resulting in an error response from S3. See below for a sample error from CloudTrail.

Here's a suggested fix.
tommn2@8704162

SDK version number

@aws-sdk/client-s3@3.306.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v14.19.0

Reproduction Steps

Use PutObjectCommand to upload a file to S3 and include the { ChecksumAlgorithm: 'SHA256' } option. Without the checksum option the upload seems to bypass the getAwsChunkedEncodingStream() stream.

Observed Behavior

	{
		"eventVersion": "1.08",
		"userIdentity": {
			"type": "IAMUser",
			"principalId": "<redacted>",
			"arn": "arn:aws:iam::<redacted>:user/<redacted>",
			"accountId": "<redacted>",
			"accessKeyId": "<redacted>",
			"userName": "<redacted>"
		},
		"eventTime": "2023-04-06T20:18:53Z",
		"eventSource": "s3.amazonaws.com",
		"eventName": "PutObject",
		"awsRegion": "us-east-2",
		"sourceIPAddress": "<redacted>",
		"userAgent": "[aws-sdk-js/3.306.0 os/darwin/22.3.0 lang/js md/nodejs/14.19.0 api/s3/3.306.0]",
		"errorCode": "IncompleteBody",
		"errorMessage": "The request body terminated unexpectedly",
		"requestParameters": {
			"bucketName": "<redacted>",
			"Host": "<redacted>.s3.us-east-2.amazonaws.com",
			"x-amz-server-side-encryption": "AES256",
			"key": "file-repo-tests/2023-04-06T20:18:51.314Z/RJ0ge8bnTbGRGPbzl7ksZQ/audio/sample-audio-short.mp3",
			"x-id": "PutObject",
			"x-amz-storage-class": "INTELLIGENT_TIERING"
		},
		"responseElements": null,
		"additionalEventData": {
			"SignatureVersion": "SigV4",
			"CipherSuite": "ECDHE-RSA-AES128-GCM-SHA256",
			"bytesTransferredIn": 118401,
			"AuthenticationMethod": "AuthHeader",
			"x-amz-id-2": "OI8PVcGLlKLCBnwgWT1bn+Zb1aK92uVjJUx7WDo0rbDEV2t7AC3lGyOIBTZyPXA2Wf37Qay8Sy8=",
			"bytesTransferredOut": 272
		},
		"requestID": "RMSVSJ95DZ1RYR22",
		"eventID": "6d0008eb-cce0-43fe-ad5e-4bd21aa38ea9",
		"readOnly": false,
		"resources": [
			{
				"type": "AWS::S3::Object",
				"ARN": "arn:aws:s3:::<redacted>/file-repo-tests/2023-04-06T20:18:51.314Z/RJ0ge8bnTbGRGPbzl7ksZQ/audio/sample-audio-short.mp3"
			},
			{
				"accountId": "<redacted>",
				"type": "AWS::S3::Bucket",
				"ARN": "arn:aws:s3:::<redacted>"
			}
		],
		"eventType": "AwsApiCall",
		"managementEvent": false,
		"recipientAccountId": "<redacted>",
		"eventCategory": "Data",
		"tlsDetails": {
			"tlsVersion": "TLSv1.2",
			"cipherSuite": "ECDHE-RSA-AES128-GCM-SHA256",
			"clientProvidedHostHeader": "<redacted>.s3.us-east-2.amazonaws.com"
		}
	}

Expected Behavior

Successful upload to S3.

Possible Solution

tommn2@8704162

Additional Information/Context

No idea why this has not previously been reported as a bug. Do most people do not use the checksum feature and so it has just gone un-noticed?

@tommn2 tommn2 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 12, 2023
@RanVaknin RanVaknin self-assigned this Apr 18, 2023
@RanVaknin RanVaknin added investigating Issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Apr 19, 2023
@RanVaknin
Copy link
Contributor

Hi @tommn2 ,

Thanks for opening the issue. I'm able to reproduce and confirm that this is a bug.
Tested it with a similar code in the Go SDK and did not run into the same error.

Repro steps and behavior on JS SDK:

import { S3Client, PutObjectCommand, ChecksumAlgorithm } from '@aws-sdk/client-s3';
import * as fs from 'fs';
const client = new S3Client({ region: 'us-east-1' });

const fileStream = fs.createReadStream("path/to/file");

fileStream.on("error", function (err) {
  console.log("File Error", err);
});

const buffer = Buffer.from(ChecksumAlgorithm.SHA256);
const encodedString = buffer.toString("base64");

fileStream.on("open", async function (){
    try {
        const data = await client.send(new PutObjectCommand({
            Bucket: "my-bucket",
            Key: "my-key",
            Body: fileStream,
            ChecksumAlgorithm: "SHA256"
        }));
        console.log(data.ETag)
    } catch (error) {
        console.log(error);
    }
})
IncompleteBody: The request body terminated unexpectedly
    at throwDefaultError (/Users/user/test_folder/4635/node_modules/@aws-sdk/smithy-client/dist-cjs/default-error-handler.js:8:22)
    at /Users/user/test_folder/4635/node_modules/@aws-sdk/smithy-client/dist-cjs/default-error-handler.js:18:39
    at de_PutObjectCommandError (/Users/user/test_folder/4635/node_modules/@aws-sdk/client-s3/dist-cjs/protocols/Aws_restXml.js:5701:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async /Users/user/test_folder/4635/node_modules/@aws-sdk/middleware-serde/dist-cjs/deserializerMiddleware.js:7:24
    at async /Users/user/test_folder/4635/node_modules/@aws-sdk/middleware-signing/dist-cjs/middleware.js:14:20
    at async /Users/user/test_folder/4635/node_modules/@aws-sdk/middleware-retry/dist-cjs/retryMiddleware.js:27:46
    at async /Users/user/test_folder/4635/node_modules/@aws-sdk/middleware-flexible-checksums/dist-cjs/flexibleChecksumsMiddleware.js:58:20
    at async /Users/user/test_folder/4635/node_modules/@aws-sdk/middleware-logger/dist-cjs/loggerMiddleware.js:7:26
    at async ReadStream.<anonymous> (file:///Users/user/test_folder/4635/sample.js:34:22) {
  '$fault': 'client',
  '$metadata': {
    httpStatusCode: 400,
    requestId: 'REDACTED',
    extendedRequestId: 'REDACTED',
    cfId: undefined,
    attempts: 1,
    totalRetryDelay: 0
  },
  Code: 'IncompleteBody',
  RequestId: 'REDACTED',
  HostId: 'REDACTED'
}

Go SDK v2:

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

	client := s3.NewFromConfig(cfg)

	file, err := os.Open("path/to/file")
	if err != nil {
		panic(err)
	}
	defer file.Close()

	_, err = client.PutObject(context.Background(), &s3.PutObjectInput{
		Key:               aws.String("my-key"),
		Body:              file,
		Bucket:            aws.String("my-bucket"),
		ChecksumAlgorithm: types.ChecksumAlgorithmSha256,
	})
	if err != nil {
		panic(err)
	}
}
SDK 2023/04/20 09:26:53 DEBUG Response
HTTP/1.1 200 OK
Content-Length: 0
Date: Thu, 20 Apr 2023 16:26:54 GMT
Etag: "d41d8cd98f00b204e9800998ecf8427e"
Server: AmazonS3
X-Amz-Checksum-Sha256: REDACTED
X-Amz-Id-2: REDACTED
X-Amz-Request-Id: REDACTED
X-Amz-Server-Side-Encryption: AES256

Will assign needs-review label so that this will get added to the backlog.

Thank you,
Ran~

@RanVaknin RanVaknin added needs-review This issue/pr needs review from an internal developer. p2 This is a standard priority issue and removed investigating Issue is being investigated and/or work is in progress to resolve the issue. labels Apr 20, 2023
@tommn2
Copy link
Author

tommn2 commented Apr 20, 2023

Awesome, thanks!

@RanVaknin RanVaknin added queued This issues is on the AWS team's backlog and removed needs-review This issue/pr needs review from an internal developer. labels Apr 27, 2023
@RanVaknin RanVaknin removed their assignment Apr 27, 2023
@kuhe kuhe self-assigned this May 9, 2023
@kuhe kuhe added pending-release This issue will be fixed by an approved PR that hasn't been released yet. and removed pending-release This issue will be fixed by an approved PR that hasn't been released yet. queued This issues is on the AWS team's backlog labels May 10, 2023
@kuhe kuhe closed this as completed Sep 11, 2023
@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 Sep 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue is a bug. p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

3 participants