Skip to content

Commit a71e654

Browse files
committed
fix: byteLengthSource
1 parent b16f302 commit a71e654

File tree

10 files changed

+175
-42
lines changed

10 files changed

+175
-42
lines changed

lib/lib-storage/src/Upload.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -969,7 +969,7 @@ describe(Upload.name, () => {
969969
partSize: 1024, // Too small
970970
client: new S3({}),
971971
});
972-
}).toThrow(/EntityTooSmall: Your proposed upload partsize/);
972+
}).toThrow(/EntityTooSmall: Your proposed upload part size/);
973973
});
974974

975975
it("should validate minimum queueSize", () => {

lib/lib-storage/src/Upload.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ import { extendedEncodeURIComponent } from "@smithy/smithy-client";
2727
import type { AbortController as IAbortController, AbortSignal as IAbortSignal, Endpoint } from "@smithy/types";
2828
import { EventEmitter } from "events";
2929

30-
import { byteLength } from "./bytelength";
30+
import { byteLength } from "./byteLength";
31+
import { BYTE_LENGTH_SOURCE, byteLengthSource } from "./byteLengthSource";
3132
import { getChunk } from "./chunker";
3233
import { BodyDataTypes, Options, Progress } from "./types";
3334

@@ -60,6 +61,7 @@ export class Upload extends EventEmitter {
6061

6162
// used for reporting progress.
6263
private totalBytes?: number;
64+
private readonly totalBytesSource?: BYTE_LENGTH_SOURCE;
6365
private bytesUploadedSoFar: number;
6466

6567
// used in the upload.
@@ -98,6 +100,7 @@ export class Upload extends EventEmitter {
98100

99101
// set progress defaults
100102
this.totalBytes = this.params.ContentLength ?? byteLength(this.params.Body);
103+
this.totalBytesSource = byteLengthSource(this.params.Body, this.params.ContentLength);
101104
this.bytesUploadedSoFar = 0;
102105
this.abortController = options.abortController ?? new AbortController();
103106

@@ -381,9 +384,14 @@ export class Upload extends EventEmitter {
381384

382385
let result;
383386
if (this.isMultiPart) {
384-
const { expectedPartsCount, uploadedParts } = this;
385-
if (expectedPartsCount !== undefined && uploadedParts.length !== expectedPartsCount) {
386-
throw new Error(`Expected ${expectedPartsCount} part(s) but uploaded ${uploadedParts.length} part(s).`);
387+
const { expectedPartsCount, uploadedParts, totalBytes, totalBytesSource } = this;
388+
if (totalBytes !== undefined && expectedPartsCount !== undefined && uploadedParts.length !== expectedPartsCount) {
389+
throw new Error(`Expected ${expectedPartsCount} part(s) but uploaded ${uploadedParts.length} part(s).
390+
The expected part count is based on the byte-count of the input.params.Body,
391+
which was read from ${totalBytesSource} and is ${totalBytes}.
392+
If this is not correct, provide an override value by setting a number
393+
to input.params.ContentLength in bytes.
394+
`);
387395
}
388396

389397
this.uploadedParts.sort((a, b) => a.PartNumber! - b.PartNumber!);
@@ -478,7 +486,7 @@ export class Upload extends EventEmitter {
478486

479487
if (this.partSize < Upload.MIN_PART_SIZE) {
480488
throw new Error(
481-
`EntityTooSmall: Your proposed upload partsize [${this.partSize}] is smaller than the minimum allowed size [${Upload.MIN_PART_SIZE}] (5MB)`
489+
`EntityTooSmall: Your proposed upload part size [${this.partSize}] is smaller than the minimum allowed size [${Upload.MIN_PART_SIZE}] (5MB)`
482490
);
483491
}
484492

lib/lib-storage/src/byteLength.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import { Buffer } from "buffer"; // do not remove this import: Node.js buffer or buffer NPM module for browser.
2+
3+
import { runtimeConfig } from "./runtimeConfig";
4+
5+
/**
6+
* Clients use util-body-length-[node|browser] instead.
7+
* @internal
8+
* @param input - to examine.
9+
* @returns byte count of input or undefined if indeterminable.
10+
*/
11+
export const byteLength = (input: any): number | undefined => {
12+
if (input == null) {
13+
return 0;
14+
}
15+
16+
if (typeof input === "string") {
17+
return Buffer.byteLength(input);
18+
}
19+
20+
if (typeof input.byteLength === "number") {
21+
// Uint8Array, ArrayBuffer, Buffer, and ArrayBufferView
22+
return input.byteLength;
23+
} else if (typeof input.length === "number") {
24+
// todo: unclear in what cases this is a valid byte count.
25+
return input.length;
26+
} else if (typeof input.size === "number") {
27+
// todo: unclear in what cases this is a valid byte count.
28+
return input.size;
29+
} else if (typeof input.start === "number" && typeof input.end === "number") {
30+
// file read stream with range.
31+
return input.end + 1 - input.start;
32+
} else if (typeof input.path === "string") {
33+
// file read stream with path.
34+
try {
35+
return runtimeConfig.lstatSync(input.path).size;
36+
} catch (error) {
37+
return undefined;
38+
}
39+
}
40+
return undefined;
41+
};
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { runtimeConfig } from "./runtimeConfig";
2+
3+
/**
4+
* @internal
5+
*/
6+
export enum BYTE_LENGTH_SOURCE {
7+
EMPTY_INPUT = "a null or undefined Body",
8+
CONTENT_LENGTH = "the ContentLength property of the params set by the caller",
9+
STRING_LENGTH = "the encoded byte length of the Body string",
10+
TYPED_ARRAY = "the byteLength of a typed byte array such as Uint8Array",
11+
LENGTH = "the value of Body.length",
12+
SIZE = "the value of Body.size",
13+
START_END_DIFF = "the numeric difference between Body.start and Body.end",
14+
LSTAT = "the size of the file given by Body.path on disk as reported by lstatSync",
15+
}
16+
17+
/**
18+
* The returned value should complete the sentence, "The byte count of the data was determined by ...".
19+
* @internal
20+
* @param input - to examine.
21+
* @param override - manually specified value.
22+
* @returns source of byte count information.
23+
*/
24+
export const byteLengthSource = (input: any, override?: number): BYTE_LENGTH_SOURCE | undefined => {
25+
if (override != null) {
26+
return BYTE_LENGTH_SOURCE.CONTENT_LENGTH;
27+
}
28+
29+
if (input == null) {
30+
return BYTE_LENGTH_SOURCE.EMPTY_INPUT;
31+
}
32+
33+
if (typeof input === "string") {
34+
return BYTE_LENGTH_SOURCE.STRING_LENGTH;
35+
}
36+
37+
if (typeof input.byteLength === "number") {
38+
return BYTE_LENGTH_SOURCE.TYPED_ARRAY;
39+
} else if (typeof input.length === "number") {
40+
return BYTE_LENGTH_SOURCE.LENGTH;
41+
} else if (typeof input.size === "number") {
42+
return BYTE_LENGTH_SOURCE.SIZE;
43+
} else if (typeof input.start === "number" && typeof input.end === "number") {
44+
return BYTE_LENGTH_SOURCE.START_END_DIFF;
45+
} else if (typeof input.path === "string") {
46+
try {
47+
runtimeConfig.lstatSync(input.path).size;
48+
return BYTE_LENGTH_SOURCE.LSTAT;
49+
} catch (error) {
50+
return undefined;
51+
}
52+
}
53+
return undefined;
54+
};

lib/lib-storage/src/bytelength.ts

Lines changed: 0 additions & 26 deletions
This file was deleted.

lib/lib-storage/src/lib-storage.e2e.spec.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { getE2eTestResources } from "@aws-sdk/aws-util-test/src";
22
import { ChecksumAlgorithm, S3 } from "@aws-sdk/client-s3";
33
import { Upload } from "@aws-sdk/lib-storage";
44
import { randomBytes } from "crypto";
5+
import fs from "node:fs";
56
import { Readable } from "stream";
67
import { afterAll, beforeAll, describe, expect, test as it } from "vitest";
78

@@ -179,4 +180,59 @@ describe("@aws-sdk/lib-storage", () => {
179180
});
180181
}
181182
);
183+
184+
describe("inferring the byte length of the input", () => {
185+
beforeAll(async () => {
186+
const e2eTestResourcesEnv = await getE2eTestResources();
187+
Object.assign(process.env, e2eTestResourcesEnv);
188+
});
189+
190+
it("should throw an informative error about the correct override if the SDK infers the byte count incorrectly", async () => {
191+
const s3 = new S3({
192+
region: process.env.AWS_SMOKE_TEST_REGION,
193+
});
194+
195+
const pseudoFileReadStream = fs.createReadStream("/dev/urandom", { end: 6 * 1024 * 1024 });
196+
197+
const upload = new Upload({
198+
client: s3,
199+
params: {
200+
Key: `/dev/urandom`,
201+
Bucket: process.env.AWS_SMOKE_TEST_BUCKET,
202+
Body: pseudoFileReadStream,
203+
},
204+
});
205+
206+
const error = await upload.done().catch((e) => e);
207+
208+
expect(error).toBeInstanceOf(Error);
209+
expect(error.message).toEqual(`Expected 0 part(s) but uploaded 2 part(s).
210+
The expected part count is based on the byte-count of the input.params.Body,
211+
which was read from the size of the file given by Body.path on disk as reported by lstatSync and is 0.
212+
If this is not correct, provide an override value by setting a number
213+
to input.params.ContentLength in bytes.
214+
`);
215+
});
216+
217+
it("should use the input ContentLength as the total byte count if supplied by the caller", async () => {
218+
const s3 = new S3({
219+
region: process.env.AWS_SMOKE_TEST_REGION,
220+
});
221+
222+
const pseudoFileReadStream = fs.createReadStream("/dev/urandom", { end: 6 * 1024 * 1024 });
223+
224+
const upload = new Upload({
225+
client: s3,
226+
params: {
227+
Key: `/dev/urandom`,
228+
Bucket: process.env.AWS_SMOKE_TEST_BUCKET,
229+
Body: pseudoFileReadStream,
230+
ContentLength: 6 * 1024 * 1024,
231+
},
232+
});
233+
234+
await upload.done();
235+
// no thrown error is sufficient.
236+
});
237+
});
182238
}, 60_000);
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
import { ClientSharedValues } from "./runtimeConfig.shared";
1+
import { runtimeConfigShared as shared } from "./runtimeConfig.shared";
22

33
/**
44
* @internal
55
*/
6-
export const ClientDefaultValues = {
7-
...ClientSharedValues,
6+
export const runtimeConfig = {
7+
...shared,
88
runtime: "browser",
99
};
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
import { ClientDefaultValues as BrowserDefaults } from "./runtimeConfig.browser";
1+
import { runtimeConfig as browserConfig } from "./runtimeConfig.browser";
22

33
/**
44
* @internal
55
*/
6-
export const ClientDefaultValues = {
7-
...BrowserDefaults,
6+
export const runtimeConfig = {
7+
...browserConfig,
88
runtime: "react-native",
99
};
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
* @internal
33
*/
4-
export const ClientSharedValues = {
4+
export const runtimeConfigShared = {
55
lstatSync: () => {},
66
};
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { lstatSync } from "fs";
22

3-
import { ClientSharedValues } from "./runtimeConfig.shared";
3+
import { runtimeConfigShared as shared } from "./runtimeConfig.shared";
44

55
/**
66
* @internal
77
*/
8-
export const ClientDefaultValues = {
9-
...ClientSharedValues,
8+
export const runtimeConfig = {
9+
...shared,
1010
runtime: "node",
1111
lstatSync,
1212
};

0 commit comments

Comments
 (0)