buffer: isUtf8 and isAscii accept detached TypedArray views

[Archkon]

Version

latest main branch

Platform

7.1.2-arch3-1

Subsystem

buffer

What steps will reproduce the bug?

const { isUtf8, isAscii } = require('node:buffer');

const view = new Uint8Array([0xff]);
structuredClone(view.buffer, { transfer: [view.buffer] });

console.log(isUtf8(view));
console.log(isAscii(view));

How often does it reproduce? Is there a required condition?

ArrayBufferViewContents reads the view's zero byte length but does not record the detached state of its backing ArrayBuffer.

What is the expected behavior? Why is that the expected behavior?

Both functions should throw ERR_INVALID_STATE, consistent with passing a detached ArrayBuffer directly.

What do you see instead?

Both functions return true.

Additional information

[jasnell]

It's not obvious this is a bug. In both cases, the detached ArrayBuffer just comes back as zero-length. These APIs handle zero-length just fine (e.g. isUtf8(new Uint8Array(0))).

I do see that the docs say "Throws if the input is a detached array buffer" but I kind of view that as a doc error rather than an implementation error.

[Archkon]

It's not obvious this is a bug. In both cases, the detached ArrayBuffer just comes back as zero-length. These APIs handle zero-length just fine (e.g. isUtf8(new Uint8Array(0))).

I do see that the docs say "Throws if the input is a detached array buffer" but I kind of view that as a doc error rather than an implementation error.

I know this design ,but I don't mean this

[Renegade334]

We also only check the detached_ property in these two methods AFAICT, all other uses of ABVC in the Buffer API just treat detached buffers as zero-length, including other string-encoding operations like Buffer.prototype.toString(). I'm ±0 on whether we should be erroring with detached backing ArrayBuffers, but it should be consistent imo

[Archkon]

It's not obvious this is a bug. In both cases, the detached ArrayBuffer just comes back as zero-length. These APIs handle zero-length just fine (e.g. isUtf8(new Uint8Array(0))).

I do see that the docs say "Throws if the input is a detached array buffer" but I kind of view that as a doc error rather than an implementation error.

isUtf8(new Uint8Array(0)) should return true but the actual behavior is that

 const arrayBuffer = new ArrayBuffer(4);
  const typedArray = new Uint8Array(arrayBuffer);
  const buffer = Buffer.from(arrayBuffer);

then

  structuredClone(arrayBuffer, {
    transfer: [arrayBuffer],
  });

and then

  console.log({
    arrayBufferLength: arrayBuffer.byteLength,
    typedArrayLength: typedArray.byteLength,
    bufferLength: buffer.byteLength,
  });

but

// isUtf8 or isAscii
  check('detached ArrayBuffer', arrayBuffer);
  check('detached Uint8Array', typedArray);
  check('detached Buffer', buffer);

would cause

  detached ArrayBuffer: threw ERR_INVALID_STATE
  detached Uint8Array: true
  detached Buffer: true

[Archkon]

We also only check the detached_ property in these two methods AFAICT, all other uses of ABVC in the Buffer API just treat detached buffers as zero-length, including other string-encoding operations like Buffer.prototype.toString(). I'm ±0 on whether we should be erroring with detached backing ArrayBuffers, but it should be consistent imo

I just think this is a kind of implementation problem
This should refer to this: #45947 (comment)

[jasnell]

Ok, the implementation behavior is a bit inconsistent. I would suggest that the final isUtf8(transferedArrayBuffer) should just return true, being treated as a zero-length.

[Archkon]

Ok, the implementation behavior is a bit inconsistent. I would suggest that the final isUtf8(transferedArrayBuffer) should just return true, being treated as a zero-length.

Should we add change entry for doc ?Or just directly change the doc description?