Skip to content

Commit

Permalink
fix(util-stream): do not convert stream data to string in AwsChunkedE…
Browse files Browse the repository at this point in the history 
…ncodingStream (#4711)

* fix(util-stream): do not convert stream data to string in AwsChunkedEncodingStream

* fix(util-stream-node): make unit test easier to understand
  • Loading branch information
@kuhe
kuhe committed May 10, 2023
1 parent b43ee0d commit 5a9149b
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 66 deletions.
36 changes: 36 additions & 0 deletions packages/middleware-flexible-checksums/src/middleware-flexible-checksums.integ.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { S3 } from "@aws-sdk/client-s3";
import { Transform } from "stream";

import { requireRequestsFrom } from "../../../private/aws-util-test/src";

Expand Down Expand Up @@ -80,5 +81,40 @@ describe("middleware-flexible-checksums", () => {

expect.hasAssertions();
});

it("should not set binary file content length", async () => {
const client = new S3({ region: "us-west-2" });

requireRequestsFrom(client).toMatch({
method: "PUT",
hostname: "s3.us-west-2.amazonaws.com",
protocol: "https:",
path: "/b/k",
headers: {
"content-type": "application/octet-stream",
"x-amz-content-sha256": "STREAMING-UNSIGNED-PAYLOAD-TRAILER",
"content-length": /undefined/,
},
query: {
"x-id": "PutObject",
},
});

const stream = new Transform({
transform(chunk) {
return chunk;
},
});
stream.write("hello");

await client.putObject({
Bucket: "b",
Key: "k",
Body: stream,
ChecksumAlgorithm: "SHA256",
});

expect.hasAssertions();
});
});
});
50 changes: 18 additions & 32 deletions packages/util-stream-node/src/getAwsChunkedEncodingStream.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,19 @@ describe(getAwsChunkedEncodingStream.name, () => {

describe("skips checksum computation", () => {
const validateStreamWithoutChecksum = (awsChunkedEncodingStream: Readable, done: Function) => {
let dataEventCount = 0;
let buffer = "";
awsChunkedEncodingStream.on("data", (data) => {
if (dataEventCount === mockStreamChunks.length) {
expect(data.toString()).toStrictEqual(`0\r\n`);
} else {
expect(data.toString()).toStrictEqual(
`${mockBodyLength}\r\n${mockStreamChunks[dataEventCount].toString()}\r\n`
);
}
dataEventCount++;
buffer += data.toString();
});
awsChunkedEncodingStream.on("end", () => {
expect(mockStreamHasher).not.toHaveBeenCalled();
expect(mockBase64Encoder).not.toHaveBeenCalled();
expect(dataEventCount).toStrictEqual(mockStreamChunks.length + 1);
expect(buffer).toEqual(`5\r
Hello\r
5\r
World\r
0\r
`);
done();
});
};
Expand Down Expand Up @@ -90,33 +88,21 @@ describe(getAwsChunkedEncodingStream.name, () => {
const readableStream = getMockReadableStream();
const awsChunkedEncodingStream = getAwsChunkedEncodingStream(readableStream, mockOptions);

let dataEventCount = 0;
let buffer = "";
awsChunkedEncodingStream.on("data", (data) => {
if (dataEventCount >= mockStreamChunks.length) {
switch (dataEventCount) {
case mockStreamChunks.length:
expect(data.toString()).toStrictEqual(`0\r\n`);
break;
case mockStreamChunks.length + 1:
expect(data.toString()).toStrictEqual(`${mockChecksumLocationName}:${mockChecksum}\r\n`);
break;
case mockStreamChunks.length + 2:
expect(data.toString()).toStrictEqual(`\r\n`);
break;
default:
throw Error("Code shouldn't reach here");
}
} else {
expect(data.toString()).toStrictEqual(
`${mockBodyLength}\r\n${mockStreamChunks[dataEventCount].toString()}\r\n`
);
}
dataEventCount++;
buffer += data.toString();
});
awsChunkedEncodingStream.on("end", () => {
expect(mockStreamHasher).toHaveBeenCalledWith(mockChecksumAlgorithmFn, readableStream);
expect(mockBase64Encoder).toHaveBeenCalledWith(mockRawChecksum);
expect(dataEventCount).toStrictEqual(mockStreamChunks.length + 3);
expect(buffer).toStrictEqual(`5\r
Hello\r
5\r
World\r
0\r
mockChecksumLocationName:mockChecksum\r
\r
`);
done();
});
});
Expand Down
5 changes: 4 additions & 1 deletion packages/util-stream-node/src/getAwsChunkedEncodingStream.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ export const getAwsChunkedEncodingStream: GetAwsChunkedEncodingStream<Readable>

const awsChunkedEncodingStream = new Readable({ read: () => {} });
readableStream.on("data", (data) => {
awsChunkedEncodingStream.push(`${(bodyLengthChecker(data) || 0).toString(16)}\r\n${data.toString()}\r\n`);
const length = bodyLengthChecker(data) || 0;
awsChunkedEncodingStream.push(`${length.toString(16)}\r\n`);
awsChunkedEncodingStream.push(data);
awsChunkedEncodingStream.push("\r\n");
});
readableStream.on("end", async () => {
awsChunkedEncodingStream.push(`0\r\n`);
Expand Down
50 changes: 18 additions & 32 deletions packages/util-stream/src/getAwsChunkedEncodingStream.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,19 @@ describe(getAwsChunkedEncodingStream.name, () => {

describe("skips checksum computation", () => {
const validateStreamWithoutChecksum = (awsChunkedEncodingStream: Readable, done: Function) => {
let dataEventCount = 0;
let buffer = "";
awsChunkedEncodingStream.on("data", (data) => {
if (dataEventCount === mockStreamChunks.length) {
expect(data.toString()).toStrictEqual(`0\r\n`);
} else {
expect(data.toString()).toStrictEqual(
`${mockBodyLength}\r\n${mockStreamChunks[dataEventCount].toString()}\r\n`
);
}
dataEventCount++;
buffer += data.toString();
});
awsChunkedEncodingStream.on("end", () => {
expect(mockStreamHasher).not.toHaveBeenCalled();
expect(mockBase64Encoder).not.toHaveBeenCalled();
expect(dataEventCount).toStrictEqual(mockStreamChunks.length + 1);
expect(buffer).toEqual(`5\r
Hello\r
5\r
World\r
0\r
`);
done();
});
};
Expand Down Expand Up @@ -90,33 +88,21 @@ describe(getAwsChunkedEncodingStream.name, () => {
const readableStream = getMockReadableStream();
const awsChunkedEncodingStream = getAwsChunkedEncodingStream(readableStream, mockOptions);

let dataEventCount = 0;
let buffer = "";
awsChunkedEncodingStream.on("data", (data) => {
if (dataEventCount >= mockStreamChunks.length) {
switch (dataEventCount) {
case mockStreamChunks.length:
expect(data.toString()).toStrictEqual(`0\r\n`);
break;
case mockStreamChunks.length + 1:
expect(data.toString()).toStrictEqual(`${mockChecksumLocationName}:${mockChecksum}\r\n`);
break;
case mockStreamChunks.length + 2:
expect(data.toString()).toStrictEqual(`\r\n`);
break;
default:
throw Error("Code shouldn't reach here");
}
} else {
expect(data.toString()).toStrictEqual(
`${mockBodyLength}\r\n${mockStreamChunks[dataEventCount].toString()}\r\n`
);
}
dataEventCount++;
buffer += data.toString();
});
awsChunkedEncodingStream.on("end", () => {
expect(mockStreamHasher).toHaveBeenCalledWith(mockChecksumAlgorithmFn, readableStream);
expect(mockBase64Encoder).toHaveBeenCalledWith(mockRawChecksum);
expect(dataEventCount).toStrictEqual(mockStreamChunks.length + 3);
expect(buffer).toStrictEqual(`5\r
Hello\r
5\r
World\r
0\r
mockChecksumLocationName:mockChecksum\r
\r
`);
done();
});
});
Expand Down
5 changes: 4 additions & 1 deletion packages/util-stream/src/getAwsChunkedEncodingStream.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ export const getAwsChunkedEncodingStream: GetAwsChunkedEncodingStream<Readable>

const awsChunkedEncodingStream = new Readable({ read: () => {} });
readableStream.on("data", (data) => {
awsChunkedEncodingStream.push(`${(bodyLengthChecker(data) || 0).toString(16)}\r\n${data.toString()}\r\n`);
const length = bodyLengthChecker(data) || 0;
awsChunkedEncodingStream.push(`${length.toString(16)}\r\n`);
awsChunkedEncodingStream.push(data);
awsChunkedEncodingStream.push("\r\n");
});
readableStream.on("end", async () => {
awsChunkedEncodingStream.push(`0\r\n`);
Expand Down

0 comments on commit 5a9149b

Please sign in to comment.