Skip to content

Commit

Permalink
Remove Error and make BufferSource checks realm-independent
Browse files Browse the repository at this point in the history
The Error type has been removed from the Web IDL specification, so we can remove it here.

For the BufferSource types, we improve their checks to be realm-independent. Given jsdom's move to separate the Node.js globals from the JSDOM globals, instanceof being incorrect is now causing actual problems.

Helps with jsdom/jsdom#2743. Makes #12 slightly better, but there are still forgeable checks for the typed arrays, based on V.constructor.name.
  • Loading branch information
domenic committed Dec 7, 2019
1 parent cf3d1c4 commit aa79e8c
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 98 deletions.
3 changes: 1 addition & 2 deletions README.md
Expand Up @@ -40,7 +40,6 @@ Conversions for all of the basic types from the Web IDL specification are implem
- [`DOMString`](https://heycam.github.io/webidl/#es-DOMString), which can additionally be provided the boolean option `{ treatNullAsEmptyString }` as a second parameter
- [`ByteString`](https://heycam.github.io/webidl/#es-ByteString), [`USVString`](https://heycam.github.io/webidl/#es-USVString)
- [`object`](https://heycam.github.io/webidl/#es-object)
- [`Error`](https://heycam.github.io/webidl/#es-Error)
- [Buffer source types](https://heycam.github.io/webidl/#es-buffer-source-types)

Additionally, for convenience, the following derived type definitions are implemented:
Expand Down Expand Up @@ -77,4 +76,4 @@ And getting to that payoff is the goal of _this_ project—but for JavaScript im

Seriously, why would you ever use this? You really shouldn't. Web IDL is … strange, and you shouldn't be emulating its semantics. If you're looking for a generic argument-processing library, you should find one with better rules than those from Web IDL. In general, your JavaScript should not be trying to become more like Web IDL; if anything, we should fix Web IDL to make it more like JavaScript.

The _only_ people who should use this are those trying to create faithful implementations (or polyfills) of web platform interfaces defined in Web IDL. Its main consumer is the [jsdom](https://github.com/tmpvar/jsdom) project.
The _only_ people who should use this are those trying to create faithful implementations (or polyfills) of web platform interfaces defined in Web IDL. Its main consumer is the [jsdom](https://github.com/jsdom/jsdom) project.
39 changes: 34 additions & 5 deletions lib/index.js
Expand Up @@ -290,16 +290,45 @@ function convertCallbackFunction(V, opts) {
return V;
}

const abByteLengthGetter =
Object.getOwnPropertyDescriptor(ArrayBuffer.prototype, "byteLength").get;

function isArrayBuffer(V) {
try {
abByteLengthGetter.call(V);
return true;
} catch (e) {
return false;
}
}

// I don't think we can reliably detect detached ArrayBuffers.
exports.ArrayBuffer = (V, opts) => {
if (!isArrayBuffer(V)) {
throw new TypeError(_("is not a view on an ArrayBuffer object", opts));
}
return V;
};

const dvByteLengthGetter =
Object.getOwnPropertyDescriptor(DataView.prototype, "byteLength").get;
exports.DataView = (V, opts) => {
try {
dvByteLengthGetter.call(V);
return V;
} catch (e) {
throw new TypeError(_("is not a view on an DataView object", opts));
}
};

[
Error,
ArrayBuffer, // The IsDetachedBuffer abstract operation is not exposed in JS
DataView, Int8Array, Int16Array, Int32Array, Uint8Array,
Int8Array, Int16Array, Int32Array, Uint8Array,
Uint16Array, Uint32Array, Uint8ClampedArray, Float32Array, Float64Array
].forEach(func => {
const name = func.name;
const article = /^[AEIOU]/.test(name) ? "an" : "a";
exports[name] = (V, opts) => {
if (!(V instanceof func)) {
if (!ArrayBuffer.isView(V) || V.constructor.name !== name) {
throw new TypeError(_(`is not ${article} ${name} object`, opts));
}

Expand All @@ -318,7 +347,7 @@ exports.ArrayBufferView = (V, opts) => {
};

exports.BufferSource = (V, opts) => {
if (!(ArrayBuffer.isView(V) || V instanceof ArrayBuffer)) {
if (!ArrayBuffer.isView(V) && !isArrayBuffer(V)) {
throw new TypeError(_("is not an ArrayBuffer object or a view on one", opts));
}

Expand Down
65 changes: 36 additions & 29 deletions test/buffer-source.js
@@ -1,5 +1,6 @@
"use strict";
const assert = require("assert");
const vm = require("vm");

const conversions = require("..");

Expand Down Expand Up @@ -62,11 +63,10 @@ function testNotOk(name, sut, create) {
});
}

const bufferSourceCreators = [
{ constructor: DataView, creator: () => new DataView(new ArrayBuffer(0)) }
];
const differentRealm = vm.createContext();

[
const bufferSourceConstructors = [
DataView,
ArrayBuffer,
Int8Array,
Int16Array,
Expand All @@ -77,48 +77,55 @@ const bufferSourceCreators = [
Uint8ClampedArray,
Float32Array,
Float64Array
].forEach(constructor => {
bufferSourceCreators.push({ constructor, creator: () => new constructor(0) });
});
];

bufferSourceCreators.forEach(type => {
const name = type.constructor.name;
const sut = conversions[name];
const bufferSourceCreators = [];
for (const constructor of bufferSourceConstructors) {
bufferSourceCreators.push(
{
typeName: constructor.name,
label: `${constructor.name} same realm`,
creator: () => new constructor(new ArrayBuffer(0))
},
{
typeName: constructor.name,
label: `${constructor.name} different realm`,
creator: () => vm.runInContext(`new ${constructor.name}(new ArrayBuffer(0))`, differentRealm)
}
);
}

describe("WebIDL " + name + " type", () => {
bufferSourceCreators.forEach(innerType => {
const innerTypeName = innerType.constructor.name;
const create = innerType.creator;
for (const type of bufferSourceConstructors) {
const typeName = type.name;
const sut = conversions[typeName];

(innerType === type ? testOk : testNotOk)(innerTypeName, sut, create);
});
describe("WebIDL " + typeName + " type", () => {
for (const innerType of bufferSourceCreators) {
const testFunction = innerType.typeName === typeName ? testOk : testNotOk;
testFunction(innerType.label, sut, innerType.creator);
}

commonNotOk(sut);
});
});
}

describe("WebIDL ArrayBufferView type", () => {
const sut = conversions.ArrayBufferView;

bufferSourceCreators.forEach(type => {
const name = type.constructor.name;
const create = type.creator;

(name === "ArrayBuffer" ? testNotOk : testOk)(name, sut, create);
});
for (const { label, typeName, creator } of bufferSourceCreators) {
const testFunction = typeName !== "ArrayBuffer" ? testOk : testNotOk;
testFunction(label, sut, creator);
}

commonNotOk(sut);
});

describe("WebIDL BufferSource type", () => {
const sut = conversions.BufferSource;

bufferSourceCreators.forEach(type => {
const name = type.constructor.name;
const create = type.creator;

testOk(name, sut, create);
});
for (const { label, creator } of bufferSourceCreators) {
testOk(label, sut, creator);
}

commonNotOk(sut);
});
62 changes: 0 additions & 62 deletions test/error.js

This file was deleted.

0 comments on commit aa79e8c

Please sign in to comment.