Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions javascript/packages/core/lib/writer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
12 changes: 11 additions & 1 deletion javascript/packages/hps/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
};
Expand Down
4 changes: 3 additions & 1 deletion javascript/packages/hps/src/fastcall.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,10 @@ static void serializeString(const v8::FunctionCallbackInfo<v8::Value> &args) {
uint32_t offset = args[2].As<v8::Number>()->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<uint8_t *>(dst->Buffer()->GetBackingStore()->Data());
reinterpret_cast<uint8_t *>(dst->Buffer()->GetBackingStore()->Data()) +
dst->ByteOffset();

if (is_one_byte && str->IsExternalOneByte()) {
offset += writeVarUint32(dst_data, offset,
Expand Down
54 changes: 51 additions & 3 deletions javascript/test/hps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand All @@ -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);
}
});
});
Loading