fix(parser): defensive Bitmap copy and Struct prototype-chain guard by RobinBol · Pull Request #44 · athombv/node-data-types

RobinBol · 2026-05-20T13:53:00Z

Summary

Two small robustness fixes to the parser primitives. Both have zero behavior impact on correct callers; both close real footguns. Each lands with a regression-locking test.

1. `Bitmap.fromBuffer` now owns its bytes

Bitmap.fromBuffer previously stored buf.slice(i, i + len) as the bitmap's backing store. In Node, `Buffer.slice` returns a view sharing memory with the source. If anything mutated the source after parsing (e.g. a radio driver reusing its receive buffer across frames), the bits read from an already-parsed Bitmap would silently change - a TOCTOU-style hazard that depends on caller buffer-management behavior.

Wrapping the slice in `Buffer.from(...)` makes the Bitmap own its bytes. One-character diff.

2. `Struct` rejects prototype-chain keys

`Struct`'s constructor used `if (!defs[key]) throw` to validate incoming field names. That check walks the prototype chain, so `defs.constructor` resolves to `Object` (truthy), and `constructor` is silently accepted as a valid field name - quietly shadowing the instance's `.constructor` reference. `toString`, `valueOf`, etc. were affected the same way.

Switching to `Object.prototype.hasOwnProperty.call(defs, key)` restricts acceptance to declared own-property fields. `proto` was already rejected (its prototype-chain value is treated as the unexpected key); this aligns the rest of the prototype-chain keys with the same behavior.

Test plan

New test in `test/DataTypes.test.js`: parse a map8 from a source buffer, mutate the source, read the bitmap again - bits must be unchanged.
New tests in `test/Struct.test.js`: `constructor`, `toString` rejected; declared and undeclared field names handled as before.
Full suite: 14/14 passing (9 pre-existing + 5 new). No regressions.
Lint and typecheck clean.
Verified against downstream consumer `node-zigbee-clusters` (81/81 tests pass against the patched build).

Out of scope

The silent zero-fallback behavior in `uintFromBuf` / `enumFromBuf` / `dataFromBuf` / `bufferFromBuf` on truncated input is a separate, larger discussion (it's a breaking change to make those throw). The downstream consumer `node-zigbee-clusters` has separately added a strict length precheck around command-arg parsing (athombv/node-zigbee-clusters#193), which addresses the most consequential consumer impact without requiring changes here.

Two small robustness fixes: 1. `Bitmap.fromBuffer` previously stored the result of `buf.slice(...)`, which in Node returns a Buffer that shares memory with the source. If the source buffer was mutated after parsing (e.g. a radio driver reusing its receive buffer across frames), the bits read from an already-parsed Bitmap would change. Wrap the slice in `Buffer.from(...)` so the Bitmap owns its bytes. 2. `Struct`'s constructor used `!defs[key]` to validate field names. That check walks the prototype chain, so `defs.constructor` resolves to `Object` (truthy) and `constructor` is accepted as a valid field name, silently overwriting the instance's `.constructor` reference. Use `Object.prototype.hasOwnProperty.call(defs, key)` instead so only declared own-property fields are accepted. Other prototype-chain keys (`toString`, `valueOf`, ...) are also now correctly rejected. Tests added for both. Neither change affects behavior for any correct caller.

Copilot

Pull request overview

This PR strengthens two core parser primitives to avoid subtle footguns: (1) preventing Bitmap instances from aliasing caller buffers, and (2) tightening Struct construction to reject unexpected keys that come from the prototype chain.

Changes:

Make Bitmap.fromBuffer copy its backing bytes so later mutations of the source buffer can’t change already-parsed bitmaps.
Validate Struct constructor input keys using hasOwnProperty (instead of truthiness lookup) to reject prototype-chain keys like constructor/toString.
Add regression tests covering buffer aliasing and prototype-chain key rejection.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
lib/DataTypes.js	Copies bitmap bytes in `Bitmap.fromBuffer` to prevent source-buffer aliasing.
lib/Struct.js	Switches unexpected-property validation to `hasOwnProperty` to block prototype-chain keys.
test/DataTypes.test.js	Adds regression test ensuring parsed bitmaps don’t change when the source buffer is mutated.
test/Struct.test.js	Adds tests ensuring prototype-chain keys are rejected while normal declared/undeclared behavior remains.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Address Copilot review feedback on #44: the throw path previously used `this.constructor.name` for the error prefix. If a struct legitimately declares a `constructor` field and the caller sets it before an unexpected key is encountered, `this.constructor` is already the user-supplied value and `.name` would crash or be undefined. Switch to the closed-over `name` parameter, which is lexically bound and cannot be shadowed by instance assignments. Adds a regression test for the edge case.

…sages Followup to the previous commit. Closed-over `name` was correct on the shadow-resistance axis but lost the subclass-aware error message that `this.constructor.name` originally provided: when a Struct-generated class is subclassed, the error should identify the actual instantiated subclass, not the underlying Struct name. `new.target` is a syntactic binding to the constructor actually invoked via `new`. It reflects the subclass identity AND cannot be shadowed by a `constructor` field the caller may have set earlier in the same loop. Strictly better than both `this.constructor.name` and closed-over `name`. Adds a subclassing regression test so this property is locked in.

RobinBol · 2026-05-20T13:53:00Z

Summary

Two small robustness fixes to the parser primitives. Both have zero behavior impact on correct callers; both close real footguns. Each lands with a regression-locking test.

1. `Bitmap.fromBuffer` now owns its bytes

Bitmap.fromBuffer previously stored buf.slice(i, i + len) as the bitmap's backing store. In Node, `Buffer.slice` returns a view sharing memory with the source. If anything mutated the source after parsing (e.g. a radio driver reusing its receive buffer across frames), the bits read from an already-parsed Bitmap would silently change - a TOCTOU-style hazard that depends on caller buffer-management behavior.

Wrapping the slice in `Buffer.from(...)` makes the Bitmap own its bytes. One-character diff.

2. `Struct` rejects prototype-chain keys

`Struct`'s constructor used `if (!defs[key]) throw` to validate incoming field names. That check walks the prototype chain, so `defs.constructor` resolves to `Object` (truthy), and `constructor` is silently accepted as a valid field name - quietly shadowing the instance's `.constructor` reference. `toString`, `valueOf`, etc. were affected the same way.

Switching to `Object.prototype.hasOwnProperty.call(defs, key)` restricts acceptance to declared own-property fields. `proto` was already rejected (its prototype-chain value is treated as the unexpected key); this aligns the rest of the prototype-chain keys with the same behavior.

Test plan

New test in `test/DataTypes.test.js`: parse a map8 from a source buffer, mutate the source, read the bitmap again - bits must be unchanged.
New tests in `test/Struct.test.js`: `constructor`, `toString` rejected; declared and undeclared field names handled as before.
Full suite: 14/14 passing (9 pre-existing + 5 new). No regressions.
Lint and typecheck clean.
Verified against downstream consumer `node-zigbee-clusters` (81/81 tests pass against the patched build).

Out of scope

The silent zero-fallback behavior in `uintFromBuf` / `enumFromBuf` / `dataFromBuf` / `bufferFromBuf` on truncated input is a separate, larger discussion (it's a breaking change to make those throw). The downstream consumer `node-zigbee-clusters` has separately added a strict length precheck around command-arg parsing (athombv/node-zigbee-clusters#193), which addresses the most consequential consumer impact without requiring changes here.

Two small robustness fixes: 1. `Bitmap.fromBuffer` previously stored the result of `buf.slice(...)`, which in Node returns a Buffer that shares memory with the source. If the source buffer was mutated after parsing (e.g. a radio driver reusing its receive buffer across frames), the bits read from an already-parsed Bitmap would change. Wrap the slice in `Buffer.from(...)` so the Bitmap owns its bytes. 2. `Struct`'s constructor used `!defs[key]` to validate field names. That check walks the prototype chain, so `defs.constructor` resolves to `Object` (truthy) and `constructor` is accepted as a valid field name, silently overwriting the instance's `.constructor` reference. Use `Object.prototype.hasOwnProperty.call(defs, key)` instead so only declared own-property fields are accepted. Other prototype-chain keys (`toString`, `valueOf`, ...) are also now correctly rejected. Tests added for both. Neither change affects behavior for any correct caller.

Copilot

Pull request overview

This PR strengthens two core parser primitives to avoid subtle footguns: (1) preventing Bitmap instances from aliasing caller buffers, and (2) tightening Struct construction to reject unexpected keys that come from the prototype chain.

Changes:

Make Bitmap.fromBuffer copy its backing bytes so later mutations of the source buffer can’t change already-parsed bitmaps.
Validate Struct constructor input keys using hasOwnProperty (instead of truthiness lookup) to reject prototype-chain keys like constructor/toString.
Add regression tests covering buffer aliasing and prototype-chain key rejection.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
lib/DataTypes.js	Copies bitmap bytes in `Bitmap.fromBuffer` to prevent source-buffer aliasing.
lib/Struct.js	Switches unexpected-property validation to `hasOwnProperty` to block prototype-chain keys.
test/DataTypes.test.js	Adds regression test ensuring parsed bitmaps don’t change when the source buffer is mutated.
test/Struct.test.js	Adds tests ensuring prototype-chain keys are rejected while normal declared/undeclared behavior remains.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Address Copilot review feedback on #44: the throw path previously used `this.constructor.name` for the error prefix. If a struct legitimately declares a `constructor` field and the caller sets it before an unexpected key is encountered, `this.constructor` is already the user-supplied value and `.name` would crash or be undefined. Switch to the closed-over `name` parameter, which is lexically bound and cannot be shadowed by instance assignments. Adds a regression test for the edge case.

…sages Followup to the previous commit. Closed-over `name` was correct on the shadow-resistance axis but lost the subclass-aware error message that `this.constructor.name` originally provided: when a Struct-generated class is subclassed, the error should identify the actual instantiated subclass, not the underlying Struct name. `new.target` is a syntactic binding to the constructor actually invoked via `new`. It reflects the subclass identity AND cannot be shadowed by a `constructor` field the caller may have set earlier in the same loop. Strictly better than both `this.constructor.name` and closed-over `name`. Adds a subclassing regression test so this property is locked in.

RobinBol · 2026-05-20T13:53:00Z

Summary

Two small robustness fixes to the parser primitives. Both have zero behavior impact on correct callers; both close real footguns. Each lands with a regression-locking test.

1. `Bitmap.fromBuffer` now owns its bytes

Bitmap.fromBuffer previously stored buf.slice(i, i + len) as the bitmap's backing store. In Node, `Buffer.slice` returns a view sharing memory with the source. If anything mutated the source after parsing (e.g. a radio driver reusing its receive buffer across frames), the bits read from an already-parsed Bitmap would silently change - a TOCTOU-style hazard that depends on caller buffer-management behavior.

Wrapping the slice in `Buffer.from(...)` makes the Bitmap own its bytes. One-character diff.

2. `Struct` rejects prototype-chain keys

`Struct`'s constructor used `if (!defs[key]) throw` to validate incoming field names. That check walks the prototype chain, so `defs.constructor` resolves to `Object` (truthy), and `constructor` is silently accepted as a valid field name - quietly shadowing the instance's `.constructor` reference. `toString`, `valueOf`, etc. were affected the same way.

Switching to `Object.prototype.hasOwnProperty.call(defs, key)` restricts acceptance to declared own-property fields. `proto` was already rejected (its prototype-chain value is treated as the unexpected key); this aligns the rest of the prototype-chain keys with the same behavior.

Test plan

New test in `test/DataTypes.test.js`: parse a map8 from a source buffer, mutate the source, read the bitmap again - bits must be unchanged.
New tests in `test/Struct.test.js`: `constructor`, `toString` rejected; declared and undeclared field names handled as before.
Full suite: 14/14 passing (9 pre-existing + 5 new). No regressions.
Lint and typecheck clean.
Verified against downstream consumer `node-zigbee-clusters` (81/81 tests pass against the patched build).

Out of scope

The silent zero-fallback behavior in `uintFromBuf` / `enumFromBuf` / `dataFromBuf` / `bufferFromBuf` on truncated input is a separate, larger discussion (it's a breaking change to make those throw). The downstream consumer `node-zigbee-clusters` has separately added a strict length precheck around command-arg parsing (athombv/node-zigbee-clusters#193), which addresses the most consequential consumer impact without requiring changes here.

Two small robustness fixes: 1. `Bitmap.fromBuffer` previously stored the result of `buf.slice(...)`, which in Node returns a Buffer that shares memory with the source. If the source buffer was mutated after parsing (e.g. a radio driver reusing its receive buffer across frames), the bits read from an already-parsed Bitmap would change. Wrap the slice in `Buffer.from(...)` so the Bitmap owns its bytes. 2. `Struct`'s constructor used `!defs[key]` to validate field names. That check walks the prototype chain, so `defs.constructor` resolves to `Object` (truthy) and `constructor` is accepted as a valid field name, silently overwriting the instance's `.constructor` reference. Use `Object.prototype.hasOwnProperty.call(defs, key)` instead so only declared own-property fields are accepted. Other prototype-chain keys (`toString`, `valueOf`, ...) are also now correctly rejected. Tests added for both. Neither change affects behavior for any correct caller.

Copilot

Pull request overview

This PR strengthens two core parser primitives to avoid subtle footguns: (1) preventing Bitmap instances from aliasing caller buffers, and (2) tightening Struct construction to reject unexpected keys that come from the prototype chain.

Changes:

Make Bitmap.fromBuffer copy its backing bytes so later mutations of the source buffer can’t change already-parsed bitmaps.
Validate Struct constructor input keys using hasOwnProperty (instead of truthiness lookup) to reject prototype-chain keys like constructor/toString.
Add regression tests covering buffer aliasing and prototype-chain key rejection.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
lib/DataTypes.js	Copies bitmap bytes in `Bitmap.fromBuffer` to prevent source-buffer aliasing.
lib/Struct.js	Switches unexpected-property validation to `hasOwnProperty` to block prototype-chain keys.
test/DataTypes.test.js	Adds regression test ensuring parsed bitmaps don’t change when the source buffer is mutated.
test/Struct.test.js	Adds tests ensuring prototype-chain keys are rejected while normal declared/undeclared behavior remains.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Address Copilot review feedback on #44: the throw path previously used `this.constructor.name` for the error prefix. If a struct legitimately declares a `constructor` field and the caller sets it before an unexpected key is encountered, `this.constructor` is already the user-supplied value and `.name` would crash or be undefined. Switch to the closed-over `name` parameter, which is lexically bound and cannot be shadowed by instance assignments. Adds a regression test for the edge case.

…sages Followup to the previous commit. Closed-over `name` was correct on the shadow-resistance axis but lost the subclass-aware error message that `this.constructor.name` originally provided: when a Struct-generated class is subclassed, the error should identify the actual instantiated subclass, not the underlying Struct name. `new.target` is a syntactic binding to the constructor actually invoked via `new`. It reflects the subclass identity AND cannot be shadowed by a `constructor` field the caller may have set earlier in the same loop. Strictly better than both `this.constructor.name` and closed-over `name`. Adds a subclassing regression test so this property is locked in.

RobinBol · 2026-05-20T13:53:00Z

Summary

Two small robustness fixes to the parser primitives. Both have zero behavior impact on correct callers; both close real footguns. Each lands with a regression-locking test.

1. `Bitmap.fromBuffer` now owns its bytes

Bitmap.fromBuffer previously stored buf.slice(i, i + len) as the bitmap's backing store. In Node, `Buffer.slice` returns a view sharing memory with the source. If anything mutated the source after parsing (e.g. a radio driver reusing its receive buffer across frames), the bits read from an already-parsed Bitmap would silently change - a TOCTOU-style hazard that depends on caller buffer-management behavior.

Wrapping the slice in `Buffer.from(...)` makes the Bitmap own its bytes. One-character diff.

2. `Struct` rejects prototype-chain keys

`Struct`'s constructor used `if (!defs[key]) throw` to validate incoming field names. That check walks the prototype chain, so `defs.constructor` resolves to `Object` (truthy), and `constructor` is silently accepted as a valid field name - quietly shadowing the instance's `.constructor` reference. `toString`, `valueOf`, etc. were affected the same way.

Switching to `Object.prototype.hasOwnProperty.call(defs, key)` restricts acceptance to declared own-property fields. `proto` was already rejected (its prototype-chain value is treated as the unexpected key); this aligns the rest of the prototype-chain keys with the same behavior.

Test plan

New test in `test/DataTypes.test.js`: parse a map8 from a source buffer, mutate the source, read the bitmap again - bits must be unchanged.
New tests in `test/Struct.test.js`: `constructor`, `toString` rejected; declared and undeclared field names handled as before.
Full suite: 14/14 passing (9 pre-existing + 5 new). No regressions.
Lint and typecheck clean.
Verified against downstream consumer `node-zigbee-clusters` (81/81 tests pass against the patched build).

Out of scope

The silent zero-fallback behavior in `uintFromBuf` / `enumFromBuf` / `dataFromBuf` / `bufferFromBuf` on truncated input is a separate, larger discussion (it's a breaking change to make those throw). The downstream consumer `node-zigbee-clusters` has separately added a strict length precheck around command-arg parsing (athombv/node-zigbee-clusters#193), which addresses the most consequential consumer impact without requiring changes here.

Two small robustness fixes: 1. `Bitmap.fromBuffer` previously stored the result of `buf.slice(...)`, which in Node returns a Buffer that shares memory with the source. If the source buffer was mutated after parsing (e.g. a radio driver reusing its receive buffer across frames), the bits read from an already-parsed Bitmap would change. Wrap the slice in `Buffer.from(...)` so the Bitmap owns its bytes. 2. `Struct`'s constructor used `!defs[key]` to validate field names. That check walks the prototype chain, so `defs.constructor` resolves to `Object` (truthy) and `constructor` is accepted as a valid field name, silently overwriting the instance's `.constructor` reference. Use `Object.prototype.hasOwnProperty.call(defs, key)` instead so only declared own-property fields are accepted. Other prototype-chain keys (`toString`, `valueOf`, ...) are also now correctly rejected. Tests added for both. Neither change affects behavior for any correct caller.

Copilot

Pull request overview

This PR strengthens two core parser primitives to avoid subtle footguns: (1) preventing Bitmap instances from aliasing caller buffers, and (2) tightening Struct construction to reject unexpected keys that come from the prototype chain.

Changes:

Make Bitmap.fromBuffer copy its backing bytes so later mutations of the source buffer can’t change already-parsed bitmaps.
Validate Struct constructor input keys using hasOwnProperty (instead of truthiness lookup) to reject prototype-chain keys like constructor/toString.
Add regression tests covering buffer aliasing and prototype-chain key rejection.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
lib/DataTypes.js	Copies bitmap bytes in `Bitmap.fromBuffer` to prevent source-buffer aliasing.
lib/Struct.js	Switches unexpected-property validation to `hasOwnProperty` to block prototype-chain keys.
test/DataTypes.test.js	Adds regression test ensuring parsed bitmaps don’t change when the source buffer is mutated.
test/Struct.test.js	Adds tests ensuring prototype-chain keys are rejected while normal declared/undeclared behavior remains.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Address Copilot review feedback on #44: the throw path previously used `this.constructor.name` for the error prefix. If a struct legitimately declares a `constructor` field and the caller sets it before an unexpected key is encountered, `this.constructor` is already the user-supplied value and `.name` would crash or be undefined. Switch to the closed-over `name` parameter, which is lexically bound and cannot be shadowed by instance assignments. Adds a regression test for the edge case.

…sages Followup to the previous commit. Closed-over `name` was correct on the shadow-resistance axis but lost the subclass-aware error message that `this.constructor.name` originally provided: when a Struct-generated class is subclassed, the error should identify the actual instantiated subclass, not the underlying Struct name. `new.target` is a syntactic binding to the constructor actually invoked via `new`. It reflects the subclass identity AND cannot be shadowed by a `constructor` field the caller may have set earlier in the same loop. Strictly better than both `this.constructor.name` and closed-over `name`. Adds a subclassing regression test so this property is locked in.

RobinBol · 2026-05-20T13:53:00Z

Summary

Two small robustness fixes to the parser primitives. Both have zero behavior impact on correct callers; both close real footguns. Each lands with a regression-locking test.

1. `Bitmap.fromBuffer` now owns its bytes

Bitmap.fromBuffer previously stored buf.slice(i, i + len) as the bitmap's backing store. In Node, `Buffer.slice` returns a view sharing memory with the source. If anything mutated the source after parsing (e.g. a radio driver reusing its receive buffer across frames), the bits read from an already-parsed Bitmap would silently change - a TOCTOU-style hazard that depends on caller buffer-management behavior.

Wrapping the slice in `Buffer.from(...)` makes the Bitmap own its bytes. One-character diff.

2. `Struct` rejects prototype-chain keys

`Struct`'s constructor used `if (!defs[key]) throw` to validate incoming field names. That check walks the prototype chain, so `defs.constructor` resolves to `Object` (truthy), and `constructor` is silently accepted as a valid field name - quietly shadowing the instance's `.constructor` reference. `toString`, `valueOf`, etc. were affected the same way.

Switching to `Object.prototype.hasOwnProperty.call(defs, key)` restricts acceptance to declared own-property fields. `proto` was already rejected (its prototype-chain value is treated as the unexpected key); this aligns the rest of the prototype-chain keys with the same behavior.

Test plan

New test in `test/DataTypes.test.js`: parse a map8 from a source buffer, mutate the source, read the bitmap again - bits must be unchanged.
New tests in `test/Struct.test.js`: `constructor`, `toString` rejected; declared and undeclared field names handled as before.
Full suite: 14/14 passing (9 pre-existing + 5 new). No regressions.
Lint and typecheck clean.
Verified against downstream consumer `node-zigbee-clusters` (81/81 tests pass against the patched build).

Out of scope

The silent zero-fallback behavior in `uintFromBuf` / `enumFromBuf` / `dataFromBuf` / `bufferFromBuf` on truncated input is a separate, larger discussion (it's a breaking change to make those throw). The downstream consumer `node-zigbee-clusters` has separately added a strict length precheck around command-arg parsing (athombv/node-zigbee-clusters#193), which addresses the most consequential consumer impact without requiring changes here.

Two small robustness fixes: 1. `Bitmap.fromBuffer` previously stored the result of `buf.slice(...)`, which in Node returns a Buffer that shares memory with the source. If the source buffer was mutated after parsing (e.g. a radio driver reusing its receive buffer across frames), the bits read from an already-parsed Bitmap would change. Wrap the slice in `Buffer.from(...)` so the Bitmap owns its bytes. 2. `Struct`'s constructor used `!defs[key]` to validate field names. That check walks the prototype chain, so `defs.constructor` resolves to `Object` (truthy) and `constructor` is accepted as a valid field name, silently overwriting the instance's `.constructor` reference. Use `Object.prototype.hasOwnProperty.call(defs, key)` instead so only declared own-property fields are accepted. Other prototype-chain keys (`toString`, `valueOf`, ...) are also now correctly rejected. Tests added for both. Neither change affects behavior for any correct caller.

Copilot

Pull request overview

This PR strengthens two core parser primitives to avoid subtle footguns: (1) preventing Bitmap instances from aliasing caller buffers, and (2) tightening Struct construction to reject unexpected keys that come from the prototype chain.

Changes:

Make Bitmap.fromBuffer copy its backing bytes so later mutations of the source buffer can’t change already-parsed bitmaps.
Validate Struct constructor input keys using hasOwnProperty (instead of truthiness lookup) to reject prototype-chain keys like constructor/toString.
Add regression tests covering buffer aliasing and prototype-chain key rejection.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
lib/DataTypes.js	Copies bitmap bytes in `Bitmap.fromBuffer` to prevent source-buffer aliasing.
lib/Struct.js	Switches unexpected-property validation to `hasOwnProperty` to block prototype-chain keys.
test/DataTypes.test.js	Adds regression test ensuring parsed bitmaps don’t change when the source buffer is mutated.
test/Struct.test.js	Adds tests ensuring prototype-chain keys are rejected while normal declared/undeclared behavior remains.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Address Copilot review feedback on #44: the throw path previously used `this.constructor.name` for the error prefix. If a struct legitimately declares a `constructor` field and the caller sets it before an unexpected key is encountered, `this.constructor` is already the user-supplied value and `.name` would crash or be undefined. Switch to the closed-over `name` parameter, which is lexically bound and cannot be shadowed by instance assignments. Adds a regression test for the edge case.

…sages Followup to the previous commit. Closed-over `name` was correct on the shadow-resistance axis but lost the subclass-aware error message that `this.constructor.name` originally provided: when a Struct-generated class is subclassed, the error should identify the actual instantiated subclass, not the underlying Struct name. `new.target` is a syntactic binding to the constructor actually invoked via `new`. It reflects the subclass identity AND cannot be shadowed by a `constructor` field the caller may have set earlier in the same loop. Strictly better than both `this.constructor.name` and closed-over `name`. Adds a subclassing regression test so this property is locked in.

RobinBol · 2026-05-20T13:53:00Z

Summary

Two small robustness fixes to the parser primitives. Both have zero behavior impact on correct callers; both close real footguns. Each lands with a regression-locking test.

1. `Bitmap.fromBuffer` now owns its bytes

Bitmap.fromBuffer previously stored buf.slice(i, i + len) as the bitmap's backing store. In Node, `Buffer.slice` returns a view sharing memory with the source. If anything mutated the source after parsing (e.g. a radio driver reusing its receive buffer across frames), the bits read from an already-parsed Bitmap would silently change - a TOCTOU-style hazard that depends on caller buffer-management behavior.

Wrapping the slice in `Buffer.from(...)` makes the Bitmap own its bytes. One-character diff.

2. `Struct` rejects prototype-chain keys

`Struct`'s constructor used `if (!defs[key]) throw` to validate incoming field names. That check walks the prototype chain, so `defs.constructor` resolves to `Object` (truthy), and `constructor` is silently accepted as a valid field name - quietly shadowing the instance's `.constructor` reference. `toString`, `valueOf`, etc. were affected the same way.

Switching to `Object.prototype.hasOwnProperty.call(defs, key)` restricts acceptance to declared own-property fields. `proto` was already rejected (its prototype-chain value is treated as the unexpected key); this aligns the rest of the prototype-chain keys with the same behavior.

Test plan

New test in `test/DataTypes.test.js`: parse a map8 from a source buffer, mutate the source, read the bitmap again - bits must be unchanged.
New tests in `test/Struct.test.js`: `constructor`, `toString` rejected; declared and undeclared field names handled as before.
Full suite: 14/14 passing (9 pre-existing + 5 new). No regressions.
Lint and typecheck clean.
Verified against downstream consumer `node-zigbee-clusters` (81/81 tests pass against the patched build).

Out of scope

The silent zero-fallback behavior in `uintFromBuf` / `enumFromBuf` / `dataFromBuf` / `bufferFromBuf` on truncated input is a separate, larger discussion (it's a breaking change to make those throw). The downstream consumer `node-zigbee-clusters` has separately added a strict length precheck around command-arg parsing (athombv/node-zigbee-clusters#193), which addresses the most consequential consumer impact without requiring changes here.

Two small robustness fixes: 1. `Bitmap.fromBuffer` previously stored the result of `buf.slice(...)`, which in Node returns a Buffer that shares memory with the source. If the source buffer was mutated after parsing (e.g. a radio driver reusing its receive buffer across frames), the bits read from an already-parsed Bitmap would change. Wrap the slice in `Buffer.from(...)` so the Bitmap owns its bytes. 2. `Struct`'s constructor used `!defs[key]` to validate field names. That check walks the prototype chain, so `defs.constructor` resolves to `Object` (truthy) and `constructor` is accepted as a valid field name, silently overwriting the instance's `.constructor` reference. Use `Object.prototype.hasOwnProperty.call(defs, key)` instead so only declared own-property fields are accepted. Other prototype-chain keys (`toString`, `valueOf`, ...) are also now correctly rejected. Tests added for both. Neither change affects behavior for any correct caller.

Copilot

Pull request overview

This PR strengthens two core parser primitives to avoid subtle footguns: (1) preventing Bitmap instances from aliasing caller buffers, and (2) tightening Struct construction to reject unexpected keys that come from the prototype chain.

Changes:

Make Bitmap.fromBuffer copy its backing bytes so later mutations of the source buffer can’t change already-parsed bitmaps.
Validate Struct constructor input keys using hasOwnProperty (instead of truthiness lookup) to reject prototype-chain keys like constructor/toString.
Add regression tests covering buffer aliasing and prototype-chain key rejection.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
lib/DataTypes.js	Copies bitmap bytes in `Bitmap.fromBuffer` to prevent source-buffer aliasing.
lib/Struct.js	Switches unexpected-property validation to `hasOwnProperty` to block prototype-chain keys.
test/DataTypes.test.js	Adds regression test ensuring parsed bitmaps don’t change when the source buffer is mutated.
test/Struct.test.js	Adds tests ensuring prototype-chain keys are rejected while normal declared/undeclared behavior remains.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Address Copilot review feedback on #44: the throw path previously used `this.constructor.name` for the error prefix. If a struct legitimately declares a `constructor` field and the caller sets it before an unexpected key is encountered, `this.constructor` is already the user-supplied value and `.name` would crash or be undefined. Switch to the closed-over `name` parameter, which is lexically bound and cannot be shadowed by instance assignments. Adds a regression test for the edge case.

…sages Followup to the previous commit. Closed-over `name` was correct on the shadow-resistance axis but lost the subclass-aware error message that `this.constructor.name` originally provided: when a Struct-generated class is subclassed, the error should identify the actual instantiated subclass, not the underlying Struct name. `new.target` is a syntactic binding to the constructor actually invoked via `new`. It reflects the subclass identity AND cannot be shadowed by a `constructor` field the caller may have set earlier in the same loop. Strictly better than both `this.constructor.name` and closed-over `name`. Adds a subclassing regression test so this property is locked in.

RobinBol · 2026-05-20T13:53:00Z

Summary

Two small robustness fixes to the parser primitives. Both have zero behavior impact on correct callers; both close real footguns. Each lands with a regression-locking test.

1. `Bitmap.fromBuffer` now owns its bytes

Bitmap.fromBuffer previously stored buf.slice(i, i + len) as the bitmap's backing store. In Node, `Buffer.slice` returns a view sharing memory with the source. If anything mutated the source after parsing (e.g. a radio driver reusing its receive buffer across frames), the bits read from an already-parsed Bitmap would silently change - a TOCTOU-style hazard that depends on caller buffer-management behavior.

Wrapping the slice in `Buffer.from(...)` makes the Bitmap own its bytes. One-character diff.

2. `Struct` rejects prototype-chain keys

`Struct`'s constructor used `if (!defs[key]) throw` to validate incoming field names. That check walks the prototype chain, so `defs.constructor` resolves to `Object` (truthy), and `constructor` is silently accepted as a valid field name - quietly shadowing the instance's `.constructor` reference. `toString`, `valueOf`, etc. were affected the same way.

Switching to `Object.prototype.hasOwnProperty.call(defs, key)` restricts acceptance to declared own-property fields. `proto` was already rejected (its prototype-chain value is treated as the unexpected key); this aligns the rest of the prototype-chain keys with the same behavior.

Test plan

New test in `test/DataTypes.test.js`: parse a map8 from a source buffer, mutate the source, read the bitmap again - bits must be unchanged.
New tests in `test/Struct.test.js`: `constructor`, `toString` rejected; declared and undeclared field names handled as before.
Full suite: 14/14 passing (9 pre-existing + 5 new). No regressions.
Lint and typecheck clean.
Verified against downstream consumer `node-zigbee-clusters` (81/81 tests pass against the patched build).

Out of scope

The silent zero-fallback behavior in `uintFromBuf` / `enumFromBuf` / `dataFromBuf` / `bufferFromBuf` on truncated input is a separate, larger discussion (it's a breaking change to make those throw). The downstream consumer `node-zigbee-clusters` has separately added a strict length precheck around command-arg parsing (athombv/node-zigbee-clusters#193), which addresses the most consequential consumer impact without requiring changes here.

Two small robustness fixes: 1. `Bitmap.fromBuffer` previously stored the result of `buf.slice(...)`, which in Node returns a Buffer that shares memory with the source. If the source buffer was mutated after parsing (e.g. a radio driver reusing its receive buffer across frames), the bits read from an already-parsed Bitmap would change. Wrap the slice in `Buffer.from(...)` so the Bitmap owns its bytes. 2. `Struct`'s constructor used `!defs[key]` to validate field names. That check walks the prototype chain, so `defs.constructor` resolves to `Object` (truthy) and `constructor` is accepted as a valid field name, silently overwriting the instance's `.constructor` reference. Use `Object.prototype.hasOwnProperty.call(defs, key)` instead so only declared own-property fields are accepted. Other prototype-chain keys (`toString`, `valueOf`, ...) are also now correctly rejected. Tests added for both. Neither change affects behavior for any correct caller.

Copilot

Pull request overview

This PR strengthens two core parser primitives to avoid subtle footguns: (1) preventing Bitmap instances from aliasing caller buffers, and (2) tightening Struct construction to reject unexpected keys that come from the prototype chain.

Changes:

Make Bitmap.fromBuffer copy its backing bytes so later mutations of the source buffer can’t change already-parsed bitmaps.
Validate Struct constructor input keys using hasOwnProperty (instead of truthiness lookup) to reject prototype-chain keys like constructor/toString.
Add regression tests covering buffer aliasing and prototype-chain key rejection.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
lib/DataTypes.js	Copies bitmap bytes in `Bitmap.fromBuffer` to prevent source-buffer aliasing.
lib/Struct.js	Switches unexpected-property validation to `hasOwnProperty` to block prototype-chain keys.
test/DataTypes.test.js	Adds regression test ensuring parsed bitmaps don’t change when the source buffer is mutated.
test/Struct.test.js	Adds tests ensuring prototype-chain keys are rejected while normal declared/undeclared behavior remains.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Address Copilot review feedback on #44: the throw path previously used `this.constructor.name` for the error prefix. If a struct legitimately declares a `constructor` field and the caller sets it before an unexpected key is encountered, `this.constructor` is already the user-supplied value and `.name` would crash or be undefined. Switch to the closed-over `name` parameter, which is lexically bound and cannot be shadowed by instance assignments. Adds a regression test for the edge case.

…sages Followup to the previous commit. Closed-over `name` was correct on the shadow-resistance axis but lost the subclass-aware error message that `this.constructor.name` originally provided: when a Struct-generated class is subclassed, the error should identify the actual instantiated subclass, not the underlying Struct name. `new.target` is a syntactic binding to the constructor actually invoked via `new`. It reflects the subclass identity AND cannot be shadowed by a `constructor` field the caller may have set earlier in the same loop. Strictly better than both `this.constructor.name` and closed-over `name`. Adds a subclassing regression test so this property is locked in.

RobinBol · 2026-05-20T13:53:00Z

Summary

Two small robustness fixes to the parser primitives. Both have zero behavior impact on correct callers; both close real footguns. Each lands with a regression-locking test.

1. `Bitmap.fromBuffer` now owns its bytes

Bitmap.fromBuffer previously stored buf.slice(i, i + len) as the bitmap's backing store. In Node, `Buffer.slice` returns a view sharing memory with the source. If anything mutated the source after parsing (e.g. a radio driver reusing its receive buffer across frames), the bits read from an already-parsed Bitmap would silently change - a TOCTOU-style hazard that depends on caller buffer-management behavior.

Wrapping the slice in `Buffer.from(...)` makes the Bitmap own its bytes. One-character diff.

2. `Struct` rejects prototype-chain keys

`Struct`'s constructor used `if (!defs[key]) throw` to validate incoming field names. That check walks the prototype chain, so `defs.constructor` resolves to `Object` (truthy), and `constructor` is silently accepted as a valid field name - quietly shadowing the instance's `.constructor` reference. `toString`, `valueOf`, etc. were affected the same way.

Switching to `Object.prototype.hasOwnProperty.call(defs, key)` restricts acceptance to declared own-property fields. `proto` was already rejected (its prototype-chain value is treated as the unexpected key); this aligns the rest of the prototype-chain keys with the same behavior.

Test plan

New test in `test/DataTypes.test.js`: parse a map8 from a source buffer, mutate the source, read the bitmap again - bits must be unchanged.
New tests in `test/Struct.test.js`: `constructor`, `toString` rejected; declared and undeclared field names handled as before.
Full suite: 14/14 passing (9 pre-existing + 5 new). No regressions.
Lint and typecheck clean.
Verified against downstream consumer `node-zigbee-clusters` (81/81 tests pass against the patched build).

Out of scope

The silent zero-fallback behavior in `uintFromBuf` / `enumFromBuf` / `dataFromBuf` / `bufferFromBuf` on truncated input is a separate, larger discussion (it's a breaking change to make those throw). The downstream consumer `node-zigbee-clusters` has separately added a strict length precheck around command-arg parsing (athombv/node-zigbee-clusters#193), which addresses the most consequential consumer impact without requiring changes here.

Two small robustness fixes: 1. `Bitmap.fromBuffer` previously stored the result of `buf.slice(...)`, which in Node returns a Buffer that shares memory with the source. If the source buffer was mutated after parsing (e.g. a radio driver reusing its receive buffer across frames), the bits read from an already-parsed Bitmap would change. Wrap the slice in `Buffer.from(...)` so the Bitmap owns its bytes. 2. `Struct`'s constructor used `!defs[key]` to validate field names. That check walks the prototype chain, so `defs.constructor` resolves to `Object` (truthy) and `constructor` is accepted as a valid field name, silently overwriting the instance's `.constructor` reference. Use `Object.prototype.hasOwnProperty.call(defs, key)` instead so only declared own-property fields are accepted. Other prototype-chain keys (`toString`, `valueOf`, ...) are also now correctly rejected. Tests added for both. Neither change affects behavior for any correct caller.

Copilot

Pull request overview

This PR strengthens two core parser primitives to avoid subtle footguns: (1) preventing Bitmap instances from aliasing caller buffers, and (2) tightening Struct construction to reject unexpected keys that come from the prototype chain.

Changes:

Make Bitmap.fromBuffer copy its backing bytes so later mutations of the source buffer can’t change already-parsed bitmaps.
Validate Struct constructor input keys using hasOwnProperty (instead of truthiness lookup) to reject prototype-chain keys like constructor/toString.
Add regression tests covering buffer aliasing and prototype-chain key rejection.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
lib/DataTypes.js	Copies bitmap bytes in `Bitmap.fromBuffer` to prevent source-buffer aliasing.
lib/Struct.js	Switches unexpected-property validation to `hasOwnProperty` to block prototype-chain keys.
test/DataTypes.test.js	Adds regression test ensuring parsed bitmaps don’t change when the source buffer is mutated.
test/Struct.test.js	Adds tests ensuring prototype-chain keys are rejected while normal declared/undeclared behavior remains.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Address Copilot review feedback on #44: the throw path previously used `this.constructor.name` for the error prefix. If a struct legitimately declares a `constructor` field and the caller sets it before an unexpected key is encountered, `this.constructor` is already the user-supplied value and `.name` would crash or be undefined. Switch to the closed-over `name` parameter, which is lexically bound and cannot be shadowed by instance assignments. Adds a regression test for the edge case.

…sages Followup to the previous commit. Closed-over `name` was correct on the shadow-resistance axis but lost the subclass-aware error message that `this.constructor.name` originally provided: when a Struct-generated class is subclassed, the error should identify the actual instantiated subclass, not the underlying Struct name. `new.target` is a syntactic binding to the constructor actually invoked via `new`. It reflects the subclass identity AND cannot be shadowed by a `constructor` field the caller may have set earlier in the same loop. Strictly better than both `this.constructor.name` and closed-over `name`. Adds a subclassing regression test so this property is locked in.

RobinBol requested a review from Copilot May 20, 2026 13:54

Copilotstarted reviewing on behalf of RobinBol May 20, 2026 13:54 View session

RobinBol self-assigned this May 20, 2026

CopilotAI reviewed May 20, 2026
View reviewed changes

Comment threadlib/Struct.js Outdated

RobinBol added 2 commits May 20, 2026 15:59

RobinBol merged commit 7e6177e into masterMay 20, 2026
4 checks passed

RobinBol deleted the fix/parser-robustness branch May 20, 2026 14:08

RobinBol mentioned this pull request May 21, 2026
fix(cluster): reject truncated command payloads with MALFORMED_COMMAND athombv/node-zigbee-clusters#193
Open
5 tasks

RobinBol requested a review from Copilot May 20, 2026 13:54

Copilotstarted reviewing on behalf of RobinBol May 20, 2026 13:54 View session

RobinBol self-assigned this May 20, 2026

CopilotAI reviewed May 20, 2026
View reviewed changes

Comment threadlib/Struct.js Outdated

RobinBol added 2 commits May 20, 2026 15:59

RobinBol merged commit 7e6177e into masterMay 20, 2026
4 checks passed

RobinBol deleted the fix/parser-robustness branch May 20, 2026 14:08

RobinBol mentioned this pull request May 21, 2026
fix(cluster): reject truncated command payloads with MALFORMED_COMMAND athombv/node-zigbee-clusters#193
Open
5 tasks

RobinBol requested a review from Copilot May 20, 2026 13:54

Copilotstarted reviewing on behalf of RobinBol May 20, 2026 13:54 View session

RobinBol self-assigned this May 20, 2026

CopilotAI reviewed May 20, 2026
View reviewed changes

Comment threadlib/Struct.js Outdated

RobinBol added 2 commits May 20, 2026 15:59

RobinBol merged commit 7e6177e into masterMay 20, 2026
4 checks passed

RobinBol deleted the fix/parser-robustness branch May 20, 2026 14:08

RobinBol mentioned this pull request May 21, 2026
fix(cluster): reject truncated command payloads with MALFORMED_COMMAND athombv/node-zigbee-clusters#193
Open
5 tasks

RobinBol requested a review from Copilot May 20, 2026 13:54

Copilotstarted reviewing on behalf of RobinBol May 20, 2026 13:54 View session

RobinBol self-assigned this May 20, 2026

CopilotAI reviewed May 20, 2026
View reviewed changes

Comment threadlib/Struct.js Outdated

RobinBol added 2 commits May 20, 2026 15:59

RobinBol merged commit 7e6177e into masterMay 20, 2026
4 checks passed

RobinBol deleted the fix/parser-robustness branch May 20, 2026 14:08

RobinBol mentioned this pull request May 21, 2026
fix(cluster): reject truncated command payloads with MALFORMED_COMMAND athombv/node-zigbee-clusters#193
Open
5 tasks

RobinBol requested a review from Copilot May 20, 2026 13:54

Copilotstarted reviewing on behalf of RobinBol May 20, 2026 13:54 View session

RobinBol self-assigned this May 20, 2026

CopilotAI reviewed May 20, 2026
View reviewed changes

Comment threadlib/Struct.js Outdated

RobinBol added 2 commits May 20, 2026 15:59

RobinBol merged commit 7e6177e into masterMay 20, 2026
4 checks passed

RobinBol deleted the fix/parser-robustness branch May 20, 2026 14:08

RobinBol mentioned this pull request May 21, 2026
fix(cluster): reject truncated command payloads with MALFORMED_COMMAND athombv/node-zigbee-clusters#193
Open
5 tasks

RobinBol requested a review from Copilot May 20, 2026 13:54

Copilotstarted reviewing on behalf of RobinBol May 20, 2026 13:54 View session

RobinBol self-assigned this May 20, 2026

CopilotAI reviewed May 20, 2026
View reviewed changes

Comment threadlib/Struct.js Outdated

RobinBol added 2 commits May 20, 2026 15:59

RobinBol merged commit 7e6177e into masterMay 20, 2026
4 checks passed

RobinBol deleted the fix/parser-robustness branch May 20, 2026 14:08

RobinBol mentioned this pull request May 21, 2026
fix(cluster): reject truncated command payloads with MALFORMED_COMMAND athombv/node-zigbee-clusters#193
Open
5 tasks

RobinBol requested a review from Copilot May 20, 2026 13:54

Copilotstarted reviewing on behalf of RobinBol May 20, 2026 13:54 View session

RobinBol self-assigned this May 20, 2026

CopilotAI reviewed May 20, 2026
View reviewed changes

Comment threadlib/Struct.js Outdated

RobinBol added 2 commits May 20, 2026 15:59

RobinBol merged commit 7e6177e into masterMay 20, 2026
4 checks passed

RobinBol deleted the fix/parser-robustness branch May 20, 2026 14:08

RobinBol mentioned this pull request May 21, 2026
fix(cluster): reject truncated command payloads with MALFORMED_COMMAND athombv/node-zigbee-clusters#193
Open
5 tasks

RobinBol requested a review from Copilot May 20, 2026 13:54

Copilotstarted reviewing on behalf of RobinBol May 20, 2026 13:54 View session

RobinBol self-assigned this May 20, 2026

CopilotAI reviewed May 20, 2026
View reviewed changes

Comment threadlib/Struct.js Outdated

RobinBol added 2 commits May 20, 2026 15:59

RobinBol merged commit 7e6177e into masterMay 20, 2026
4 checks passed

RobinBol deleted the fix/parser-robustness branch May 20, 2026 14:08

RobinBol mentioned this pull request May 21, 2026
fix(cluster): reject truncated command payloads with MALFORMED_COMMAND athombv/node-zigbee-clusters#193
Open
5 tasks

Uh oh!

Conversation

RobinBol commented May 20, 2026

Summary

1. Bitmap.fromBuffer now owns its bytes

2. `Struct` rejects prototype-chain keys

Test plan

Out of scope

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

RobinBol commented May 20, 2026

Summary

1. Bitmap.fromBuffer now owns its bytes

2. `Struct` rejects prototype-chain keys

Test plan

Out of scope

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

RobinBol commented May 20, 2026

Summary

1. Bitmap.fromBuffer now owns its bytes

2. `Struct` rejects prototype-chain keys

Test plan

Out of scope

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

RobinBol commented May 20, 2026

Summary

1. Bitmap.fromBuffer now owns its bytes

2. `Struct` rejects prototype-chain keys

Test plan

Out of scope

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

1. `Bitmap.fromBuffer` now owns its bytes

1. `Bitmap.fromBuffer` now owns its bytes

1. `Bitmap.fromBuffer` now owns its bytes

1. `Bitmap.fromBuffer` now owns its bytes

1. `Bitmap.fromBuffer` now owns its bytes

1. `Bitmap.fromBuffer` now owns its bytes