From e79af3a0b1294127afb4938eaa73d56719e569fd Mon Sep 17 00:00:00 2001 From: George Fu Date: Tue, 7 Oct 2025 12:14:50 -0400 Subject: [PATCH 1/2] fix(lib-storage): restrict lstat calls to fs.ReadStream objects --- lib/lib-storage/src/byteLength.spec.ts | 106 ++++++++++--------- lib/lib-storage/src/byteLength.ts | 3 +- lib/lib-storage/src/byteLengthSource.spec.ts | 68 ++++++++++++ lib/lib-storage/src/byteLengthSource.ts | 2 +- lib/lib-storage/src/runtimeConfig.shared.ts | 3 + lib/lib-storage/src/runtimeConfig.ts | 5 +- 6 files changed, 134 insertions(+), 53 deletions(-) create mode 100644 lib/lib-storage/src/byteLengthSource.spec.ts diff --git a/lib/lib-storage/src/byteLength.spec.ts b/lib/lib-storage/src/byteLength.spec.ts index a0d19647f0e5..a5c53ee01abd 100644 --- a/lib/lib-storage/src/byteLength.spec.ts +++ b/lib/lib-storage/src/byteLength.spec.ts @@ -1,70 +1,78 @@ -import { describe, expect, it, vi } from "vitest"; +import fs from "node:fs"; +import { describe, expect, it } from "vitest"; -import { BYTE_LENGTH_SOURCE, byteLengthSource } from "./byteLengthSource"; -import { runtimeConfig } from "./runtimeConfig"; +import { byteLength } from "./byteLength"; -vi.mock("./runtimeConfig", () => ({ - runtimeConfig: { - lstatSync: vi.fn(), - }, -})); - -describe("byteLengthSource", () => { - it("should return CONTENT_LENGTH when override is provided", () => { - expect(byteLengthSource({}, 100)).toBe(BYTE_LENGTH_SOURCE.CONTENT_LENGTH); +describe("byteLength", () => { + it("should handle null and undefined input", () => { + expect(byteLength(null)).toBe(0); + expect(byteLength(undefined)).toBe(0); }); - it("should return EMPTY_INPUT for null input", () => { - expect(byteLengthSource(null)).toBe(BYTE_LENGTH_SOURCE.EMPTY_INPUT); - }); + describe("strings", () => { + it("empty", () => { + expect(byteLength("")).toBe(0); + }); - it("should return EMPTY_INPUT for undefined input", () => { - expect(byteLengthSource(undefined)).toBe(BYTE_LENGTH_SOURCE.EMPTY_INPUT); - }); + it("should return correct length for ASCII characters", () => { + expect(byteLength("hello")).toBe(5); + expect(byteLength("12345")).toBe(5); + expect(byteLength("!@#$%")).toBe(5); + }); - it("should return STRING_LENGTH for string input", () => { - expect(byteLengthSource("test")).toBe(BYTE_LENGTH_SOURCE.STRING_LENGTH); - }); + it("should return correct length for unicode characters", () => { + expect(byteLength("πŸ˜€")).toBe(4); + }); - it("should return TYPED_ARRAY for input with byteLength", () => { - const input = new Uint8Array(10); - expect(byteLengthSource(input)).toBe(BYTE_LENGTH_SOURCE.TYPED_ARRAY); + it("should handle mixed ASCII and unicode characters", () => { + expect(byteLength("hello δΈ–η•Œ")).toBe(12); + expect(byteLength("hi πŸ˜€")).toBe(7); + }); }); - it("should return LENGTH for input with length property", () => { - const input = { length: 10 }; - expect(byteLengthSource(input)).toBe(BYTE_LENGTH_SOURCE.LENGTH); - }); + describe("byte arrays", () => { + it("should handle Uint8Array", () => { + expect(byteLength(new Uint8Array([1, 2, 3]))).toBe(3); + expect(byteLength(new Uint8Array([]))).toBe(0); + }); - it("should return SIZE for input with size property", () => { - const input = { size: 10 }; - expect(byteLengthSource(input)).toBe(BYTE_LENGTH_SOURCE.SIZE); + it("should handle Buffer", () => { + expect(byteLength(Buffer.from([1, 2, 3]))).toBe(3); + expect(byteLength(Buffer.from([]))).toBe(0); + }); }); - it("should return START_END_DIFF for input with start and end properties", () => { - const input = { start: 0, end: 10 }; - expect(byteLengthSource(input)).toBe(BYTE_LENGTH_SOURCE.START_END_DIFF); - }); + describe("things with length or size properties", () => { + it("should handle arrays", () => { + expect(byteLength([1, 2, 3])).toBe(3); + expect(byteLength([])).toBe(0); + }); - it("should return LSTAT for input with path that exists", () => { - const input = { path: "/test/path" }; - vi.mocked(runtimeConfig.lstatSync).mockReturnValue({ size: 100 } as any); + it("should handle objects with length property", () => { + expect(byteLength({ length: 5 })).toBe(5); + expect(byteLength({ length: 0 })).toBe(0); + }); - expect(byteLengthSource(input)).toBe(BYTE_LENGTH_SOURCE.LSTAT); - expect(runtimeConfig.lstatSync).toHaveBeenCalledWith("/test/path"); + it("should handle objects with size property", () => { + expect(byteLength({ size: 10 })).toBe(10); + expect(byteLength({ size: 0 })).toBe(0); + }); }); - it("should return undefined for input with path that throws error", () => { - const input = { path: "/test/path" }; - vi.mocked(runtimeConfig.lstatSync).mockImplementation(() => { - throw new Error("File not found"); + describe("start end differentials", () => { + it("should handle readable streams", () => { + const stream = fs.createReadStream(__filename, { + start: 1000, + end: 1499, + }); + expect(byteLength(stream)).toBe(500); }); - - expect(byteLengthSource(input)).toBeUndefined(); }); - it("should return undefined for input with no matching properties", () => { - const input = { foo: "bar" }; - expect(byteLengthSource(input)).toBeUndefined(); + describe("filestreams", () => { + it("should handle readable streams", () => { + const stream = fs.createReadStream(__filename); + expect(byteLength(stream)).toBe(fs.lstatSync(__filename).size); + }); }); }); diff --git a/lib/lib-storage/src/byteLength.ts b/lib/lib-storage/src/byteLength.ts index fdc860501a01..b57e4d4686f3 100644 --- a/lib/lib-storage/src/byteLength.ts +++ b/lib/lib-storage/src/byteLength.ts @@ -29,8 +29,7 @@ export const byteLength = (input: any): number | undefined => { } else if (typeof input.start === "number" && typeof input.end === "number") { // file read stream with range. return input.end + 1 - input.start; - } else if (typeof input.path === "string") { - // file read stream with path. + } else if (runtimeConfig.isFileReadStream(input)) { try { return runtimeConfig.lstatSync(input.path).size; } catch (error) { diff --git a/lib/lib-storage/src/byteLengthSource.spec.ts b/lib/lib-storage/src/byteLengthSource.spec.ts new file mode 100644 index 000000000000..1a7c338be6f3 --- /dev/null +++ b/lib/lib-storage/src/byteLengthSource.spec.ts @@ -0,0 +1,68 @@ +import fs from "node:fs"; +import { describe, expect, it, vi } from "vitest"; + +import { BYTE_LENGTH_SOURCE, byteLengthSource } from "./byteLengthSource"; +import { runtimeConfig } from "./runtimeConfig"; + +describe("byteLengthSource", () => { + it("should return CONTENT_LENGTH when override is provided", () => { + expect(byteLengthSource({}, 100)).toBe(BYTE_LENGTH_SOURCE.CONTENT_LENGTH); + }); + + it("should return EMPTY_INPUT for null input", () => { + expect(byteLengthSource(null)).toBe(BYTE_LENGTH_SOURCE.EMPTY_INPUT); + }); + + it("should return EMPTY_INPUT for undefined input", () => { + expect(byteLengthSource(undefined)).toBe(BYTE_LENGTH_SOURCE.EMPTY_INPUT); + }); + + it("should return STRING_LENGTH for string input", () => { + expect(byteLengthSource("test")).toBe(BYTE_LENGTH_SOURCE.STRING_LENGTH); + }); + + it("should return TYPED_ARRAY for input with byteLength", () => { + const input = new Uint8Array(10); + expect(byteLengthSource(input)).toBe(BYTE_LENGTH_SOURCE.TYPED_ARRAY); + }); + + it("should return LENGTH for input with length property", () => { + const input = { length: 10 }; + expect(byteLengthSource(input)).toBe(BYTE_LENGTH_SOURCE.LENGTH); + }); + + it("should return SIZE for input with size property", () => { + const input = { size: 10 }; + expect(byteLengthSource(input)).toBe(BYTE_LENGTH_SOURCE.SIZE); + }); + + it("should return START_END_DIFF for input with start and end properties", () => { + const input = { start: 0, end: 10 }; + expect(byteLengthSource(input)).toBe(BYTE_LENGTH_SOURCE.START_END_DIFF); + }); + + it("should return LSTAT for input with path that exists", () => { + const input = fs.createReadStream(__filename); + vi.spyOn(runtimeConfig, "lstatSync"); + + expect(byteLengthSource(input)).toBe(BYTE_LENGTH_SOURCE.LSTAT); + expect(runtimeConfig.lstatSync).toHaveBeenCalledWith(__filename); + }); + + it("ignores objects with a path property that aren't fs.ReadStream objects", () => { + const input = { path: __filename }; + + expect(byteLengthSource(input)).toBe(undefined); + }); + + it("should return undefined for input with path that throws error", () => { + const input = { path: "surely-this-path-does-not-exist" }; + + expect(byteLengthSource(input)).toBeUndefined(); + }); + + it("should return undefined for input with no matching properties", () => { + const input = { foo: "bar" }; + expect(byteLengthSource(input)).toBeUndefined(); + }); +}); diff --git a/lib/lib-storage/src/byteLengthSource.ts b/lib/lib-storage/src/byteLengthSource.ts index 5a47864039e6..07de676f55c6 100644 --- a/lib/lib-storage/src/byteLengthSource.ts +++ b/lib/lib-storage/src/byteLengthSource.ts @@ -42,7 +42,7 @@ export const byteLengthSource = (input: any, override?: number): BYTE_LENGTH_SOU return BYTE_LENGTH_SOURCE.SIZE; } else if (typeof input.start === "number" && typeof input.end === "number") { return BYTE_LENGTH_SOURCE.START_END_DIFF; - } else if (typeof input.path === "string") { + } else if (runtimeConfig.isFileReadStream(input)) { try { runtimeConfig.lstatSync(input.path).size; return BYTE_LENGTH_SOURCE.LSTAT; diff --git a/lib/lib-storage/src/runtimeConfig.shared.ts b/lib/lib-storage/src/runtimeConfig.shared.ts index 4cdcdcf9e11b..94fa48c1f3c7 100644 --- a/lib/lib-storage/src/runtimeConfig.shared.ts +++ b/lib/lib-storage/src/runtimeConfig.shared.ts @@ -3,4 +3,7 @@ */ export const runtimeConfigShared = { lstatSync: () => {}, + isFileReadStream(f: unknown) { + return false; + }, }; diff --git a/lib/lib-storage/src/runtimeConfig.ts b/lib/lib-storage/src/runtimeConfig.ts index 3b773d9daa7b..f16ee9921a87 100644 --- a/lib/lib-storage/src/runtimeConfig.ts +++ b/lib/lib-storage/src/runtimeConfig.ts @@ -1,4 +1,4 @@ -import { lstatSync } from "fs"; +import { lstatSync, ReadStream } from "fs"; import { runtimeConfigShared as shared } from "./runtimeConfig.shared"; @@ -9,4 +9,7 @@ export const runtimeConfig = { ...shared, runtime: "node", lstatSync, + isFileReadStream(f: unknown): f is ReadStream { + return f instanceof ReadStream; + }, }; From dbd15f3504377ba8b0698e9ff9c5dfbe91515734 Mon Sep 17 00:00:00 2001 From: George Fu Date: Tue, 7 Oct 2025 12:36:10 -0400 Subject: [PATCH 2/2] test: add test for runtimeConfig --- lib/lib-storage/src/index.spec.ts | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/lib/lib-storage/src/index.spec.ts b/lib/lib-storage/src/index.spec.ts index e37707c04a53..a3353fa37759 100644 --- a/lib/lib-storage/src/index.spec.ts +++ b/lib/lib-storage/src/index.spec.ts @@ -1,9 +1,34 @@ +import fs from "node:fs"; import { describe, expect, test as it } from "vitest"; import * as Storage from "./index"; +import { runtimeConfig } from "./runtimeConfig"; +import { runtimeConfig as runtimeConfigBrowser } from "./runtimeConfig.browser"; +import { runtimeConfig as runtimeConfigNative } from "./runtimeConfig.native"; describe("Storage Packages", () => { it("has Upload", () => { expect(Storage.Upload).toBeDefined(); }); }); + +describe("runtimeConfig", () => { + it("all environments have the same signature", () => { + const configs = [runtimeConfig, runtimeConfigBrowser, runtimeConfigNative]; + + for (const config of configs) { + expect(typeof config.lstatSync).toBe("function"); + expect(typeof config.isFileReadStream).toBe("function"); + } + + const stream = fs.createReadStream(__filename); + + expect(runtimeConfig.isFileReadStream(stream)).toBe(true); + expect(runtimeConfigBrowser.isFileReadStream(stream)).toBe(false); + expect(runtimeConfigNative.isFileReadStream(stream)).toBe(false); + + expect(runtimeConfig.runtime).toBe("node"); + expect(runtimeConfigBrowser.runtime).toBe("browser"); + expect(runtimeConfigNative.runtime).toBe("react-native"); + }); +});