Skip to content
This repository was archived by the owner on Mar 13, 2025. It is now read-only.

Commit 63befac

Browse files
mrbbotmrbbot
committed
Revert "Don't proxy primitive instanceof checks by default"
This reverts commit 720794a.
1 parent d99015a commit 63befac

File tree

15 files changed

+17
-197
lines changed

15 files changed

+17
-197
lines changed

‎packages/cli-parser/test/help.spec.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ Core Options:
5656
--global-timers Allow setting timers outside handlers [boolean]
5757
--global-random Allow secure random generation outside [boolean]
5858
handlers
59-
--proxy-primitive Proxy primitives' instanceof (for WASM) [boolean]
6059
6160
Test Options:
6261
-b, --boolean-option Boolean option [boolean]

‎packages/core/src/index.ts‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -697,12 +697,8 @@ export class MiniflareCore<
697697
const setupWatch = this.#setupWatch!.get(name);
698698
if (setupWatch) addAll(newWatchPaths, setupWatch);
699699
}
700-
const {
701-
modules,
702-
processedModuleRules,
703-
logUnhandledRejections,
704-
proxyPrimitiveInstanceOf,
705-
} = this.#instances!.CorePlugin;
700+
const { modules, processedModuleRules, logUnhandledRejections } =
701+
this.#instances!.CorePlugin;
706702

707703
// Clean up process-wide promise rejection event listeners
708704
this.#globalScope?.[kDispose]();
@@ -738,8 +734,7 @@ export class MiniflareCore<
738734
globalScope,
739735
script,
740736
rules,
741-
additionalModules,
742-
proxyPrimitiveInstanceOf
737+
additionalModules
743738
);
744739

745740
this.#scriptWatchPaths.clear();

‎packages/core/src/plugins/core.ts‎

Lines changed: 0 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@ export interface CoreOptions {
102102
globalAsyncIO?: boolean;
103103
globalTimers?: boolean;
104104
globalRandom?: boolean;
105-
proxyPrimitiveInstanceOf?: boolean;
106105
}
107106

108107
function mapMountEntries(
@@ -340,15 +339,6 @@ export class CorePlugin extends Plugin<CoreOptions> implements CoreOptions {
340339
})
341340
globalRandom?: boolean;
342341

343-
@Option({
344-
type: OptionType.BOOLEAN,
345-
name: "proxy-primitive",
346-
description: "Proxy primitives' instanceof (for WASM)",
347-
logName: "Proxy Primitives' instanceof",
348-
fromWrangler: ({ miniflare }) => miniflare?.proxy_primitive_instanceof,
349-
})
350-
proxyPrimitiveInstanceOf?: boolean;
351-
352342
readonly processedModuleRules: ProcessedModuleRule[] = [];
353343

354344
readonly upstreamURL?: URL;
@@ -483,76 +473,6 @@ export class CorePlugin extends Plugin<CoreOptions> implements CoreOptions {
483473
// could be used as an escape hatch if behaviour needs to be different
484474
// locally for any reason
485475
MINIFLARE: true,
486-
487-
// Object, Array, Promise, RegExp, Error, EvalError, RangeError,
488-
// ReferenceError, SyntaxError, TypeError, URIError and Function are
489-
// intentionally omitted. There's a trade-off here, ideally we'd like all
490-
// these checks to pass:
491-
//
492-
// ```js
493-
// import vm from "vm";
494-
//
495-
// const ctx1 = vm.createContext({ objectFunction: () => ({}) });
496-
//
497-
// vm.runInContext("({}) instanceof Object", ctx1); // true
498-
// vm.runInContext("({}).constructor === Object", ctx1); // true
499-
//
500-
// vm.runInContext("objectFunction() instanceof Object", ctx1); // false
501-
// vm.runInContext("objectFunction().constructor === Object", ctx1); // false
502-
//
503-
// const ctx2 = vm.createContext({ Object: Object, objectFunction: () => ({}) });
504-
//
505-
// vm.runInContext("({}) instanceof Object", ctx2); // false
506-
// vm.runInContext("({}).constructor === Object", ctx2); // false
507-
//
508-
// vm.runInContext("objectFunction() instanceof Object", ctx2); // true
509-
// vm.runInContext("objectFunction().constructor === Object", ctx2); // true
510-
// ```
511-
//
512-
// wasm-bindgen (a tool used to make compiling Rust to WebAssembly easier),
513-
// often generates code that looks like `value instanceof Object`.
514-
// We'd like this check to succeed for both objects generated outside of
515-
// the worker (e.g. KV), and inside user code. This is what the
516-
// `proxyPrimitiveInstanceOf` option is for. It replaces `Object` with
517-
// a proxy that overrides `Symbol.hasInstance` with a cross-realm check:
518-
//
519-
// ```js
520-
// function isObject(value) {
521-
// return value !== null && typeof value === "object";
522-
// }
523-
//
524-
// const ObjectProxy = new Proxy(Object, {
525-
// get(target, property, receiver) {
526-
// if (property === Symbol.hasInstance) return isObject;
527-
// return Reflect.get(target, property, receiver);
528-
// },
529-
// });
530-
//
531-
// const ctx3 = vm.createContext({
532-
// Object: ObjectProxy,
533-
// objectFunction: () => ({}),
534-
// });
535-
//
536-
// vm.runInContext("({}) instanceof Object", ctx3); // true
537-
// vm.runInContext("({}).constructor === Object", ctx3); // false
538-
//
539-
// vm.runInContext("objectFunction() instanceof Object", ctx3); // true
540-
// vm.runInContext("objectFunction().constructor === Object", ctx3); // false
541-
// ```
542-
//
543-
// The problem with this option (it used to be the default) is that the
544-
// `constructor`/`prototype` checks fail. These are used quite a lot in
545-
// JS, and this was the cause of several issues:
546-
// - https://github.com/cloudflare/miniflare/issues/109
547-
// - https://github.com/cloudflare/miniflare/issues/137
548-
// - https://github.com/cloudflare/miniflare/issues/141
549-
// - https://github.com/cloudflare/wrangler2/issues/91
550-
//
551-
// The new default behaviour (omitting `Object` completely) still has
552-
// the issue `constructor`/`prototype`/`instanceof` checks for `Object`s
553-
// created outside the sandbox (e.g. KV) would fail, but I think that's
554-
// less likely to be an issue, since the return types are always known
555-
// in those cases.
556476
};
557477

558478
// Process module rules if modules mode was enabled

‎packages/core/test/index.spec.ts‎

Lines changed: 0 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -677,61 +677,6 @@ test("MiniflareCore: #reload: only runs script if module exports needed when scr
677677
await mf.getPlugins(); // Allow script to run
678678
t.true(calledback);
679679
});
680-
test("MiniflareCore: #reload: primitive constructor/prototype/instanceof checks succeed", async (t) => {
681-
const mf = useMiniflareWithHandler(
682-
{ BindingsPlugin },
683-
{ globals: { outsideObject: {} } },
684-
(globals) => {
685-
const tests = {
686-
// Simulating wasm-bindgen
687-
outsideInstanceOf: globals.outsideObject instanceof globals.Object,
688-
insideInstanceOf: {} instanceof globals.Object,
689-
690-
// https://github.com/cloudflare/miniflare/issues/109
691-
// https://github.com/cloudflare/miniflare/issues/141
692-
outsideConstructor:
693-
globals.outsideObject.constructor === globals.Object,
694-
insideConstructor: {}.constructor === globals.Object,
695-
696-
// https://github.com/cloudflare/miniflare/issues/137
697-
newObject: new globals.Object({ a: 1 }),
698-
699-
// https://github.com/cloudflare/wrangler2/issues/91
700-
outsidePrototype:
701-
Object.getPrototypeOf(globals.outsideObject) ===
702-
globals.Object.prototype,
703-
insidePrototype: Object.getPrototypeOf({}) === globals.Object.prototype,
704-
};
705-
return new globals.Response(JSON.stringify(tests), {
706-
headers: { "Content-Type": "application/json" },
707-
});
708-
}
709-
);
710-
711-
let res = await mf.dispatchFetch("http://localhost/");
712-
t.deepEqual(await res.json(), {
713-
outsideInstanceOf: false, // :(
714-
insideInstanceOf: true,
715-
outsideConstructor: false, // :(
716-
insideConstructor: true,
717-
newObject: { a: 1 },
718-
outsidePrototype: false, // :(
719-
insidePrototype: true,
720-
});
721-
722-
// Check with proxyPrimitiveInstanceOf enabled (for Rust development)
723-
await mf.setOptions({ proxyPrimitiveInstanceOf: true });
724-
res = await mf.dispatchFetch("http://localhost/");
725-
t.deepEqual(await res.json(), {
726-
outsideInstanceOf: true,
727-
insideInstanceOf: true,
728-
outsideConstructor: false,
729-
insideConstructor: false,
730-
newObject: {}, // This is super strange
731-
outsidePrototype: true,
732-
insidePrototype: false,
733-
});
734-
});
735680
test("MiniflareCore: #reload: watches files", async (t) => {
736681
const log = new TestLog();
737682
const tmp = await useTmp(t);

‎packages/core/test/plugins/bindings.spec.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -508,8 +508,6 @@ test("BindingsPlugin: propagates error if service throws", async (t) => {
508508
script: `export default {
509509
fetch: () => { throw new Error("oops"); },
510510
}`,
511-
// Required for instanceOf assertion below to work
512-
proxyPrimitiveInstanceOf: true,
513511
},
514512
},
515513
}

‎packages/core/test/plugins/core.spec.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ test("CorePlugin: parses options from argv", (t) => {
6363
"--global-async-io",
6464
"--global-timers",
6565
"--global-random",
66-
"--proxy-primitive",
6766
]);
6867
t.deepEqual(options, {
6968
scriptPath: "script.js",
@@ -106,7 +105,6 @@ test("CorePlugin: parses options from argv", (t) => {
106105
globalAsyncIO: true,
107106
globalTimers: true,
108107
globalRandom: true,
109-
proxyPrimitiveInstanceOf: true,
110108
});
111109
options = parsePluginArgv(CorePlugin, [
112110
"-c",
@@ -159,7 +157,6 @@ test("CorePlugin: parses options from wrangler config", async (t) => {
159157
global_async_io: true,
160158
global_timers: true,
161159
global_random: true,
162-
proxy_primitive_instanceof: true,
163160
},
164161
},
165162
configDir
@@ -213,7 +210,6 @@ test("CorePlugin: parses options from wrangler config", async (t) => {
213210
globalAsyncIO: true,
214211
globalTimers: true,
215212
globalRandom: true,
216-
proxyPrimitiveInstanceOf: true,
217213
});
218214
// Check build upload dir defaults to dist
219215
options = parsePluginWranglerConfig(
@@ -247,7 +243,6 @@ test("CorePlugin: logs options", (t) => {
247243
globalAsyncIO: true,
248244
globalTimers: true,
249245
globalRandom: true,
250-
proxyPrimitiveInstanceOf: true,
251246
});
252247
t.deepEqual(logs, [
253248
// script is OptionType.NONE so omitted
@@ -268,7 +263,6 @@ test("CorePlugin: logs options", (t) => {
268263
"Allow Global Async I/O: true",
269264
"Allow Global Timers: true",
270265
"Allow Global Secure Random: true",
271-
"Proxy Primitives' instanceof: true",
272266
]);
273267
// Check logs default wrangler config/package paths
274268
logs = logPluginOptions(CorePlugin, {

‎packages/jest-environment-miniflare/src/index.ts‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -169,18 +169,12 @@ export default class MiniflareEnvironment implements JestEnvironment {
169169
}
170170
);
171171

172-
const plugins = await mf.getPlugins();
173172
const mfGlobalScope = await mf.getGlobalScope();
174173
mfGlobalScope.global = global;
175174
mfGlobalScope.self = global;
176175
// Make sure Miniflare's global scope is assigned to Jest's global context,
177176
// even if we didn't run a script because we had no Durable Objects
178-
if (plugins.CorePlugin.proxyPrimitiveInstanceOf) {
179-
Object.assign(
180-
global,
181-
makeProxiedGlobals(/* blockCodeGeneration */ false)
182-
);
183-
}
177+
Object.assign(global, makeProxiedGlobals(/* blockCodeGeneration */ false));
184178
Object.assign(global, mfGlobalScope);
185179

186180
// Add a way of getting bindings in modules mode to allow seeding data.

‎packages/jest-environment-miniflare/test/fixtures/autoload/autoload.spec.js‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,6 @@ test("auto-loads package.json and wrangler.toml", async () => {
66
expect(await res.text()).toBe("test");
77
});
88

9-
test("respects proxy_primitive_instanceof option", async () => {
10-
// Should be auto-loaded from wrangler.toml
11-
const { OBJECT } = getMiniflareBindings();
12-
expect(OBJECT instanceof Object).toBe(true);
13-
expect({} instanceof Object).toBe(true);
14-
});
15-
169
test("auto-loads .env", () => {
1710
const { ENV_KEY } = getMiniflareBindings();
1811
expect(ENV_KEY).toBe("value");

‎packages/jest-environment-miniflare/test/fixtures/autoload/wrangler.toml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,3 @@ bindings = [
55

66
[build.upload]
77
format = "modules"
8-
9-
[miniflare]
10-
proxy_primitive_instanceof = true

‎packages/jest-environment-miniflare/test/fixtures/core.worker.spec.js‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,3 @@ describe("uses Jest fake timers", () => {
3939
test("can generate secure random numbers", () => {
4040
crypto.getRandomValues(new Uint8Array(8));
4141
});
42-
43-
test("Object constructor checks succeed", () => {
44-
expect({}.constructor === Object).toBe(true);
45-
});

0 commit comments

Comments
 (0)