Skip to content

Commit aa79e8c

Browse files
authored
Remove Error and make BufferSource checks realm-independent
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.
1 parent cf3d1c4 commit aa79e8c

File tree

4 files changed

+71
-98
lines changed

4 files changed

+71
-98
lines changed

README.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ Conversions for all of the basic types from the Web IDL specification are implem
4040
- [`DOMString`](https://heycam.github.io/webidl/#es-DOMString), which can additionally be provided the boolean option `{ treatNullAsEmptyString }` as a second parameter
4141
- [`ByteString`](https://heycam.github.io/webidl/#es-ByteString), [`USVString`](https://heycam.github.io/webidl/#es-USVString)
4242
- [`object`](https://heycam.github.io/webidl/#es-object)
43-
- [`Error`](https://heycam.github.io/webidl/#es-Error)
4443
- [Buffer source types](https://heycam.github.io/webidl/#es-buffer-source-types)
4544

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

7877
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.
7978

80-
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.
79+
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.

lib/index.js

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -290,16 +290,45 @@ function convertCallbackFunction(V, opts) {
290290
return V;
291291
}
292292

293+
const abByteLengthGetter =
294+
Object.getOwnPropertyDescriptor(ArrayBuffer.prototype, "byteLength").get;
295+
296+
function isArrayBuffer(V) {
297+
try {
298+
abByteLengthGetter.call(V);
299+
return true;
300+
} catch (e) {
301+
return false;
302+
}
303+
}
304+
305+
// I don't think we can reliably detect detached ArrayBuffers.
306+
exports.ArrayBuffer = (V, opts) => {
307+
if (!isArrayBuffer(V)) {
308+
throw new TypeError(_("is not a view on an ArrayBuffer object", opts));
309+
}
310+
return V;
311+
};
312+
313+
const dvByteLengthGetter =
314+
Object.getOwnPropertyDescriptor(DataView.prototype, "byteLength").get;
315+
exports.DataView = (V, opts) => {
316+
try {
317+
dvByteLengthGetter.call(V);
318+
return V;
319+
} catch (e) {
320+
throw new TypeError(_("is not a view on an DataView object", opts));
321+
}
322+
};
323+
293324
[
294-
Error,
295-
ArrayBuffer, // The IsDetachedBuffer abstract operation is not exposed in JS
296-
DataView, Int8Array, Int16Array, Int32Array, Uint8Array,
325+
Int8Array, Int16Array, Int32Array, Uint8Array,
297326
Uint16Array, Uint32Array, Uint8ClampedArray, Float32Array, Float64Array
298327
].forEach(func => {
299328
const name = func.name;
300329
const article = /^[AEIOU]/.test(name) ? "an" : "a";
301330
exports[name] = (V, opts) => {
302-
if (!(V instanceof func)) {
331+
if (!ArrayBuffer.isView(V) || V.constructor.name !== name) {
303332
throw new TypeError(_(`is not ${article} ${name} object`, opts));
304333
}
305334

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

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

test/buffer-source.js

Lines changed: 36 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"use strict";
22
const assert = require("assert");
3+
const vm = require("vm");
34

45
const conversions = require("..");
56

@@ -62,11 +63,10 @@ function testNotOk(name, sut, create) {
6263
});
6364
}
6465

65-
const bufferSourceCreators = [
66-
{ constructor: DataView, creator: () => new DataView(new ArrayBuffer(0)) }
67-
];
66+
const differentRealm = vm.createContext();
6867

69-
[
68+
const bufferSourceConstructors = [
69+
DataView,
7070
ArrayBuffer,
7171
Int8Array,
7272
Int16Array,
@@ -77,48 +77,55 @@ const bufferSourceCreators = [
7777
Uint8ClampedArray,
7878
Float32Array,
7979
Float64Array
80-
].forEach(constructor => {
81-
bufferSourceCreators.push({ constructor, creator: () => new constructor(0) });
82-
});
80+
];
8381

84-
bufferSourceCreators.forEach(type => {
85-
const name = type.constructor.name;
86-
const sut = conversions[name];
82+
const bufferSourceCreators = [];
83+
for (const constructor of bufferSourceConstructors) {
84+
bufferSourceCreators.push(
85+
{
86+
typeName: constructor.name,
87+
label: `${constructor.name} same realm`,
88+
creator: () => new constructor(new ArrayBuffer(0))
89+
},
90+
{
91+
typeName: constructor.name,
92+
label: `${constructor.name} different realm`,
93+
creator: () => vm.runInContext(`new ${constructor.name}(new ArrayBuffer(0))`, differentRealm)
94+
}
95+
);
96+
}
8797

88-
describe("WebIDL " + name + " type", () => {
89-
bufferSourceCreators.forEach(innerType => {
90-
const innerTypeName = innerType.constructor.name;
91-
const create = innerType.creator;
98+
for (const type of bufferSourceConstructors) {
99+
const typeName = type.name;
100+
const sut = conversions[typeName];
92101

93-
(innerType === type ? testOk : testNotOk)(innerTypeName, sut, create);
94-
});
102+
describe("WebIDL " + typeName + " type", () => {
103+
for (const innerType of bufferSourceCreators) {
104+
const testFunction = innerType.typeName === typeName ? testOk : testNotOk;
105+
testFunction(innerType.label, sut, innerType.creator);
106+
}
95107

96108
commonNotOk(sut);
97109
});
98-
});
110+
}
99111

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

103-
bufferSourceCreators.forEach(type => {
104-
const name = type.constructor.name;
105-
const create = type.creator;
106-
107-
(name === "ArrayBuffer" ? testNotOk : testOk)(name, sut, create);
108-
});
115+
for (const { label, typeName, creator } of bufferSourceCreators) {
116+
const testFunction = typeName !== "ArrayBuffer" ? testOk : testNotOk;
117+
testFunction(label, sut, creator);
118+
}
109119

110120
commonNotOk(sut);
111121
});
112122

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

116-
bufferSourceCreators.forEach(type => {
117-
const name = type.constructor.name;
118-
const create = type.creator;
119-
120-
testOk(name, sut, create);
121-
});
126+
for (const { label, creator } of bufferSourceCreators) {
127+
testOk(label, sut, creator);
128+
}
122129

123130
commonNotOk(sut);
124131
});

test/error.js

Lines changed: 0 additions & 62 deletions
This file was deleted.

0 commit comments

Comments
 (0)