diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 88b5562e1f..41c2b22449 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1178,6 +1178,9 @@ jobs: matrix: node-version: [18, 24] os: [ubuntu-latest, macos-latest, windows-latest] + include: + - node-version: 20 + os: ubuntu-latest runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v7.0.1 diff --git a/javascript/packages/core/lib/writer/index.ts b/javascript/packages/core/lib/writer/index.ts index f08f6bb5ec..d9f5bd1c7d 100644 --- a/javascript/packages/core/lib/writer/index.ts +++ b/javascript/packages/core/lib/writer/index.ts @@ -296,6 +296,8 @@ export class BinaryWriter { } stringWithHeaderFast(v: string) { + // Worst case: 5-byte varint header plus UTF-16 body. + this.reserve(5 + v.length * 2); const { serializeString } = this.config.hps!; this.cursor = serializeString(v, this.platformBuffer, this.cursor); } diff --git a/javascript/packages/hps/index.ts b/javascript/packages/hps/index.ts index 507cc67595..82cf9c7f53 100644 --- a/javascript/packages/hps/index.ts +++ b/javascript/packages/hps/index.ts @@ -31,7 +31,17 @@ const build = () => { if (typeof v !== "string") { throw new Error(`isLatin1 requires string but got ${typeof v}`); } - // todo boundary check + // The native callback converts offsets to uint32_t before unchecked writes. + if (offset !== offset >>> 0) { + throw new RangeError("serializeString offset must be an unsigned 32-bit integer"); + } + // The native writer copies without bounds checks: a 5-byte varint header + // plus at most 2 bytes per UTF-16 code unit must fit in `dist`. + if (offset + 5 + v.length * 2 > dist.byteLength) { + throw new RangeError( + `serializeString needs up to ${5 + v.length * 2} bytes at offset ${offset} but buffer length is ${dist.byteLength}`, + ); + } return _serializeString(dist, v, offset, 0); }, }; diff --git a/javascript/packages/hps/src/fastcall.cc b/javascript/packages/hps/src/fastcall.cc index 94e8c0ad78..e1e2c7722c 100644 --- a/javascript/packages/hps/src/fastcall.cc +++ b/javascript/packages/hps/src/fastcall.cc @@ -96,8 +96,10 @@ static void serializeString(const v8::FunctionCallbackInfo &args) { uint32_t offset = args[2].As()->Uint32Value(context).ToChecked(); bool is_one_byte = str->IsOneByte(); + // A Uint8Array may be a view into a larger (e.g. pooled) ArrayBuffer. uint8_t *dst_data = - reinterpret_cast(dst->Buffer()->GetBackingStore()->Data()); + reinterpret_cast(dst->Buffer()->GetBackingStore()->Data()) + + dst->ByteOffset(); if (is_one_byte && str->IsExternalOneByte()) { offset += writeVarUint32(dst_data, offset, diff --git a/javascript/test/hps.test.ts b/javascript/test/hps.test.ts index 471d278e34..cd9be9bb3e 100644 --- a/javascript/test/hps.test.ts +++ b/javascript/test/hps.test.ts @@ -17,13 +17,21 @@ * under the License. */ -import { BinaryReader } from "../packages/core/index"; +import Fory, { BinaryReader } from "../packages/core/index"; import hps from "../packages/hps/index"; -import { describe, expect, test } from "@jest/globals"; +import { beforeAll, describe, expect, test } from "@jest/globals"; -const skipableDescribe = hps ? describe : describe.skip; +const { engines } = require("../packages/hps/package.json"); +const skipableDescribe = require("semver").satisfies(process.version, engines.node) + ? describe + : describe.skip; skipableDescribe("hps", () => { + beforeAll(() => { + // A missing addon on a supported Node.js version must fail, not skip these tests. + expect(hps).not.toBeNull(); + }); + test("should isLatin1 work", () => { const { serializeString } = hps!; for (let index = 0; index < 10000; index++) { @@ -39,4 +47,44 @@ skipableDescribe("hps", () => { expect(reader.stringWithHeader()).toBe("😁"); } }); + + test("should reject strings exceeding buffer capacity", () => { + const { serializeString } = hps!; + const bf = Buffer.alloc(32); + expect(() => serializeString("A".repeat(10000), bf, 0)).toThrow(RangeError); + }); + + test.each([-1, -0.5, 0.5, NaN, Infinity, -Infinity, 2 ** 32])( + "should reject invalid offset %s", + (offset) => { + const bf = Buffer.alloc(32, 0x7f); + expect(() => hps!.serializeString("A", bf, offset)).toThrow(RangeError); + expect(bf.every((b) => b === 0x7f)).toBe(true); + }, + ); + + test.each(["hello", "\u4f60\u597d", "😁"])( + "should write %s into a view with non-zero byteOffset", + (value) => { + const { serializeString } = hps!; + // Exercise both alignments of the UTF-16 body in the native slow callback. + for (const offset of [0, 1]) { + const backing = Buffer.alloc(200); + const view = backing.subarray(101, 190); + const end = serializeString(value, view, offset); + expect(backing.subarray(0, 101 + offset).every((b) => b === 0)).toBe(true); + expect(backing.subarray(101 + end).every((b) => b === 0)).toBe(true); + const reader = new BinaryReader({}); + reader.reset(view.subarray(offset, end)); + expect(reader.stringWithHeader()).toBe(value); + } + }, + ); + + test("should grow writer buffer for large strings", () => { + const fory = new Fory({ hps }); + for (const value of ["A".repeat(200000), "\u4f60".repeat(200000)]) { + expect(fory.deserialize(fory.serialize(value))).toBe(value); + } + }); });