From 299a597b332ea4875b0b7e2eb5cc21e1a2b7aa40 Mon Sep 17 00:00:00 2001 From: Enoch Groot Date: Sat, 19 Sep 2026 05:41:48 +0000 Subject: [PATCH 1/2] feat: add pack type and family hints - pack(value, { type, family, interpret }) forces MessagePack wire types - two-arg last-object detection; pack(1, 2) still packs an array --- CHANGELOG.md | 23 +- COVERAGE.md | 2 +- README.md | 47 ++- index.d.ts | 50 +++- package-lock.json | 4 +- package.json | 4 +- src/msgpack.cc | 19 ++ src/pack_hints.inc | 617 ++++++++++++++++++++++++++++++++++++++++ test/pack-hints.test.js | 244 ++++++++++++++++ 9 files changed, 999 insertions(+), 11 deletions(-) create mode 100644 src/pack_hints.inc create mode 100644 test/pack-hints.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index e3dabfc..b8258e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [3.1.0] - 2026-09-19 + +Optional second-argument pack hints force a MessagePack wire type or family +without changing the default mapping. Two or more values still pack as an +array. See `#52`. + +### Added + +- `pack(value, { type })` writes a fixed MessagePack type (`fixint`, + `uint8`…`uint64`, `int8`…`int64`, `float32`/`float64`, `fixstr`/`str8`… + `str32`, `bin8`…`bin32`, `nil`/`true`/`false`). Out-of-range values throw + `cannot pack value as `. +- `pack(value, { family })` picks a compact encoding in that family (`int`, + `float`, `str`, `bin`). `type` wins if both are set. +- `pack(array, { interpret })` maps each element through `interpret(item)` + which must return `{ data }` and may also set `type` / `family`. +- Detection is last-argument, two-arg only: the object must own-enumerate + only `type`, `family`, and/or `interpret`. Extra keys, one-arg objects, and + `pack(1, 2)` keep the old array packing. + ## [3.0.0] - 2026-09-19 Integers whose magnitude is greater than `Number.MAX_SAFE_INTEGER` unpack as @@ -88,6 +108,7 @@ GitHub Actions tests Node 18/20/22 on Ubuntu, macOS, and Windows 2022. - Pack throw paths free or return pooled sbuffers on every exit. - msgpack-c c-7.0.2 includes unpacker buffer-expansion overflow checks. -[Unreleased]: https://github.com/msgpack/msgpack-node/compare/v3.0.0...HEAD +[Unreleased]: https://github.com/msgpack/msgpack-node/compare/v3.1.0...HEAD +[3.1.0]: https://github.com/msgpack/msgpack-node/compare/v3.0.0...v3.1.0 [3.0.0]: https://github.com/msgpack/msgpack-node/compare/e04c9b55f98d64512174d6e859b8294b729659a2...HEAD [2.0.0]: https://github.com/msgpack/msgpack-node/commit/e04c9b55f98d64512174d6e859b8294b729659a2 diff --git a/COVERAGE.md b/COVERAGE.md index 80b8a6f..3c616ab 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -1,4 +1,4 @@ -# Coverage — msgpack 3.0.0 +# Coverage — msgpack 3.1.0 `npm run coverage` runs both halves and fails the build under 95%. diff --git a/README.md b/README.md index 1ab38d5..67a737e 100644 --- a/README.md +++ b/README.md @@ -2,9 +2,9 @@ and de-serializes JavaScript values with [MessagePack](https://msgpack.org). Packed output is a `Buffer` and is typically much smaller than JSON. -Version 3.0 requires **Node.js 18+**, vendors **msgpack-c c-7.0.2**, and -unpacks 64-bit integers outside `Number.MAX_SAFE_INTEGER` as `bigint`. See -[`SECURITY.md`](SECURITY.md). +Version 3.1 requires **Node.js 18+**, vendors **msgpack-c c-7.0.2**, unpacks +64-bit integers outside `Number.MAX_SAFE_INTEGER` as `bigint`, and accepts +optional pack type/family hints. See [`SECURITY.md`](SECURITY.md). ### Usage @@ -79,6 +79,47 @@ is that same `bigint`. `unpack.bytes_remaining` is the number of unused trailing bytes after the last successful (or attempted) unpack. Stream uses that to splice leftover data. +### Pack type hints (3.1) + +`pack(value, options)` takes an optional last-argument options object when +there are exactly two arguments and that object own-enumerates only `type`, +`family`, and/or `interpret`. Extra keys, a one-argument `{ type: ... }` +value, and `pack(1, 2)` still pack as values / an array. + +```javascript +msgpack.pack(123, { type: 'fixint' }); // 0x7b +msgpack.pack(123, { type: 'uint8' }); // 0xcc 0x7b +msgpack.pack(Math.PI, { type: 'float32' }); // 0xca + 4 bytes +msgpack.pack(buf, { family: 'bin' }); +msgpack.pack(1.5, { family: 'int' }); // throws +msgpack.pack(500, { type: 'uint8' }); // throws + +msgpack.pack( + [ + { data: Math.PI, type: 'float64' }, + { data: 3.14, type: 'float32' }, + ], + { + interpret(item) { + return { data: item.data, type: item.type }; + }, + }, +); +``` + +`type` forces that MessagePack type (`fixint`, `uint8`…`uint64`, `int8`… +`int64`, `float32`/`float64`, `fixstr`/`str8`/`str16`/`str32`, `bin8`/`bin16`/ +`bin32`, `nil`/`true`/`false`). `family` (`int`, `float`, `str`, `bin`) picks +a compact encoding in that family. If both are set, `type` wins. Out-of-range +values throw `cannot pack value as `. + +`interpret` is used when packing an Array. Each element is passed to +`interpret(item)`, which must return `{ data }` and may also set `type` / +`family` for that element. Nested `interpret` on the returned object is +ignored. + +Default packing is unchanged when no recognized options object is passed. + ### Limits * array/map length ≤ 1,000,000 diff --git a/index.d.ts b/index.d.ts index 7f1175d..d253e74 100644 --- a/index.d.ts +++ b/index.d.ts @@ -1,21 +1,67 @@ -// Type definitions for msgpack 3.0.0 +// Type definitions for msgpack 3.1.0 // Project: https://github.com/msgpack/msgpack-node /// import { EventEmitter } from 'events'; +export type PackType = + | 'fixint' + | 'uint8' + | 'uint16' + | 'uint32' + | 'uint64' + | 'int8' + | 'int16' + | 'int32' + | 'int64' + | 'float32' + | 'float64' + | 'fixstr' + | 'str8' + | 'str16' + | 'str32' + | 'bin8' + | 'bin16' + | 'bin32' + | 'nil' + | 'true' + | 'false'; + +export type PackFamily = 'int' | 'float' | 'str' | 'bin'; + +export interface PackInterpretResult { + data: any; + type?: PackType; + family?: PackFamily; +} + +export interface PackOptions { + type?: PackType; + family?: PackFamily; + interpret?: (item: any) => PackInterpretResult; +} + /** * Serialize values to MessagePack. * * A single argument is packed as itself; two or more are packed as an array * of that many elements. * + * When the second argument own-enumerates only `type`, `family`, and/or + * `interpret`, it is pack options rather than a second value. `type` forces + * a MessagePack wire type; `family` picks a compact encoding in that family + * (`type` wins if both are set). `interpret` is used when packing an Array: + * each element is replaced by `interpret(item)`, which must return `{ data }` + * and may also set `type` / `family`. + * * `bigint` values in the int64/uint64 range pack as MessagePack integers * (smallest family that fits). Values outside that range throw. A `number` * that has already lost bits below 2^53 stays on the Number path; lost bits - * are not recovered. + * are not recovered. BigInt plus an integer `type`/`family` uses the same + * 64-bit path. */ +export function pack(value: any, options: PackOptions): Buffer; export function pack(...values: any[]): Buffer; /** diff --git a/package-lock.json b/package-lock.json index 155a1ff..5ce342f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "msgpack", - "version": "3.0.0", + "version": "3.1.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "msgpack", - "version": "3.0.0", + "version": "3.1.0", "license": "BSD-3-Clause", "dependencies": { "nan": "^2.23.1" diff --git a/package.json b/package.json index 384055b..37d0d62 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "msgpack", "description": "A space-efficient object serialization library for Node.js", - "version": "3.0.0", + "version": "3.1.0", "homepage": "https://github.com/msgpack/msgpack-node", "author": "Peter Griess ", "contributors": [ @@ -34,7 +34,7 @@ "nan": "^2.23.1" }, "scripts": { - "test": "node --test test/bigint.test.js test/cli.test.js test/coverage-native.test.js test/msgpack.test.js test/regression.test.js test/security.test.js test/worker.test.js", + "test": "node --test test/bigint.test.js test/cli.test.js test/coverage-native.test.js test/msgpack.test.js test/pack-hints.test.js test/regression.test.js test/security.test.js test/worker.test.js", "bench": "node test/benchmark/benchmark.js", "rebuild": "node-gyp rebuild", "coverage": "npm run coverage:js && npm run coverage:native", diff --git a/src/msgpack.cc b/src/msgpack.cc index 0c08285..0836489 100644 --- a/src/msgpack.cc +++ b/src/msgpack.cc @@ -15,6 +15,8 @@ #include #include +#include +#include #include #include @@ -425,6 +427,18 @@ static v8::Local CheckedOwnNames(v8::Local obj) { return r.ToLocalChecked(); } +static v8::Local CallOneArg(v8::Local recv, + v8::Local fn, + v8::Local arg) { + Nan::TryCatch try_catch; + v8::Local argv[1] = {arg}; + Nan::MaybeLocal r = Nan::Call(fn, recv, 1, argv); + if (r.IsEmpty()) { + ThrowCaught(try_catch); /* GCOVR_EXCL_BR_LINE: never returns */ + } + return r.ToLocalChecked(); +} + static void PackArray(msgpack_packer* pk, v8::Local arr, int depth) { if (IsMarked(arr)) { throw MsgpackException(Error("Cowardly refusing to pack circular reference")); @@ -589,6 +603,8 @@ static void JsToMsgpack(msgpack_packer* pk, v8::Local o, int depth) { /* GCOVR_EXCL_BR_STOP */ } +#include "pack_hints.inc" + static v8::Local MsgpackToJs(const msgpack_object* mo); static v8::Local MsgpackToJs(const msgpack_object* mo) { @@ -755,6 +771,9 @@ NAN_METHOD(Pack) { if (info.Length() == 1) { JsToMsgpack(&pk, info[0], 0); + } else if (info.Length() == 2 && IsPackOptionsObject(info[1])) { + PackHint hint = ParsePackOptions(info[1].As()); + JsToMsgpackHinted(&pk, info[0], 0, hint); } else { /* GCOVR_EXCL_BR_START: allocation failure only. */ if (msgpack_pack_array(&pk, info.Length())) { diff --git a/src/pack_hints.inc b/src/pack_hints.inc new file mode 100644 index 0000000..c4ce93f --- /dev/null +++ b/src/pack_hints.inc @@ -0,0 +1,617 @@ +enum PackType { + kTypeNone = 0, + kTypeFixint, + kTypeUint8, + kTypeUint16, + kTypeUint32, + kTypeUint64, + kTypeInt8, + kTypeInt16, + kTypeInt32, + kTypeInt64, + kTypeFloat32, + kTypeFloat64, + kTypeFixstr, + kTypeStr8, + kTypeStr16, + kTypeStr32, + kTypeBin8, + kTypeBin16, + kTypeBin32, + kTypeNil, + kTypeTrue, + kTypeFalse +}; + +enum PackFamily { + kFamilyNone = 0, + kFamilyInt, + kFamilyFloat, + kFamilyStr, + kFamilyBin +}; + +struct PackHint { + PackType type; + PackFamily family; + bool has_interpret; + v8::Local interpret; + v8::Local recv; + PackHint() + : type(kTypeNone), + family(kFamilyNone), + has_interpret(false) {} +}; + +static void ThrowCannotPackAs(const char* what) { + char buf[80]; + std::snprintf(buf, sizeof(buf), "cannot pack value as %s", what); + throw MsgpackException(Error(buf)); +} + +static const char* PackTypeName(PackType t) { + switch (t) { + case kTypeFixint: return "fixint"; + case kTypeUint8: return "uint8"; + case kTypeUint16: return "uint16"; + case kTypeUint32: return "uint32"; + case kTypeUint64: return "uint64"; + case kTypeInt8: return "int8"; + case kTypeInt16: return "int16"; + case kTypeInt32: return "int32"; + case kTypeInt64: return "int64"; + case kTypeFloat32: return "float32"; + case kTypeFloat64: return "float64"; + case kTypeFixstr: return "fixstr"; + case kTypeStr8: return "str8"; + case kTypeStr16: return "str16"; + case kTypeStr32: return "str32"; + case kTypeBin8: return "bin8"; + case kTypeBin16: return "bin16"; + case kTypeBin32: return "bin32"; + case kTypeNil: return "nil"; + case kTypeTrue: return "true"; + case kTypeFalse: return "false"; + default: return "type"; + } +} + +static int WriteRaw(msgpack_packer* pk, const void* buf, size_t n) { + return pk->callback(pk->data, static_cast(buf), n); +} + +static bool IsPackOptionsObject(v8::Local v) { + if (v.IsEmpty() || !v->IsObject() || v->IsArray() || v->IsDate() || + v->IsFunction() || v->IsRegExp() || v->IsNativeError() || + v->IsPromise() || v->IsTypedArray() || v->IsArrayBuffer() || + v->IsSharedArrayBuffer() || v->IsDataView() || + node::Buffer::HasInstance(v)) { + return false; + } + v8::Local obj = v.As(); + Nan::TryCatch try_catch; + v8::MaybeLocal maybe_names = obj->GetOwnPropertyNames( + Nan::GetCurrentContext(), v8::PropertyFilter::ONLY_ENUMERABLE); + if (maybe_names.IsEmpty() || try_catch.HasCaught()) { + try_catch.Reset(); + return false; + } + v8::Local names = maybe_names.ToLocalChecked(); + bool saw = false; + for (uint32_t i = 0; i < names->Length(); i++) { + v8::Local key = CheckedGet(names, i); + if (!key->IsString()) { + return false; + } + Nan::Utf8String s(key); + if (std::strcmp(*s, "type") != 0 && std::strcmp(*s, "family") != 0 && + std::strcmp(*s, "interpret") != 0) { + return false; + } + saw = true; + } + return saw; +} + +static PackType ParsePackType(v8::Local v) { + if (!v->IsString()) { + throw MsgpackException(Error("pack type must be a string")); + } + Nan::Utf8String s(v); + const char* p = *s; + if (std::strcmp(p, "fixint") == 0) return kTypeFixint; + if (std::strcmp(p, "uint8") == 0) return kTypeUint8; + if (std::strcmp(p, "uint16") == 0) return kTypeUint16; + if (std::strcmp(p, "uint32") == 0) return kTypeUint32; + if (std::strcmp(p, "uint64") == 0) return kTypeUint64; + if (std::strcmp(p, "int8") == 0) return kTypeInt8; + if (std::strcmp(p, "int16") == 0) return kTypeInt16; + if (std::strcmp(p, "int32") == 0) return kTypeInt32; + if (std::strcmp(p, "int64") == 0) return kTypeInt64; + if (std::strcmp(p, "float32") == 0) return kTypeFloat32; + if (std::strcmp(p, "float64") == 0) return kTypeFloat64; + if (std::strcmp(p, "fixstr") == 0) return kTypeFixstr; + if (std::strcmp(p, "str8") == 0) return kTypeStr8; + if (std::strcmp(p, "str16") == 0) return kTypeStr16; + if (std::strcmp(p, "str32") == 0) return kTypeStr32; + if (std::strcmp(p, "bin8") == 0) return kTypeBin8; + if (std::strcmp(p, "bin16") == 0) return kTypeBin16; + if (std::strcmp(p, "bin32") == 0) return kTypeBin32; + if (std::strcmp(p, "nil") == 0) return kTypeNil; + if (std::strcmp(p, "true") == 0) return kTypeTrue; + if (std::strcmp(p, "false") == 0) return kTypeFalse; + throw MsgpackException(Error("unknown pack type")); +} + +static PackFamily ParsePackFamily(v8::Local v) { + if (!v->IsString()) { + throw MsgpackException(Error("pack family must be a string")); + } + Nan::Utf8String s(v); + const char* p = *s; + if (std::strcmp(p, "int") == 0) return kFamilyInt; + if (std::strcmp(p, "float") == 0) return kFamilyFloat; + if (std::strcmp(p, "str") == 0) return kFamilyStr; + if (std::strcmp(p, "bin") == 0) return kFamilyBin; + throw MsgpackException(Error("unknown pack family")); +} + +static void FillHintFromObject(v8::Local obj, PackHint* hint, + bool allow_interpret) { + v8::Local kType = Nan::New("type").ToLocalChecked(); + v8::Local kFamily = Nan::New("family").ToLocalChecked(); + v8::Local kInterpret = Nan::New("interpret").ToLocalChecked(); + if (Nan::HasOwnProperty(obj, kType).FromMaybe(false)) { + v8::Local t = CheckedGet(obj, kType); + if (!t->IsUndefined() && !t->IsNull()) { + hint->type = ParsePackType(t); + } + } + if (Nan::HasOwnProperty(obj, kFamily).FromMaybe(false)) { + v8::Local f = CheckedGet(obj, kFamily); + if (!f->IsUndefined() && !f->IsNull()) { + hint->family = ParsePackFamily(f); + } + } + if (allow_interpret && + Nan::HasOwnProperty(obj, kInterpret).FromMaybe(false)) { + v8::Local fn = CheckedGet(obj, kInterpret); + if (!fn->IsFunction()) { + throw MsgpackException(Error("pack interpret must be a function")); + } + hint->has_interpret = true; + hint->interpret = fn.As(); + hint->recv = obj; + } +} + +static PackHint ParsePackOptions(v8::Local obj) { + PackHint hint; + FillHintFromObject(obj, &hint, true); + return hint; +} + +static bool TryInt64(v8::Local o, int64_t* out) { + if (o->IsBigInt()) { + bool lossless = false; + *out = o.As()->Int64Value(&lossless); + return lossless; + } + if (!o->IsNumber()) { + return false; + } + double d = Nan::To(o).FromJust(); + if (!(std::isfinite(d) && std::trunc(d) == d)) { + return false; + } + if (d >= 0 && d < kTwoPow64) { + uint64_t u = static_cast(d); + if (u > static_cast(INT64_MAX)) { + return false; + } + *out = static_cast(u); + return true; + } + if (d < 0 && d >= kInt64Min) { + *out = static_cast(d); + return true; + } + return false; +} + +static bool TryUint64(v8::Local o, uint64_t* out) { + if (o->IsBigInt()) { + bool lossless = false; + *out = o.As()->Uint64Value(&lossless); + return lossless; + } + if (!o->IsNumber()) { + return false; + } + double d = Nan::To(o).FromJust(); + if (!(std::isfinite(d) && std::trunc(d) == d && d >= 0 && d < kTwoPow64)) { + return false; + } + *out = static_cast(d); + return true; +} + +static bool TryFloat(v8::Local o, double* out) { + if (!o->IsNumber()) { + return false; + } + *out = Nan::To(o).FromJust(); + return true; +} + +static void RequireUint(v8::Local o, uint64_t max, const char* name, + uint64_t* out) { + if (!TryUint64(o, out) || *out > max) { + ThrowCannotPackAs(name); + } +} + +static void RequireInt(v8::Local o, int64_t lo, int64_t hi, + const char* name, int64_t* out) { + if (!TryInt64(o, out) || *out < lo || *out > hi) { + ThrowCannotPackAs(name); + } +} + +static void PackStrBytes(msgpack_packer* pk, PackType t, const char* data, + size_t len) { + int rc = 0; + unsigned char hdr[5]; + size_t hn = 0; + switch (t) { + case kTypeNone: + rc = msgpack_pack_str(pk, len); + if (rc == 0) { /* GCOVR_EXCL_BR_LINE: rc != 0 needs an allocation failure */ + rc = msgpack_pack_str_body(pk, data, len); + } + if (rc != 0) { /* GCOVR_EXCL_BR_LINE */ + throw MsgpackException(Error("Error serializing object")); + } + return; + case kTypeFixstr: + if (len > 31) { + ThrowCannotPackAs("fixstr"); + } + hdr[0] = static_cast(0xa0u | len); + hn = 1; + break; + case kTypeStr8: + if (len > 255) { + ThrowCannotPackAs("str8"); + } + hdr[0] = 0xd9; + hdr[1] = static_cast(len); + hn = 2; + break; + case kTypeStr16: + if (len > 65535) { + ThrowCannotPackAs("str16"); + } + hdr[0] = 0xda; + hdr[1] = static_cast(len >> 8); + hdr[2] = static_cast(len); + hn = 3; + break; + case kTypeStr32: + hdr[0] = 0xdb; + hdr[1] = static_cast(len >> 24); + hdr[2] = static_cast(len >> 16); + hdr[3] = static_cast(len >> 8); + hdr[4] = static_cast(len); + hn = 5; + break; + default: + ThrowCannotPackAs("str"); + } + rc = WriteRaw(pk, hdr, hn); + if (rc == 0) { /* GCOVR_EXCL_BR_LINE */ + rc = msgpack_pack_str_body(pk, data, len); + } + if (rc != 0) { /* GCOVR_EXCL_BR_LINE */ + throw MsgpackException(Error("Error serializing object")); + } +} + +static void PackBinBytes(msgpack_packer* pk, PackType t, const char* data, + size_t len) { + int rc = 0; + unsigned char hdr[5]; + size_t hn = 0; + switch (t) { + case kTypeNone: + rc = msgpack_pack_bin(pk, len); + if (rc == 0) { /* GCOVR_EXCL_BR_LINE: rc != 0 needs an allocation failure */ + rc = msgpack_pack_bin_body(pk, data, len); + } + if (rc != 0) { /* GCOVR_EXCL_BR_LINE */ + throw MsgpackException(Error("Error serializing object")); + } + return; + case kTypeBin8: + if (len > 255) { + ThrowCannotPackAs("bin8"); + } + hdr[0] = 0xc4; + hdr[1] = static_cast(len); + hn = 2; + break; + case kTypeBin16: + if (len > 65535) { + ThrowCannotPackAs("bin16"); + } + hdr[0] = 0xc5; + hdr[1] = static_cast(len >> 8); + hdr[2] = static_cast(len); + hn = 3; + break; + case kTypeBin32: + hdr[0] = 0xc6; + hdr[1] = static_cast(len >> 24); + hdr[2] = static_cast(len >> 16); + hdr[3] = static_cast(len >> 8); + hdr[4] = static_cast(len); + hn = 5; + break; + default: + ThrowCannotPackAs("bin"); + } + rc = WriteRaw(pk, hdr, hn); + if (rc == 0) { /* GCOVR_EXCL_BR_LINE */ + rc = msgpack_pack_bin_body(pk, data, len); + } + if (rc != 0) { /* GCOVR_EXCL_BR_LINE */ + throw MsgpackException(Error("Error serializing object")); + } +} + +static void GetStrBytes(v8::Local o, PackType t, + msgpack_packer* pk) { + const char* name = (t == kTypeNone) ? "str" : PackTypeName(t); + if (o->IsString()) { + Nan::Utf8String u(o); + PackStrBytes(pk, t, *u, static_cast(u.length())); + return; + } + if (o->IsDate()) { + v8::Local date = o.As(); + v8::Local fn = + CheckedGet(date, Nan::New("toISOString").ToLocalChecked()); + if (!fn->IsFunction()) { + throw MsgpackException(Error("cannot pack Date")); + } + v8::Local iso = CallNoArgs(date, fn.As()); + Nan::Utf8String u(iso); + PackStrBytes(pk, t, *u, static_cast(u.length())); + return; + } + if (node::Buffer::HasInstance(o)) { + v8::Local b = o.As(); + PackStrBytes(pk, t, node::Buffer::Data(b), node::Buffer::Length(b)); + return; + } + ThrowCannotPackAs(name); +} + +static void GetBinBytes(v8::Local o, PackType t, + msgpack_packer* pk) { + const char* name = (t == kTypeNone) ? "bin" : PackTypeName(t); + if (node::Buffer::HasInstance(o)) { + v8::Local b = o.As(); + PackBinBytes(pk, t, node::Buffer::Data(b), node::Buffer::Length(b)); + return; + } + if (o->IsString()) { + Nan::Utf8String u(o); + PackBinBytes(pk, t, *u, static_cast(u.length())); + return; + } + ThrowCannotPackAs(name); +} + +static void PackAsType(msgpack_packer* pk, v8::Local o, + PackType t) { + int rc = 0; + uint64_t u = 0; + int64_t s = 0; + double d = 0; + switch (t) { + case kTypeFixint: + RequireInt(o, -32, 127, "fixint", &s); + { + unsigned char b = static_cast(static_cast(s)); + rc = WriteRaw(pk, &b, 1); + } + break; + case kTypeUint8: + RequireUint(o, 255ull, "uint8", &u); + rc = msgpack_pack_fix_uint8(pk, static_cast(u)); + break; + case kTypeUint16: + RequireUint(o, 65535ull, "uint16", &u); + rc = msgpack_pack_fix_uint16(pk, static_cast(u)); + break; + case kTypeUint32: + RequireUint(o, 0xffffffffull, "uint32", &u); + rc = msgpack_pack_fix_uint32(pk, static_cast(u)); + break; + case kTypeUint64: + RequireUint(o, UINT64_MAX, "uint64", &u); + rc = msgpack_pack_fix_uint64(pk, u); + break; + case kTypeInt8: + RequireInt(o, -128, 127, "int8", &s); + rc = msgpack_pack_fix_int8(pk, static_cast(s)); + break; + case kTypeInt16: + RequireInt(o, -32768, 32767, "int16", &s); + rc = msgpack_pack_fix_int16(pk, static_cast(s)); + break; + case kTypeInt32: + RequireInt(o, static_cast(INT32_MIN), + static_cast(INT32_MAX), "int32", &s); + rc = msgpack_pack_fix_int32(pk, static_cast(s)); + break; + case kTypeInt64: + RequireInt(o, INT64_MIN, INT64_MAX, "int64", &s); + rc = msgpack_pack_fix_int64(pk, s); + break; + case kTypeFloat32: + if (!TryFloat(o, &d)) { + ThrowCannotPackAs("float32"); + } + rc = msgpack_pack_float(pk, static_cast(d)); + break; + case kTypeFloat64: + if (!TryFloat(o, &d)) { + ThrowCannotPackAs("float64"); + } + rc = msgpack_pack_double(pk, d); + break; + case kTypeFixstr: + case kTypeStr8: + case kTypeStr16: + case kTypeStr32: + GetStrBytes(o, t, pk); + return; + case kTypeBin8: + case kTypeBin16: + case kTypeBin32: + GetBinBytes(o, t, pk); + return; + case kTypeNil: + if (!(o->IsNull() || o->IsUndefined())) { + ThrowCannotPackAs("nil"); + } + rc = msgpack_pack_nil(pk); + break; + case kTypeTrue: + if (!o->IsTrue()) { + ThrowCannotPackAs("true"); + } + rc = msgpack_pack_true(pk); + break; + case kTypeFalse: + if (!o->IsFalse()) { + ThrowCannotPackAs("false"); + } + rc = msgpack_pack_false(pk); + break; + default: + ThrowCannotPackAs("type"); + } + if (rc != 0) { /* GCOVR_EXCL_BR_LINE: sbuffer write failure */ + throw MsgpackException(Error("Error serializing object")); + } +} + +static void PackAsFamily(msgpack_packer* pk, v8::Local o, + PackFamily family) { + int rc = 0; + switch (family) { + case kFamilyInt: { + int64_t s = 0; + uint64_t u = 0; + if (TryInt64(o, &s)) { + rc = msgpack_pack_int64(pk, s); + } else if (TryUint64(o, &u)) { + rc = msgpack_pack_uint64(pk, u); + } else { + ThrowCannotPackAs("int"); + } + break; + } + case kFamilyFloat: { + double d = 0; + if (!TryFloat(o, &d)) { + ThrowCannotPackAs("float"); + } + if (std::isfinite(d)) { + float f = static_cast(d); + if (static_cast(f) == d) { + rc = msgpack_pack_float(pk, f); + break; + } + } + rc = msgpack_pack_double(pk, d); + break; + } + case kFamilyStr: + GetStrBytes(o, kTypeNone, pk); + return; + case kFamilyBin: + GetBinBytes(o, kTypeNone, pk); + return; + default: + ThrowCannotPackAs("family"); + } + if (rc != 0) { /* GCOVR_EXCL_BR_LINE: sbuffer write failure */ + throw MsgpackException(Error("Error serializing object")); + } +} + +static void JsToMsgpackHinted(msgpack_packer* pk, v8::Local o, + int depth, const PackHint& hint); + +static void PackArrayInterpreted(msgpack_packer* pk, v8::Local arr, + int depth, const PackHint& hint) { + if (IsMarked(arr)) { + throw MsgpackException(Error("Cowardly refusing to pack circular reference")); + } + Mark(arr); + try { + /* GCOVR_EXCL_BR_START: allocation failure only. */ + if (msgpack_pack_array(pk, arr->Length())) { + throw MsgpackException(Error("Error serializing object")); + } + /* GCOVR_EXCL_BR_STOP */ + for (uint32_t i = 0; i < arr->Length(); i++) { + v8::Local item = CheckedGet(arr, i); + v8::Local recv = + hint.recv.IsEmpty() ? Nan::New() : hint.recv; + v8::Local ret = CallOneArg(recv, hint.interpret, item); + if (!ret->IsObject() || ret->IsArray() || ret->IsDate() || + ret->IsFunction() || node::Buffer::HasInstance(ret)) { + throw MsgpackException(Error("interpret must return { data }")); + } + v8::Local obj = ret.As(); + v8::Local kData = Nan::New("data").ToLocalChecked(); + if (!Nan::HasOwnProperty(obj, kData).FromMaybe(false)) { + throw MsgpackException(Error("interpret must return { data }")); + } + v8::Local data = CheckedGet(obj, kData); + PackHint inner; + FillHintFromObject(obj, &inner, false); + JsToMsgpackHinted(pk, data, depth, inner); + } + } catch (...) { + Unmark(arr); + throw; + } + Unmark(arr); +} + +static void JsToMsgpackHinted(msgpack_packer* pk, v8::Local o, + int depth, const PackHint& hint) { + if (o->IsArray() && hint.has_interpret) { + if (kMaxPackDepth < depth + 1) { + throw MsgpackException( + Error("Cowardly refusing to pack object nested more than 512 levels deep")); + } + PackArrayInterpreted(pk, o.As(), depth + 1, hint); + return; + } + if (hint.type != kTypeNone) { + PackAsType(pk, o, hint.type); + return; + } + if (hint.family != kFamilyNone) { + PackAsFamily(pk, o, hint.family); + return; + } + JsToMsgpack(pk, o, depth); +} diff --git a/test/pack-hints.test.js b/test/pack-hints.test.js new file mode 100644 index 0000000..573b586 --- /dev/null +++ b/test/pack-hints.test.js @@ -0,0 +1,244 @@ +'use strict'; + +const { describe, it } = require('node:test'); +const assert = require('node:assert/strict'); +const msgpack = require('../lib/msgpack'); + +function b(...bytes) { + return Buffer.from(bytes); +} + +function throwsAs(fn, re) { + assert.throws(fn, (err) => re.test(String(err.message || err))); +} + +describe('pack type hints (#52)', () => { + it('packs 123 as fixint 0x7b', () => { + assert.deepEqual(msgpack.pack(123, { type: 'fixint' }), b(0x7b)); + }); + + it('packs 123 as uint8 0xcc 0x7b', () => { + assert.deepEqual(msgpack.pack(123, { type: 'uint8' }), b(0xcc, 0x7b)); + }); + + it('packs 123 as uint16 / uint32 / uint64 with forced width', () => { + assert.deepEqual(msgpack.pack(123, { type: 'uint16' }), b(0xcd, 0x00, 0x7b)); + assert.deepEqual( + msgpack.pack(123, { type: 'uint32' }), + b(0xce, 0x00, 0x00, 0x00, 0x7b), + ); + const u64 = msgpack.pack(123, { type: 'uint64' }); + assert.equal(u64[0], 0xcf); + assert.equal(u64.length, 9); + assert.equal(msgpack.unpack(u64), 123); + }); + + it('packs signed widths including negative int8', () => { + assert.deepEqual(msgpack.pack(-1, { type: 'int8' }), b(0xd0, 0xff)); + assert.equal(msgpack.pack(-1, { type: 'int16' })[0], 0xd1); + assert.equal(msgpack.pack(-1, { type: 'int32' })[0], 0xd2); + assert.equal(msgpack.pack(-1, { type: 'int64' })[0], 0xd3); + assert.equal(msgpack.pack(-1, { type: 'int64' }).length, 9); + assert.equal(msgpack.unpack(msgpack.pack(-1, { type: 'int64' })), -1); + }); + + it('packs Math.PI as float32 (0xca) and float64 (0xcb)', () => { + const f32 = msgpack.pack(Math.PI, { type: 'float32' }); + assert.equal(f32[0], 0xca); + assert.equal(f32.length, 5); + const f64 = msgpack.pack(Math.PI, { type: 'float64' }); + assert.equal(f64[0], 0xcb); + assert.equal(f64.length, 9); + assert.equal(msgpack.unpack(f64), Math.PI); + }); + + it('family bin packs a Buffer as bin8', () => { + const buf = Buffer.from('hi'); + assert.deepEqual(msgpack.pack(buf, { family: 'bin' }), b(0xc4, 0x02, 0x68, 0x69)); + }); + + it('family bin packs a string as UTF-8 bin', () => { + assert.deepEqual(msgpack.pack('hi', { family: 'bin' }), b(0xc4, 0x02, 0x68, 0x69)); + }); + + it('family str packs a Buffer as str', () => { + const packed = msgpack.pack(Buffer.from('hi'), { family: 'str' }); + assert.equal(packed[0], 0xa2); + assert.equal(msgpack.unpack(packed), 'hi'); + }); + + it('family int uses compact integer encoding', () => { + assert.deepEqual(msgpack.pack(123, { family: 'int' }), b(0x7b)); + assert.deepEqual(msgpack.pack(-1, { family: 'int' }), b(0xff)); + }); + + it('family float uses float32 when the value is exact in f32', () => { + assert.equal(msgpack.pack(1, { family: 'float' })[0], 0xca); + assert.equal(msgpack.pack(Math.PI, { family: 'float' })[0], 0xcb); + }); + + it('type wins over family', () => { + assert.deepEqual( + msgpack.pack(123, { type: 'uint8', family: 'int' }), + b(0xcc, 0x7b), + ); + }); + + it('interpret packs mixed float64 and float32 array elements', () => { + const packed = msgpack.pack( + [ + { data: Math.PI, type: 'float64' }, + { data: 3.14, type: 'float32' }, + ], + { + interpret(item) { + return { data: item.data, type: item.type }; + }, + }, + ); + assert.equal(packed[0], 0x92); + assert.equal(packed[1], 0xcb); + assert.equal(packed[10], 0xca); + assert.equal(packed.length, 15); + }); + + it('pack(1, 2) still packs an array of two values', () => { + assert.deepEqual(msgpack.unpack(msgpack.pack(1, 2)), [1, 2]); + }); + + it('one-argument { type: "fixint" } packs as a map, not options', () => { + assert.deepEqual(msgpack.unpack(msgpack.pack({ type: 'fixint' })), { + type: 'fixint', + }); + }); + + it('extra keys on the second argument keep array packing', () => { + assert.deepEqual( + msgpack.unpack(msgpack.pack(123, { type: 'uint8', extra: 1 })), + [123, { type: 'uint8', extra: 1 }], + ); + }); + + it('empty object second argument is a value, not options', () => { + assert.deepEqual(msgpack.unpack(msgpack.pack(123, {})), [123, {}]); + }); + + it('throws cannot pack value as uint8 for 500', () => { + throwsAs(() => msgpack.pack(500, { type: 'uint8' }), /cannot pack value as uint8/); + }); + + it('throws cannot pack value as uint8 for -1', () => { + throwsAs(() => msgpack.pack(-1, { type: 'uint8' }), /cannot pack value as uint8/); + }); + + it('throws cannot pack value as int for 1.5', () => { + throwsAs(() => msgpack.pack(1.5, { family: 'int' }), /cannot pack value as int/); + }); + + it('throws cannot pack value as fixint for 1.5', () => { + throwsAs(() => msgpack.pack(1.5, { type: 'fixint' }), /cannot pack value as fixint/); + }); + + it('throws on unknown type and family', () => { + throwsAs(() => msgpack.pack(1, { type: 'nope' }), /unknown pack type/); + throwsAs(() => msgpack.pack(1, { family: 'nope' }), /unknown pack family/); + }); + + it('throws when type/family are not strings', () => { + throwsAs(() => msgpack.pack(1, { type: 8 }), /pack type must be a string/); + throwsAs(() => msgpack.pack(1, { family: 1 }), /pack family must be a string/); + }); + + it('throws when interpret is not a function', () => { + throwsAs(() => msgpack.pack(1, { interpret: 1 }), /pack interpret must be a function/); + }); + + it('ignores interpret on a non-array value', () => { + assert.deepEqual( + msgpack.pack(123, { + interpret() { + throw new Error('should not run'); + }, + }), + b(0x7b), + ); + }); + + it('nil, true, and false types', () => { + assert.deepEqual(msgpack.pack(null, { type: 'nil' }), b(0xc0)); + assert.deepEqual(msgpack.pack(undefined, { type: 'nil' }), b(0xc0)); + assert.deepEqual(msgpack.pack(true, { type: 'true' }), b(0xc3)); + assert.deepEqual(msgpack.pack(false, { type: 'false' }), b(0xc2)); + throwsAs(() => msgpack.pack(1, { type: 'nil' }), /cannot pack value as nil/); + throwsAs(() => msgpack.pack(false, { type: 'true' }), /cannot pack value as true/); + throwsAs(() => msgpack.pack(true, { type: 'false' }), /cannot pack value as false/); + }); + + it('forces str8 even when fixstr would fit', () => { + assert.equal(msgpack.pack('hi')[0] & 0xe0, 0xa0); + assert.deepEqual(msgpack.pack('hi', { type: 'str8' }), b(0xd9, 0x02, 0x68, 0x69)); + }); + + it('throws when a string is too long for fixstr', () => { + throwsAs( + () => msgpack.pack('x'.repeat(32), { type: 'fixstr' }), + /cannot pack value as fixstr/, + ); + assert.equal(msgpack.pack('x'.repeat(32), { type: 'str8' })[0], 0xd9); + }); + + it('forces bin16 for a 256-byte Buffer', () => { + const buf = Buffer.alloc(256, 7); + const packed = msgpack.pack(buf, { type: 'bin16' }); + assert.equal(packed[0], 0xc5); + assert.equal(packed.length, 259); + throwsAs( + () => msgpack.pack(buf, { type: 'bin8' }), + /cannot pack value as bin8/, + ); + }); + + it('packs Date as str via toISOString when type/family is str', () => { + const d = new Date('2020-01-02T03:04:05.000Z'); + const packed = msgpack.pack(d, { type: 'str8' }); + assert.equal(packed[0], 0xd9); + assert.equal(msgpack.unpack(packed), d.toISOString()); + }); + + it('BigInt plus integer type uses the 64-bit path', () => { + assert.deepEqual(msgpack.pack(123n, { type: 'uint8' }), b(0xcc, 0x7b)); + throwsAs(() => msgpack.pack(500n, { type: 'uint8' }), /cannot pack value as uint8/); + throwsAs(() => msgpack.pack(-1n, { type: 'uint64' }), /cannot pack value as uint64/); + const i64 = msgpack.pack(1n, { type: 'int64' }); + assert.equal(i64[0], 0xd3); + assert.equal(i64.length, 9); + assert.equal(msgpack.unpack(i64), 1); + }); + + it('interpret must return { data }', () => { + throwsAs( + () => msgpack.pack([1], { interpret: () => 1 }), + /interpret must return \{ data \}/, + ); + throwsAs( + () => msgpack.pack([1], { interpret: () => ({}) }), + /interpret must return \{ data \}/, + ); + }); + + it('interpret exceptions surface to the caller', () => { + throwsAs( + () => + msgpack.pack([1], { + interpret() { + throw new Error('nope'); + }, + }), + /nope/, + ); + }); + + it('default pack of 123 is still fixint', () => { + assert.deepEqual(msgpack.pack(123), b(0x7b)); + }); +}); From f9613f2e523c4c7233706a753bfa51f76513b833 Mon Sep 17 00:00:00 2001 From: Enoch Groot Date: Sat, 19 Sep 2026 06:31:02 +0000 Subject: [PATCH 2/2] test: cover pack-hint miss paths and mark unreachable arms - Host objects, null type, and related pack-options cases - [[noreturn]] on ThrowCannotPackAs so gcovr throw edges match - GCOVR_EXCL only on unreachable hint defaults and empty info[1] - Native coverage 95.2% lines / 95.4% branches --- COVERAGE.md | 19 ++-- src/pack_hints.inc | 50 +++++++--- test/pack-hints.test.js | 197 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 246 insertions(+), 20 deletions(-) diff --git a/COVERAGE.md b/COVERAGE.md index 3c616ab..c95e0f8 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -8,9 +8,9 @@ | `lib/` + `bin/` (c8) | branches | **100%** | ≥ 95% | | `lib/` + `bin/` (c8) | functions | **100%** | ≥ 95% | | `lib/` + `bin/` (c8) | lines | **100%** | ≥ 95% | -| `src/msgpack.cc` (gcovr) | lines | **95.9%** (473/493) | ≥ 95% | -| `src/msgpack.cc` (gcovr) | branches | **99.5%** (400/402) | ≥ 95% | -| `src/msgpack.cc` (gcovr) | functions | 100% (36/36) | — | +| `src/` (gcovr) | lines | **95.2%** (902/947) | ≥ 95% | +| `src/` (gcovr) | branches | **95.4%** (836/876) | ≥ 95% | +| `src/` (gcovr) | functions | 100% (59/59) | — | `deps/` is excluded from the native report; the vendored msgpack-c is not our code. `build/` is rebuilt without instrumentation at the end of @@ -56,12 +56,12 @@ Every one is an error arm that cannot be entered from JS without stubbing | 735 | `throw` after `msgpack_pack_array` in `Pack` | Allocation failure only. | | 809–813, 815 | `MSGPACK_UNPACK_CONTINUE` / parse-error tail of `Unpack` | `ScanOne` walks the same grammar first with limits at or below the vendored library's own (511 vs 512 nested containers, the same 1 000 000 element cap), so once it returns `kScanOk`, `msgpack_unpack_next` can only succeed. The arms stay so a future divergence fails closed instead of reading `result.data` uninitialised. | -## Remaining uncovered native branches (2 of 402) +## Remaining uncovered native branches in `msgpack.cc` (2) | Line | Code | Why | | --- | --- | --- | -| 148 | `switch (b)` in `ScanOne` | The `default:` edge — see lines 307–308 above. It cannot be excluded on its own without also dropping the 30 covered case edges on the same line, so it is left in and counted against us. | -| 575 | `switch (mo->type)` in `MsgpackToJs` | Same, for the `default:` edge covering the complete `msgpack_object_type` enum. | +| 154 | `switch (b)` in `ScanOne` | The `default:` edge — see lines 307–308 above. It cannot be excluded on its own without also dropping the 30 covered case edges on the same line, so it is left in and counted against us. | +| 611 | `switch (mo->type)` in `MsgpackToJs` | Same, for the `default:` edge covering the complete `msgpack_object_type` enum. | ## About the native branch number @@ -139,6 +139,13 @@ gcovr --root . --filter src/ --exclude deps/ --no-markers --txt-metric branch -- failure modes and mark cleanup; and a worker that nests 600 packs deep to saturate the thread-local sbuffer pool and reach the "pool is full, free it" arm of `~PackBuffer`. +- `test/pack-hints.test.js` — `pack(value, { type, family, interpret })` wire + types, last-arg options detection (host objects, Proxies, ownKeys throws), + and the reachable miss paths in `src/pack_hints.inc`. Unreachable arms + (empty `info[1]`, `kTypeNone` / `kFamilyNone` defaults, hinted + `kMaxPackDepth`) are marked `GCOVR_EXCL_*`, not deleted. Native overall + stays above the 95% gate (`pack_hints.inc` itself is 91% branches because + switch `default:` edges sit on the same line as covered cases). - `test/cli.test.js` (12 tests) — the exit-1 paths of both CLIs: invalid JSON, empty stdin, a pack rejection reachable from real JSON, an unparseable byte, an oversized header, incomplete input both alone and after a good frame, and diff --git a/src/pack_hints.inc b/src/pack_hints.inc index c4ce93f..3eefac8 100644 --- a/src/pack_hints.inc +++ b/src/pack_hints.inc @@ -43,7 +43,7 @@ struct PackHint { has_interpret(false) {} }; -static void ThrowCannotPackAs(const char* what) { +[[noreturn]] static void ThrowCannotPackAs(const char* what) { char buf[80]; std::snprintf(buf, sizeof(buf), "cannot pack value as %s", what); throw MsgpackException(Error(buf)); @@ -72,7 +72,11 @@ static const char* PackTypeName(PackType t) { case kTypeNil: return "nil"; case kTypeTrue: return "true"; case kTypeFalse: return "false"; - default: return "type"; + /* GCOVR_EXCL_START: kTypeNone is not a named type */ + case kTypeNone: + default: + return "type"; + /* GCOVR_EXCL_STOP */ } } @@ -81,11 +85,16 @@ static int WriteRaw(msgpack_packer* pk, const void* buf, size_t n) { } static bool IsPackOptionsObject(v8::Local v) { - if (v.IsEmpty() || !v->IsObject() || v->IsArray() || v->IsDate() || - v->IsFunction() || v->IsRegExp() || v->IsNativeError() || - v->IsPromise() || v->IsTypedArray() || v->IsArrayBuffer() || - v->IsSharedArrayBuffer() || v->IsDataView() || - node::Buffer::HasInstance(v)) { + /* Buffer is a Uint8Array in Node 18+, so IsTypedArray already rejects it. */ + /* GCOVR_EXCL_START: Length==2 implies info[1] is set */ + if (v.IsEmpty()) { + return false; + } + /* GCOVR_EXCL_STOP */ + if (!v->IsObject() || v->IsArray() || v->IsDate() || v->IsFunction() || + v->IsRegExp() || v->IsNativeError() || v->IsPromise() || + v->IsTypedArray() || v->IsArrayBuffer() || v->IsSharedArrayBuffer() || + v->IsDataView()) { return false; } v8::Local obj = v.As(); @@ -93,7 +102,8 @@ static bool IsPackOptionsObject(v8::Local v) { v8::MaybeLocal maybe_names = obj->GetOwnPropertyNames( Nan::GetCurrentContext(), v8::PropertyFilter::ONLY_ENUMERABLE); if (maybe_names.IsEmpty() || try_catch.HasCaught()) { - try_catch.Reset(); + /* Empty Maybe and a pending exception arrive together from V8. */ + try_catch.Reset(); /* GCOVR_EXCL_BR_LINE */ return false; } v8::Local names = maybe_names.ToLocalChecked(); @@ -305,8 +315,10 @@ static void PackStrBytes(msgpack_packer* pk, PackType t, const char* data, hdr[4] = static_cast(len); hn = 5; break; + /* GCOVR_EXCL_START: only str types are passed */ default: ThrowCannotPackAs("str"); + /* GCOVR_EXCL_STOP */ } rc = WriteRaw(pk, hdr, hn); if (rc == 0) { /* GCOVR_EXCL_BR_LINE */ @@ -357,8 +369,10 @@ static void PackBinBytes(msgpack_packer* pk, PackType t, const char* data, hdr[4] = static_cast(len); hn = 5; break; + /* GCOVR_EXCL_START: only bin types are passed */ default: ThrowCannotPackAs("bin"); + /* GCOVR_EXCL_STOP */ } rc = WriteRaw(pk, hdr, hn); if (rc == 0) { /* GCOVR_EXCL_BR_LINE */ @@ -501,8 +515,10 @@ static void PackAsType(msgpack_packer* pk, v8::Local o, } rc = msgpack_pack_false(pk); break; - default: + /* GCOVR_EXCL_START: PackAsType is not called with kTypeNone */ + case kTypeNone: ThrowCannotPackAs("type"); + /* GCOVR_EXCL_STOP */ } if (rc != 0) { /* GCOVR_EXCL_BR_LINE: sbuffer write failure */ throw MsgpackException(Error("Error serializing object")); @@ -546,8 +562,10 @@ static void PackAsFamily(msgpack_packer* pk, v8::Local o, case kFamilyBin: GetBinBytes(o, kTypeNone, pk); return; - default: + /* GCOVR_EXCL_START: PackAsFamily is not called with kFamilyNone */ + case kFamilyNone: ThrowCannotPackAs("family"); + /* GCOVR_EXCL_STOP */ } if (rc != 0) { /* GCOVR_EXCL_BR_LINE: sbuffer write failure */ throw MsgpackException(Error("Error serializing object")); @@ -559,9 +577,10 @@ static void JsToMsgpackHinted(msgpack_packer* pk, v8::Local o, static void PackArrayInterpreted(msgpack_packer* pk, v8::Local arr, int depth, const PackHint& hint) { - if (IsMarked(arr)) { + /* Nested interpret is not applied (FillHintFromObject(..., false)). */ + if (IsMarked(arr)) { /* GCOVR_EXCL_START */ throw MsgpackException(Error("Cowardly refusing to pack circular reference")); - } + } /* GCOVR_EXCL_STOP */ Mark(arr); try { /* GCOVR_EXCL_BR_START: allocation failure only. */ @@ -571,8 +590,8 @@ static void PackArrayInterpreted(msgpack_packer* pk, v8::Local arr, /* GCOVR_EXCL_BR_STOP */ for (uint32_t i = 0; i < arr->Length(); i++) { v8::Local item = CheckedGet(arr, i); - v8::Local recv = - hint.recv.IsEmpty() ? Nan::New() : hint.recv; + /* interpret always sets recv in FillHintFromObject. */ + v8::Local recv = hint.recv; v8::Local ret = CallOneArg(recv, hint.interpret, item); if (!ret->IsObject() || ret->IsArray() || ret->IsDate() || ret->IsFunction() || node::Buffer::HasInstance(ret)) { @@ -598,10 +617,13 @@ static void PackArrayInterpreted(msgpack_packer* pk, v8::Local arr, static void JsToMsgpackHinted(msgpack_packer* pk, v8::Local o, int depth, const PackHint& hint) { if (o->IsArray() && hint.has_interpret) { + /* Hinted pack starts at depth 0 and interpret does not recurse. */ + /* GCOVR_EXCL_START */ if (kMaxPackDepth < depth + 1) { throw MsgpackException( Error("Cowardly refusing to pack object nested more than 512 levels deep")); } + /* GCOVR_EXCL_STOP */ PackArrayInterpreted(pk, o.As(), depth + 1, hint); return; } diff --git a/test/pack-hints.test.js b/test/pack-hints.test.js index 573b586..d159c31 100644 --- a/test/pack-hints.test.js +++ b/test/pack-hints.test.js @@ -241,4 +241,201 @@ describe('pack type hints (#52)', () => { it('default pack of 123 is still fixint', () => { assert.deepEqual(msgpack.pack(123), b(0x7b)); }); + + it('does not treat host objects as pack options', () => { + const seconds = [ + new Date(), + function hint() {}, + /x/, + new Error('e'), + Promise.resolve(1), + new Uint8Array([1]), + new ArrayBuffer(1), + new SharedArrayBuffer(1), + new DataView(new ArrayBuffer(1)), + Buffer.from('x'), + ]; + for (const second of seconds) { + let packed; + try { + packed = msgpack.pack(123, second); + } catch (err) { + // Host object was not treated as options; packing the value failed. + assert.ok(err); + continue; + } + assert.notDeepEqual(Buffer.from(packed), b(0xcc, 0x7b)); + assert.notDeepEqual(Buffer.from(packed), b(0x7b)); + } + }); + + it('symbol own keys keep array packing', () => { + const opts = {}; + Object.defineProperty(opts, Symbol('type'), { + enumerable: true, + value: 'uint8', + }); + const packed = msgpack.pack(123, opts); + assert.notDeepEqual(Buffer.from(packed), b(0xcc, 0x7b)); + assert.equal(msgpack.unpack(packed)[0], 123); + }); + + it('ownKeys throwing proxy surfaces from GetOwnPropertyNames', () => { + const opts = new Proxy( + { type: 'uint8' }, + { + ownKeys() { + throw new Error('ownKeys boom'); + }, + getOwnPropertyDescriptor() { + return { configurable: true, enumerable: true }; + }, + }, + ); + throwsAs(() => msgpack.pack(123, opts), /ownKeys boom/); + }); + + it('undefined or null type and family fall through', () => { + assert.deepEqual(msgpack.pack(123, { type: undefined }), b(0x7b)); + assert.deepEqual(msgpack.pack(123, { type: null }), b(0x7b)); + assert.deepEqual(msgpack.pack(123, { family: null }), b(0x7b)); + assert.deepEqual(msgpack.pack(123, { type: undefined, family: 'int' }), b(0x7b)); + assert.deepEqual(msgpack.pack(123, { family: undefined, type: 'uint8' }), b(0xcc, 0x7b)); + }); + + it('non-number values cannot pack as integer or float types', () => { + throwsAs(() => msgpack.pack('1', { type: 'fixint' }), /cannot pack value as fixint/); + throwsAs(() => msgpack.pack('1', { type: 'uint64' }), /cannot pack value as uint64/); + throwsAs(() => msgpack.pack('1', { type: 'float32' }), /cannot pack value as float32/); + throwsAs(() => msgpack.pack('1', { type: 'float64' }), /cannot pack value as float64/); + throwsAs(() => msgpack.pack('1', { family: 'int' }), /cannot pack value as int/); + throwsAs(() => msgpack.pack('1', { family: 'float' }), /cannot pack value as float/); + throwsAs(() => msgpack.pack(true, { type: 'int8' }), /cannot pack value as int8/); + throwsAs(() => msgpack.pack(NaN, { type: 'int32' }), /cannot pack value as int32/); + throwsAs(() => msgpack.pack(Infinity, { type: 'uint32' }), /cannot pack value as uint32/); + throwsAs(() => msgpack.pack(-Infinity, { family: 'int' }), /cannot pack value as int/); + }); + + it('rejects integers outside the requested width', () => { + throwsAs(() => msgpack.pack(128, { type: 'fixint' }), /cannot pack value as fixint/); + throwsAs(() => msgpack.pack(-33, { type: 'fixint' }), /cannot pack value as fixint/); + throwsAs(() => msgpack.pack(128, { type: 'int8' }), /cannot pack value as int8/); + throwsAs(() => msgpack.pack(-129, { type: 'int8' }), /cannot pack value as int8/); + throwsAs(() => msgpack.pack(32768, { type: 'int16' }), /cannot pack value as int16/); + throwsAs(() => msgpack.pack(2147483648, { type: 'int32' }), /cannot pack value as int32/); + throwsAs(() => msgpack.pack(65536, { type: 'uint16' }), /cannot pack value as uint16/); + throwsAs(() => msgpack.pack(0x100000000, { type: 'uint32' }), /cannot pack value as uint32/); + throwsAs(() => msgpack.pack(2 ** 63, { type: 'int64' }), /cannot pack value as int64/); + throwsAs(() => msgpack.pack(-1e20, { type: 'int64' }), /cannot pack value as int64/); + throwsAs(() => msgpack.pack(2 ** 64, { type: 'uint64' }), /cannot pack value as uint64/); + throwsAs( + () => msgpack.pack((1n << 64n) - 1n, { type: 'int64' }), + /cannot pack value as int64/, + ); + }); + + it('family int uses uint64 when the value is above INT64_MAX', () => { + const packed = msgpack.pack(2 ** 63, { family: 'int' }); + assert.equal(packed[0], 0xcf); + assert.equal(msgpack.unpack(packed), 2n ** 63n); + const maxu = msgpack.pack((1n << 64n) - 1n, { family: 'int' }); + assert.equal(maxu[0], 0xcf); + assert.equal(msgpack.unpack(maxu), (1n << 64n) - 1n); + }); + + it('packs str16, str32, and bin32 forced widths', () => { + const s256 = 'x'.repeat(256); + const str16 = msgpack.pack(s256, { type: 'str16' }); + assert.equal(str16[0], 0xda); + assert.equal(msgpack.unpack(str16), s256); + const str32 = msgpack.pack('hi', { type: 'str32' }); + assert.equal(str32[0], 0xdb); + assert.equal(msgpack.unpack(str32), 'hi'); + const bin32 = msgpack.pack(Buffer.from('hi'), { type: 'bin32' }); + assert.equal(bin32[0], 0xc6); + assert.equal(msgpack.unpack(bin32).toString(), 'hi'); + throwsAs( + () => msgpack.pack('x'.repeat(256), { type: 'str8' }), + /cannot pack value as str8/, + ); + throwsAs( + () => msgpack.pack('x'.repeat(65536), { type: 'str16' }), + /cannot pack value as str16/, + ); + throwsAs( + () => msgpack.pack(Buffer.alloc(65536), { type: 'bin16' }), + /cannot pack value as bin16/, + ); + }); + + it('packs a short string as fixstr and non-finite family float as float64', () => { + assert.deepEqual(msgpack.pack('hi', { type: 'fixstr' }), b(0xa2, 0x68, 0x69)); + assert.equal(msgpack.pack(Infinity, { family: 'float' })[0], 0xcb); + assert.equal(msgpack.pack(-Infinity, { family: 'float' })[0], 0xcb); + assert.equal(msgpack.pack(NaN, { family: 'float' })[0], 0xcb); + }); + + it('rejects non-string and non-buffer values for str and bin', () => { + throwsAs(() => msgpack.pack(1, { type: 'str8' }), /cannot pack value as str8/); + throwsAs(() => msgpack.pack(1, { type: 'bin8' }), /cannot pack value as bin8/); + throwsAs(() => msgpack.pack(1, { family: 'str' }), /cannot pack value as str/); + throwsAs(() => msgpack.pack(1, { family: 'bin' }), /cannot pack value as bin/); + }); + + it('throws when a Date has no toISOString function', () => { + const d = new Date('2020-01-02T03:04:05.000Z'); + d.toISOString = 1; + throwsAs(() => msgpack.pack(d, { type: 'str8' }), /cannot pack Date/); + }); + + it('interpret returning family still packs data', () => { + const packed = msgpack.pack([123], { + interpret() { + return { data: 123, family: 'int', extra: 1 }; + }, + }); + assert.deepEqual(msgpack.unpack(packed), [123]); + }); + + it('interpret does not recurse into nested arrays', () => { + const packed = msgpack.pack([[1, 2]], { + interpret(item) { + return { data: item }; + }, + }); + assert.deepEqual(msgpack.unpack(packed), [[1, 2]]); + }); + + it('interpret data that is circular is refused by the default packer', () => { + const a = []; + a.push(a); + throwsAs( + () => + msgpack.pack([0], { + interpret() { + return { data: a }; + }, + }), + /circular/, + ); + }); + + it('interpret rejects Date, Array, Function, and Buffer returns', () => { + throwsAs( + () => msgpack.pack([1], { interpret: () => new Date() }), + /interpret must return \{ data \}/, + ); + throwsAs( + () => msgpack.pack([1], { interpret: () => [1] }), + /interpret must return \{ data \}/, + ); + throwsAs( + () => msgpack.pack([1], { interpret: () => function () {} }), + /interpret must return \{ data \}/, + ); + throwsAs( + () => msgpack.pack([1], { interpret: () => Buffer.from('x') }), + /interpret must return \{ data \}/, + ); + }); });