Skip to content

fix(lib-storage): add validation for partCount and contentLength for multipart upload #7363

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Sep 24, 2025
Merged

fix(lib-storage): add validation for partCount and contentLength for multipart upload #7363

merged 4 commits into from
Sep 24, 2025

Conversation

smilkuri
Copy link
Contributor

@smilkuri smilkuri commented Sep 19, 2025 •
edited by kuhe
Loading

Issue

Internal JS-6196

Description

  • Adds validation to check total number of UploadPart requests the SDK has sent matches the expected number of parts.
  • Adds validation to check the content length of each UploadPart request matches the expected part size.

Testing

Locally
yarn test and yarn test:e2e are successful

host % yarn test

 RUN  v3.2.4 /aws-sdk-js-v3/lib/lib-storage

 ✓ src/chunks/getChunkUint8Array.spec.ts (6 tests) 7ms
 ✓ src/chunks/getDataReadableStream.spec.ts (4 tests) 168ms
 ✓ src/chunks/getDataReadable.spec.ts (3 tests) 167ms
 ✓ src/index.spec.ts (1 test) 2ms
 ✓ src/Upload.spec.ts (34 tests) 22888ms
   ✓ Upload > should add tags to the object if tags have been added multi-part  22807ms

 Test Files  5 passed (5)
      Tests  48 passed (48)
   Start at  16:15:06
   Duration  23.90s (transform 861ms, setup 0ms, collect 2.42s, tests 23.23s, environment 1ms, prepare 584ms)

Checklist

  • [n/a] If the PR is a feature, add integration tests (*.integ.spec.ts).
  • [n/a] If you wrote E2E tests, are they resilient to concurrent I/O?
  • [n/a] If adding new public functions, did you add the @public tag and enable doc generation on the package?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@smilkuri smilkuri requested a review from a team as a code owner September 19, 2025 16:22
@smilkuri smilkuri changed the title fixyarn test(lib-storage): add validation for partCount and contentLength for multipart upload test(lib-storage): add validation for partCount and contentLength for multipart upload Sep 19, 2025
@smilkuri smilkuri changed the title test(lib-storage): add validation for partCount and contentLength for multipart upload fix(lib-storage): add validation for partCount and contentLength for multipart upload Sep 19, 2025
kuhe
kuhe reviewed Sep 19, 2025
View reviewed changes
kuhe
kuhe reviewed Sep 22, 2025
View reviewed changes
lib/lib-storage/src/Upload.ts Outdated
}

private __validateUploadPart(dataPart: RawDataPart): void {
const actualPartSize = byteLength(dataPart.data) || undefined;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what falsy values does byteLength return? What if it's zero?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can return zero for empty parts, may be for scenarios like empty file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if byteLength can return undefined, then the expression should be byteLength(dataPart.data);

zero byte length should not be converted to undefined.

kuhe
kuhe reviewed Sep 22, 2025
View reviewed changes
kuhe
kuhe reviewed Sep 22, 2025
View reviewed changes
kuhe
kuhe reviewed Sep 24, 2025
View reviewed changes
@smilkuri smilkuri force-pushed the s3-durability-upload branch from bf89a19 to ef5a56e Compare September 24, 2025 16:53
@smilkuri smilkuri merged commit 0562a0d into main Sep 24, 2025
7 checks passed
@smilkuri smilkuri deleted the s3-durability-upload branch September 24, 2025 18:03
@trivikr trivikr mentioned this pull request Sep 27, 2025
Closed
4 tasks
@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 Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@kuhe kuhe kuhe approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@smilkuri @kuhe