Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normative: move 'into' methods onto prototype and rename #45

Merged
merged 1 commit into from
Feb 7, 2024
Merged

Normative: move 'into' methods onto prototype and rename #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ The hex methods do not take any options.

## Writing to an existing Uint8Array

The `Uint8Array.fromBase64Into` method allows writing to an existing Uint8Array. Like the [TextEncoder `encodeInto` method](https://developer.mozilla.org/en-US/docs/Web/API/TextEncoder/encodeInto), it returns a `{ read, written }` pair.
The `Uint8Array.prototype.setFromBase64` method allows writing to an existing Uint8Array. Like the [TextEncoder `encodeInto` method](https://developer.mozilla.org/en-US/docs/Web/API/TextEncoder/encodeInto), it returns a `{ read, written }` pair.

```js
let target = new Uint8Array(8);
let { read, written } = Uint8Array.fromBase64Into('Zm9vYmFy', target);
let { read, written } = target.setFromBase64('Zm9vYmFy');
assert.deepStrictEqual([...target], [102, 111, 111, 98, 97, 114, 0, 0]);
assert.deepStrictEqual({ read, written }, { read: 8, written: 6 });
```
Expand All @@ -55,7 +55,7 @@ This method takes an optional final options bag with the same options as above.

As with `encodeInto`, there is not explicit support for writing to specified offset of the target, but you can accomplish that by creating a subarray.

`Uint8Array.fromHexInto` is the same except for hex.
`Uint8Array.prototype.setFromHex` is the same except for hex.

## Streaming

Expand Down
8 changes: 4 additions & 4 deletions playground/index-raw.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,22 +112,22 @@ <h3>Options</h3>
</code></pre>

<h3>Writing to an existing Uint8Array</h3>
<p>The <code>Uint8Array.fromBase64Into</code> method allows writing to an existing Uint8Array. Like the <a href="https://developer.mozilla.org/en-US/docs/Web/API/TextEncoder/encodeInto">TextEncoder <code>encodeInto</code> method</a>, it returns a <code>{ read, written }</code> pair.</p>
<p>The <code>Uint8Array.prototype.setFromBase64</code> method allows writing to an existing Uint8Array. Like the <a href="https://developer.mozilla.org/en-US/docs/Web/API/TextEncoder/encodeInto">TextEncoder <code>encodeInto</code> method</a>, it returns a <code>{ read, written }</code> pair.</p>

<p>This method takes an optional final options bag with the same options as above.</p>

<pre class="language-js"><code class="language-js">
let target = new Uint8Array(7);
let { read, written } = Uint8Array.fromBase64Into('Zm9vYmFy', target);
let { read, written } = target.setFromBase64('Zm9vYmFy');
console.log({ target, read, written });
// { target: Uint8Array([102, 111, 111, 98, 97, 114, 0]), read: 8, written: 6 }
</code></pre>

<p><code>Uint8Array.fromHexInto</code> is the same except for hex.</p>
<p><code>Uint8Array.prototype.setFromHex</code> is the same except for hex.</p>

<pre class="language-js"><code class="language-js">
let target = new Uint8Array(6);
let { read, written } = Uint8Array.fromHexInto('deadbeef', target);
let { read, written } = target.setFromHex('deadbeef');
console.log({ target, read, written });
// { target: Uint8Array([222, 173, 190, 239, 0, 0]), read: 8, written: 4 }
</code></pre>
Expand Down
41 changes: 22 additions & 19 deletions playground/polyfill-install.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,19 @@ Object.defineProperty(Uint8Array, 'fromBase64', { enumerable: false });
Object.defineProperty(Uint8Array.fromBase64, 'length', { value: 1 });
Object.defineProperty(Uint8Array.fromBase64, 'name', { value: 'fromBase64' });

Uint8Array.fromBase64Into = (string, into, options) => {
if (typeof string !== 'string') {
throw new TypeError('expected input to be a string');
// method shenanigans to make a non-constructor which can refer to "this"
Uint8Array.prototype.setFromBase64 = {
setFromBase64(string, options) {
checkUint8Array(this);
if (typeof string !== 'string') {
throw new TypeError('expected input to be a string');
}
let { read, bytes } = base64ToUint8Array(string, options, this);
return { read, written: bytes.length };
}
checkUint8Array(into);
let { read, bytes } = base64ToUint8Array(string, options, into);
return { read, written: bytes.length };
};
Object.defineProperty(Uint8Array, 'fromBase64Into', { enumerable: false });
Object.defineProperty(Uint8Array.fromBase64Into, 'length', { value: 2 });
Object.defineProperty(Uint8Array.fromBase64Into, 'name', { value: 'fromBase64Into' });
}.setFromBase64;
Object.defineProperty(Uint8Array.prototype, 'setFromBase64', { enumerable: false });
Object.defineProperty(Uint8Array.prototype.setFromBase64, 'length', { value: 1 });

Uint8Array.prototype.toHex = {
toHex() {
Expand All @@ -47,13 +49,14 @@ Uint8Array.fromHex = (string) => {
Object.defineProperty(Uint8Array, 'fromHex', { enumerable: false });
Object.defineProperty(Uint8Array.fromHex, 'name', { value: 'fromHex' });

Uint8Array.fromHexInto = (string, into) => {
if (typeof string !== 'string') {
throw new TypeError('expected input to be a string');
Uint8Array.prototype.setFromHex = {
setFromHex(string) {
checkUint8Array(this);
if (typeof string !== 'string') {
throw new TypeError('expected input to be a string');
}
let { read, bytes } = hexToUint8Array(string, this);
return { read, written: bytes.length };
}
checkUint8Array(into);
let { read, bytes } = hexToUint8Array(string, into);
return { read, written: bytes.length };
};
Object.defineProperty(Uint8Array, 'fromHexInto', { enumerable: false });
Object.defineProperty(Uint8Array.fromHexInto, 'name', { value: 'fromHexInto' });
}.setFromHex;
Object.defineProperty(Uint8Array.prototype, 'setFromHex', { enumerable: false });
14 changes: 8 additions & 6 deletions spec.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,12 @@ <h1>Uint8Array.fromBase64 ( _string_ [ , _options_ ] )</h1>
</emu-alg>
</emu-clause>

<emu-clause id="sec-uint8array.frombase64into">
<h1>Uint8Array.fromBase64Into ( _string_, _into_ [ , _options_ ] )</h1>
<emu-clause id="sec-uint8array.prototype.setfrombase64">
<h1>Uint8Array.prototype.setFromBase64 ( _string_ [ , _options_ ] )</h1>
<emu-alg>
1. If _string_ is not a String, throw a *TypeError* exception.
1. Let _into_ be the *this* value.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1. Let _into_ be the *this* value.
1. Let _into_ be the *this* value.
1. Perform ? ValidateTypedArray(_into_, ~seq-cst~).

Here and below to match other TypedArray.prototype built-ins.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The closest such built-in is TypedArray.prototype.set, and it, like this method, initially checks only the presence of the internal slot on this, and then does the potentially side-effecting handling of arguments, and only after that does the IsTypedArrayOutOfBounds check.

That's what I'm doing here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypedArray.prototype.set (and TypedArray.prototype.subarray) are both legacy Khronos-based functions, so I don't think it's useful/necessary to copy the same approach in new methods. Instead it's preferable to use the same validation sequence which is present in all newer TypedArray built-ins.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Actually it'd great if we could change TypedArray.prototype.{set,subarray} to call ValidateTypedArray at the start, because it's kind of annoying that subarray can observe the original [[ByteOffset]] when the underlying buffer is detached, see https://bugzilla.mozilla.org/show_bug.cgi?id=1291003 and https://bugzilla.mozilla.org/show_bug.cgi?id=1840991. Being able to read [[ByteOffset]] from detached/out-of-bounds typed arrays also requires extra JIT support in SpiderMonkey, where subarray is a self-hosted built-in: https://phabricator.services.mozilla.com/D200658.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want to get the length (and validate non-detachedness) after side-effects happen anyway. Checking up-front would mean duplicating that work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @syg for thoughts.

Copy link

@syg syg Feb 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want to get the length (and validate non-detachedness) after side-effects happen anyway. Checking up-front would mean duplicating that work.

I agree with this.

I'm leaning towards breaking with consistency. If we do that, the most desirable processing order for TypedArray builtins are to first process all the parameters, then do a ValidateTypedArray on this. After that point, with the exception of built-ins that call user-provided callbacks in a loop (like map or something), there should be no more user code being called.

If for whatever reason committee is not amenable to breaking with consistency, the ValidateTypedArray on this, then arguments, then ValidateTypedArray again order is wasteful. And given that maximal consistency is not possible since the builtins are already inconsistent, I see no compelling reason to be wasteful.

1. Perform ? ValidateUint8Array(_into_).
1. If _string_ is not a String, throw a *TypeError* exception.
1. Let _opts_ be ? GetOptionsObject(_options_).
1. Let _alphabet_ be ? Get(_opts_, *"alphabet"*).
1. If _alphabet_ is *undefined*, set _alphabet_ to *"base64"*.
Expand Down Expand Up @@ -116,11 +117,12 @@ <h1>Uint8Array.fromHex ( _string_ )</h1>
</emu-alg>
</emu-clause>

<emu-clause id="sec-uint8array.fromhexinto">
<h1>Uint8Array.fromHexInto ( _string_, _into_ )</h1>
<emu-clause id="sec-uint8array.prototype.setfromhex">
<h1>Uint8Array.prototype.setFromHex ( _string_ )</h1>
<emu-alg>
1. If _string_ is not a String, throw a *TypeError* exception.
1. Let _into_ be the *this* value.
1. Perform ? ValidateUint8Array(_into_).
1. If _string_ is not a String, throw a *TypeError* exception.
1. Let _taRecord_ be MakeTypedArrayWithBufferWitnessRecord(_into_, ~seq-cst~).
1. If IsTypedArrayOutOfBounds(_taRecord_) is *true*, throw a *TypeError* exception.
1. Let _byteLength_ be TypedArrayByteLength(_taRecord_).
Expand Down
2 changes: 1 addition & 1 deletion stream.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class Base64Decoder {
// but may be too much if there is whitespace
// if you're really concerned about memory, a TextDecoder style API is a bad choice
let buffer = new Uint8Array(Math.ceil(chunk.length * 3 / 4));
let { read, written } = Uint8Array.fromBase64Into(chunk, buffer, opts);
let { read, written } = buffer.setFromBase64(chunk, opts);
buffer = buffer.subarray(0, written);
this.#extra = chunk.slice(read);
return buffer;
Expand Down
28 changes: 14 additions & 14 deletions test-polyfill.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,70 +96,70 @@ test('writing to an existing buffer', async t => {

await t.test('buffer exact', () => {
let output = new Uint8Array(6);
let { read, written } = Uint8Array.fromBase64Into(foobarInput, output);
let { read, written } = output.setFromBase64(foobarInput);
assert.deepStrictEqual([...output], foobarOutput);
assert.deepStrictEqual({ read, written }, { read: 8, written: 6 });
});

await t.test('buffer too large', () => {
let output = new Uint8Array(8);
let { read, written } = Uint8Array.fromBase64Into(foobarInput, output);
let { read, written } = output.setFromBase64(foobarInput);
assert.deepStrictEqual([...output], [...foobarOutput, 0, 0]);
assert.deepStrictEqual({ read, written }, { read: 8, written: 6 });
});

await t.test('buffer too small', () => {
let output = new Uint8Array(5);
let { read, written } = Uint8Array.fromBase64Into(foobarInput, output);
let { read, written } = output.setFromBase64(foobarInput);
assert.deepStrictEqual([...output], [...foobarOutput.slice(0, 3), 0, 0]);
assert.deepStrictEqual({ read, written }, { read: 4, written: 3 });
});

await t.test('buffer exact, padded', () => {
let output = new Uint8Array(5);
let { read, written } = Uint8Array.fromBase64Into(foobaInput + '=', output);
let { read, written } = output.setFromBase64(foobaInput + '=');
assert.deepStrictEqual([...output], foobarOutput.slice(0, 5));
assert.deepStrictEqual({ read, written }, { read: 8, written: 5 });
});

await t.test('buffer exact, not padded', () => {
let output = new Uint8Array(5);
let { read, written } = Uint8Array.fromBase64Into(foobaInput, output);
let { read, written } = output.setFromBase64(foobaInput);
assert.deepStrictEqual([...output], foobarOutput.slice(0, 5));
assert.deepStrictEqual({ read, written }, { read: 7, written: 5 });
});

await t.test('buffer exact, padded, stop-before-partial', () => {
let output = new Uint8Array(5);
let { read, written } = Uint8Array.fromBase64Into(foobaInput + '=', output, { lastChunkHandling: 'stop-before-partial' });
let { read, written } = output.setFromBase64(foobaInput + '=', { lastChunkHandling: 'stop-before-partial' });
assert.deepStrictEqual([...output], foobarOutput.slice(0, 5));
assert.deepStrictEqual({ read, written }, { read: 8, written: 5 });
});

await t.test('buffer exact, not padded, stop-before-partial', () => {
let output = new Uint8Array(5);
let { read, written } = Uint8Array.fromBase64Into(foobaInput, output, { lastChunkHandling: 'stop-before-partial' });
let { read, written } = output.setFromBase64(foobaInput, { lastChunkHandling: 'stop-before-partial' });
assert.deepStrictEqual([...output], [...foobarOutput.slice(0, 3), 0, 0]);
assert.deepStrictEqual({ read, written }, { read: 4, written: 3 });
});

await t.test('buffer too small, padded', () => {
let output = new Uint8Array(4);
let { read, written } = Uint8Array.fromBase64Into(foobaInput + '=', output);
let { read, written } = output.setFromBase64(foobaInput + '=');
assert.deepStrictEqual([...output], [...foobarOutput.slice(0, 3), 0]);
assert.deepStrictEqual({ read, written }, { read: 4, written: 3 });
});

await t.test('buffer too large, trailing whitespace', () => {
let output = new Uint8Array(8);
let { read, written } = Uint8Array.fromBase64Into(foobarInput + ' '.repeat(10), output);
let { read, written } = output.setFromBase64(foobarInput + ' '.repeat(10));
assert.deepStrictEqual([...output], [...foobarOutput, 0, 0]);
assert.deepStrictEqual({ read, written }, { read: 18, written: 6 });
});

await t.test('buffer too large, not padded, trailing whitespace', () => {
let output = new Uint8Array(8);
let { read, written } = Uint8Array.fromBase64Into(foobaInput + ' '.repeat(10), output);
let { read, written } = output.setFromBase64(foobaInput + ' '.repeat(10));
assert.deepStrictEqual([...output], [...foobarOutput.slice(0, 5), 0, 0, 0]);
assert.deepStrictEqual({ read, written }, { read: 17, written: 5 });
});
Expand All @@ -181,7 +181,7 @@ test('stop-before-partial', async t => {

await t.test('no padding, trailing whitespace', () => {
let output = new Uint8Array(8);
let { read, written } = Uint8Array.fromBase64Into(foobaInput + ' '.repeat(10), output, { lastChunkHandling: 'stop-before-partial' });
let { read, written } = output.setFromBase64(foobaInput + ' '.repeat(10), { lastChunkHandling: 'stop-before-partial' });
assert.deepStrictEqual([...output], [...foobarOutput.slice(0, 3), 0, 0, 0, 0, 0]);
assert.deepStrictEqual({ read, written }, { read: 4, written: 3 });
});
Expand All @@ -197,21 +197,21 @@ test('hex', async t => {

await t.test('decode into, exact', () => {
let output = new Uint8Array(4);
let { read, written } = Uint8Array.fromHexInto(encoded, output);
let { read, written } = output.setFromHex(encoded);
assert.deepStrictEqual([...output], decoded);
assert.deepStrictEqual({ read, written }, { read: 8, written: 4 });
});

await t.test('decode into, buffer too large', () => {
let output = new Uint8Array(6);
let { read, written } = Uint8Array.fromHexInto(encoded, output);
let { read, written } = output.setFromHex(encoded);
assert.deepStrictEqual([...output], [...decoded, 0, 0]);
assert.deepStrictEqual({ read, written }, { read: 8, written: 4 });
});

await t.test('decode into, buffer too small', () => {
let output = new Uint8Array(3);
let { read, written } = Uint8Array.fromHexInto(encoded, output);
let { read, written } = output.setFromHex(encoded);
assert.deepStrictEqual([...output], decoded.slice(0, 3));
assert.deepStrictEqual({ read, written }, { read: 6, written: 3 });
});
Expand Down