dualfroz
655cc5eca2
copyBuffer routed DataView through the TypedArray path, passing cur.length (undefined on a DataView) as the constructor's third argument. Since that argument is a byte length for DataView, a partial view was extended to the end of the buffer. Clone DataView explicitly using byteLength.
78 lines
2.5 KiB
Markdown
78 lines
2.5 KiB
Markdown
# Fix DataView clones getting the wrong byteLength
|
|
|
|
## Problem
|
|
|
|
Cloning a `DataView` that covers only part of its underlying `ArrayBuffer`
|
|
produces a clone with the wrong `byteLength`: the copy spans from the original
|
|
`byteOffset` to the end of the buffer instead of keeping the original length.
|
|
|
|
```js
|
|
const clone = require('rfdc')()
|
|
|
|
const buffer = new ArrayBuffer(16)
|
|
const view = new DataView(buffer, 4, 8) // offset 4, length 8
|
|
|
|
const copy = clone({ a: view }).a
|
|
copy.byteLength // 12 (expected 8)
|
|
```
|
|
|
|
The value at the start of the view is copied correctly and the clone is
|
|
independent of the source, but the clone can now read 4 bytes past the end of
|
|
the original view.
|
|
|
|
## Root cause
|
|
|
|
`index.js`, in `copyBuffer`:
|
|
|
|
```js
|
|
return new cur.constructor(cur.buffer.slice(), cur.byteOffset, cur.length)
|
|
```
|
|
|
|
This path handles every `ArrayBuffer.isView` value, i.e. both TypedArrays and
|
|
`DataView`. It is correct for TypedArrays, whose third constructor argument is an
|
|
element count matching `.length`. A `DataView` is different:
|
|
|
|
- it has no `.length` property, so `cur.length` is `undefined`;
|
|
- its third constructor argument is a byte length, not an element count.
|
|
|
|
`new DataView(buffer, byteOffset, undefined)` extends the view to the end of the
|
|
buffer, so a partial view's length is lost. (A DataView over the whole buffer
|
|
happens to work, because "to the end of the buffer" equals its byteLength.)
|
|
|
|
## Fix
|
|
|
|
Handle `DataView` explicitly and size it with `byteLength`:
|
|
|
|
```js
|
|
if (cur instanceof DataView) {
|
|
return new DataView(cur.buffer.slice(), cur.byteOffset, cur.byteLength)
|
|
}
|
|
```
|
|
|
|
TypedArrays keep using the existing `.length`-based path.
|
|
|
|
## Test
|
|
|
|
Added tests (run for all four `rfdc` variants via the shared `types()` helper)
|
|
that clone a full-buffer DataView and a partial DataView, asserting `byteOffset`,
|
|
`byteLength`, the stored value, and independence from the source.
|
|
|
|
Counterfactual (revert the `copyBuffer` DataView branch): the partial-DataView
|
|
test fails with
|
|
|
|
```
|
|
not ok - byteLength preserved
|
|
is(cloned.byteLength, view.byteLength, 'byteLength preserved')
|
|
```
|
|
|
|
(expected 8, got 12), while the full-buffer case still passes.
|
|
|
|
## Gate
|
|
|
|
- Tests: `node test/index.js` -> `1..144`, 144 ok, 0 failing, exit 0.
|
|
The repository's `npm test` invokes the `tap` CLI, whose bundled `esm` loader
|
|
crashes on load under Node 22 (`Function.prototype.apply was called on
|
|
undefined`) independently of this change; running the test file directly
|
|
executes the same assertions.
|
|
- Lint: `standard index.js test/index.js` -> exit 0.
|