From 5fa58c92165e23386b8ed3c3079103997fe1bef9 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 8 Jul 2021 09:43:36 -0400 Subject: [PATCH] fix: inspecting prototypes of built-ins with custom inspect implementations should not throw (#11308) --- cli/tests/unit/blob_test.ts | 16 +++- cli/tests/unit/dom_exception_test.ts | 10 +++ cli/tests/unit/performance_test.ts | 27 ++++++ cli/tests/unit/request_test.ts | 3 +- cli/tests/unit/response_test.ts | 8 +- extensions/console/02_console.js | 57 ++++++++++++ extensions/console/internal.d.ts | 16 ++++ extensions/fetch/23_request.js | 20 +++-- extensions/fetch/23_response.js | 26 +++--- extensions/timers/02_performance.js | 42 ++++++++- extensions/web/01_dom_exception.js | 17 +++- extensions/web/02_event.js | 127 ++++++++++++--------------- extensions/web/06_streams.js | 69 ++++++++++----- extensions/web/09_file.js | 10 ++- 14 files changed, 325 insertions(+), 123 deletions(-) create mode 100644 cli/tests/unit/dom_exception_test.ts create mode 100644 extensions/console/internal.d.ts diff --git a/cli/tests/unit/blob_test.ts b/cli/tests/unit/blob_test.ts index f9e506334f..ebb91658a7 100644 --- a/cli/tests/unit/blob_test.ts +++ b/cli/tests/unit/blob_test.ts @@ -1,5 +1,10 @@ // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. -import { assert, assertEquals, unitTest } from "./test_util.ts"; +import { + assert, + assertEquals, + assertStringIncludes, + unitTest, +} from "./test_util.ts"; import { concat } from "../../../test_util/std/bytes/mod.ts"; unitTest(function blobString(): void { @@ -104,3 +109,12 @@ unitTest(function blobConstructorNameIsBlob(): void { const blob = new Blob(); assertEquals(blob.constructor.name, "Blob"); }); + +unitTest(function blobCustomInspectFunction(): void { + const blob = new Blob(); + assertEquals( + Deno.inspect(blob), + `Blob { size: 0, type: "" }`, + ); + assertStringIncludes(Deno.inspect(Blob.prototype), "Blob"); +}); diff --git a/cli/tests/unit/dom_exception_test.ts b/cli/tests/unit/dom_exception_test.ts new file mode 100644 index 0000000000..ea310fc9a8 --- /dev/null +++ b/cli/tests/unit/dom_exception_test.ts @@ -0,0 +1,10 @@ +import { assertEquals, assertStringIncludes, unitTest } from "./test_util.ts"; + +unitTest(function customInspectFunction(): void { + const blob = new DOMException("test"); + assertEquals( + Deno.inspect(blob), + `DOMException: test`, + ); + assertStringIncludes(Deno.inspect(DOMException.prototype), "DOMException"); +}); diff --git a/cli/tests/unit/performance_test.ts b/cli/tests/unit/performance_test.ts index 156841165b..94af0731c8 100644 --- a/cli/tests/unit/performance_test.ts +++ b/cli/tests/unit/performance_test.ts @@ -2,6 +2,7 @@ import { assert, assertEquals, + assertStringIncludes, assertThrows, deferred, unitTest, @@ -81,6 +82,32 @@ unitTest(function performanceMeasure() { }); }); +unitTest(function performanceCustomInspectFunction(): void { + assertStringIncludes(Deno.inspect(performance), "Performance"); + assertStringIncludes( + Deno.inspect(Performance.prototype), + "Performance", + ); +}); + +unitTest(function performanceMarkCustomInspectFunction(): void { + const mark1 = performance.mark("mark1"); + assertStringIncludes(Deno.inspect(mark1), "PerformanceMark"); + assertStringIncludes( + Deno.inspect(PerformanceMark.prototype), + "PerformanceMark", + ); +}); + +unitTest(function performanceMeasureCustomInspectFunction(): void { + const measure1 = performance.measure("measure1"); + assertStringIncludes(Deno.inspect(measure1), "PerformanceMeasure"); + assertStringIncludes( + Deno.inspect(PerformanceMeasure.prototype), + "PerformanceMeasure", + ); +}); + unitTest(function performanceIllegalConstructor() { assertThrows(() => new Performance(), TypeError, "Illegal constructor"); assertEquals(Performance.length, 0); diff --git a/cli/tests/unit/request_test.ts b/cli/tests/unit/request_test.ts index 7a3322daac..dca9e4a73d 100644 --- a/cli/tests/unit/request_test.ts +++ b/cli/tests/unit/request_test.ts @@ -1,5 +1,5 @@ // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. -import { assertEquals, unitTest } from "./test_util.ts"; +import { assertEquals, assertStringIncludes, unitTest } from "./test_util.ts"; unitTest(async function fromInit(): Promise { const req = new Request("http://foo/", { @@ -66,4 +66,5 @@ unitTest(function customInspectFunction(): void { url: "https://example.com/" }`, ); + assertStringIncludes(Deno.inspect(Request.prototype), "Request"); }); diff --git a/cli/tests/unit/response_test.ts b/cli/tests/unit/response_test.ts index 7e444fd83b..1faf8991aa 100644 --- a/cli/tests/unit/response_test.ts +++ b/cli/tests/unit/response_test.ts @@ -1,5 +1,10 @@ // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. -import { assert, assertEquals, unitTest } from "./test_util.ts"; +import { + assert, + assertEquals, + assertStringIncludes, + unitTest, +} from "./test_util.ts"; unitTest(async function responseText() { const response = new Response("hello world"); @@ -67,4 +72,5 @@ unitTest(function customInspectFunction(): void { url: "" }`, ); + assertStringIncludes(Deno.inspect(Response.prototype), "Response"); }); diff --git a/extensions/console/02_console.js b/extensions/console/02_console.js index 6f2ed0c0ea..b4b29e1ea8 100644 --- a/extensions/console/02_console.js +++ b/extensions/console/02_console.js @@ -87,6 +87,10 @@ MathFloor, Number, NumberPrototypeToString, + Proxy, + ReflectGet, + ReflectGetOwnPropertyDescriptor, + ReflectGetPrototypeOf, WeakMap, WeakSet, } = window.__bootstrap.primordials; @@ -1953,6 +1957,58 @@ }); } + /** Creates a proxy that represents a subset of the properties + * of the original object optionally without evaluating the properties + * in order to get the values. */ + function createFilteredInspectProxy({ object, keys, evaluate }) { + return new Proxy({}, { + get(_target, key) { + if (key === SymbolToStringTag) { + return object.constructor?.name; + } else if (ArrayPrototypeIncludes(keys, key)) { + return ReflectGet(object, key); + } else { + return undefined; + } + }, + getOwnPropertyDescriptor(_target, key) { + if (!ArrayPrototypeIncludes(keys, key)) { + return undefined; + } else if (evaluate) { + return getEvaluatedDescriptor(object, key); + } else { + return getDescendantPropertyDescriptor(object, key) ?? + getEvaluatedDescriptor(object, key); + } + }, + has(_target, key) { + return ArrayPrototypeIncludes(keys, key); + }, + ownKeys() { + return keys; + }, + }); + + function getDescendantPropertyDescriptor(object, key) { + let propertyDescriptor = ReflectGetOwnPropertyDescriptor(object, key); + if (!propertyDescriptor) { + const prototype = ReflectGetPrototypeOf(object); + if (prototype) { + propertyDescriptor = getDescendantPropertyDescriptor(prototype, key); + } + } + return propertyDescriptor; + } + + function getEvaluatedDescriptor(object, key) { + return { + configurable: true, + enumerable: true, + value: object[key], + }; + } + } + // A helper function that will bind our own console implementation // with default implementation of Console from V8. This will cause // console messages to be piped to inspector console. @@ -1997,5 +2053,6 @@ customInspect, inspect, wrapConsole, + createFilteredInspectProxy, }; })(this); diff --git a/extensions/console/internal.d.ts b/extensions/console/internal.d.ts new file mode 100644 index 0000000000..ef7834ba6c --- /dev/null +++ b/extensions/console/internal.d.ts @@ -0,0 +1,16 @@ +// Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. + +/// +/// + +declare namespace globalThis { + declare namespace __bootstrap { + declare namespace console { + declare function createFilteredInspectProxy(params: { + object: TObject; + keys: (keyof TObject)[]; + evaluate: boolean; + }): Record; + } + } +} diff --git a/extensions/fetch/23_request.js b/extensions/fetch/23_request.js index 829f7e6dcf..1372125c11 100644 --- a/extensions/fetch/23_request.js +++ b/extensions/fetch/23_request.js @@ -12,6 +12,7 @@ ((window) => { const webidl = window.__bootstrap.webidl; + const consoleInternal = window.__bootstrap.console; const { HTTP_TOKEN_CODE_POINT_RE, byteUpperCase } = window.__bootstrap.infra; const { URL } = window.__bootstrap.url; const { guardFromHeaders } = window.__bootstrap.headers; @@ -393,14 +394,17 @@ } [SymbolFor("Deno.customInspect")](inspect) { - const inner = { - bodyUsed: this.bodyUsed, - headers: this.headers, - method: this.method, - redirect: this.redirect, - url: this.url, - }; - return `Request ${inspect(inner)}`; + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof Request, + keys: [ + "bodyUsed", + "headers", + "method", + "redirect", + "url", + ], + })); } } diff --git a/extensions/fetch/23_response.js b/extensions/fetch/23_response.js index 11eb13570f..0db20e90e2 100644 --- a/extensions/fetch/23_response.js +++ b/extensions/fetch/23_response.js @@ -13,6 +13,7 @@ ((window) => { const webidl = window.__bootstrap.webidl; + const consoleInternal = window.__bootstrap.console; const { HTTP_TAB_OR_SPACE, regexMatcher } = window.__bootstrap.infra; const { extractBody, mixinBody } = window.__bootstrap.fetchBody; const { getLocationHref } = window.__bootstrap.location; @@ -377,17 +378,20 @@ } [SymbolFor("Deno.customInspect")](inspect) { - const inner = { - body: this.body, - bodyUsed: this.bodyUsed, - headers: this.headers, - ok: this.ok, - redirected: this.redirected, - status: this.status, - statusText: this.statusText, - url: this.url, - }; - return `Response ${inspect(inner)}`; + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof Response, + keys: [ + "body", + "bodyUsed", + "headers", + "ok", + "redirected", + "status", + "statusText", + "url", + ], + })); } } diff --git a/extensions/timers/02_performance.js b/extensions/timers/02_performance.js index b4cf5760bc..f752ba933e 100644 --- a/extensions/timers/02_performance.js +++ b/extensions/timers/02_performance.js @@ -16,6 +16,7 @@ } = window.__bootstrap.primordials; const { webidl, structuredClone } = window.__bootstrap; + const consoleInternal = window.__bootstrap.console; const { opNow } = window.__bootstrap.timers; const { DOMException } = window.__bootstrap.domException; @@ -175,7 +176,16 @@ } [customInspect](inspect) { - return `${this.constructor.name} ${inspect(this.toJSON())}`; + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof PerformanceEntry, + keys: [ + "name", + "entryType", + "startTime", + "duration", + ], + })); } } webidl.configurePrototype(PerformanceEntry); @@ -235,7 +245,17 @@ } [customInspect](inspect) { - return `${this.constructor.name} ${inspect(this.toJSON())}`; + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof PerformanceMark, + keys: [ + "name", + "entryType", + "startTime", + "duration", + "detail", + ], + })); } } webidl.configurePrototype(PerformanceMark); @@ -283,7 +303,17 @@ } [customInspect](inspect) { - return `${this.constructor.name} ${inspect(this.toJSON())}`; + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof PerformanceMeasure, + keys: [ + "name", + "entryType", + "startTime", + "duration", + "detail", + ], + })); } } webidl.configurePrototype(PerformanceMeasure); @@ -516,7 +546,11 @@ } [customInspect](inspect) { - return `${this.constructor.name} ${inspect(this.toJSON())}`; + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof Performance, + keys: [], + })); } get [SymbolToStringTag]() { diff --git a/extensions/web/01_dom_exception.js b/extensions/web/01_dom_exception.js index 90ddb267b2..3e282d9690 100644 --- a/extensions/web/01_dom_exception.js +++ b/extensions/web/01_dom_exception.js @@ -17,6 +17,7 @@ ObjectSetPrototypeOf, } = window.__bootstrap.primordials; const webidl = window.__bootstrap.webidl; + const consoleInternal = window.__bootstrap.console; // Defined in WebIDL 4.3. // https://heycam.github.io/webidl/#idl-DOMException @@ -109,8 +110,20 @@ return "DOMException"; } - [Symbol.for("Deno.customInspect")]() { - return `DOMException: ${this.#message}`; + [Symbol.for("Deno.customInspect")](inspect) { + if (this instanceof DOMException) { + return `DOMException: ${this.#message}`; + } else { + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: false, + keys: [ + "message", + "name", + "code", + ], + })); + } } } diff --git a/extensions/web/02_event.js b/extensions/web/02_event.js index 4ae8b50f2a..6b2cc2c047 100644 --- a/extensions/web/02_event.js +++ b/extensions/web/02_event.js @@ -9,6 +9,7 @@ ((window) => { const webidl = window.__bootstrap.webidl; const { DOMException } = window.__bootstrap.domException; + const consoleInternal = window.__bootstrap.console; const { ArrayPrototypeFilter, ArrayPrototypeIncludes, @@ -28,10 +29,7 @@ ObjectCreate, ObjectDefineProperty, ObjectGetOwnPropertyDescriptor, - Proxy, ReflectDefineProperty, - ReflectGet, - ReflectGetOwnPropertyDescriptor, Symbol, SymbolFor, SymbolToStringTag, @@ -175,7 +173,11 @@ } [SymbolFor("Deno.privateCustomInspect")](inspect) { - return inspect(buildFilteredPropertyInspectObject(this, EVENT_PROPS)); + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof Event, + keys: EVENT_PROPS, + })); } get type() { @@ -397,43 +399,6 @@ } } - function buildFilteredPropertyInspectObject(object, keys) { - // forward the subset of properties from `object` without evaluating - // as evaluation could lead to an error, which is better handled - // in the inspect code - return new Proxy({}, { - get(_target, key) { - if (key === SymbolToStringTag) { - return object.constructor?.name; - } else if (ArrayPrototypeIncludes(keys, key)) { - return ReflectGet(object, key); - } else { - return undefined; - } - }, - getOwnPropertyDescriptor(_target, key) { - if (!ArrayPrototypeIncludes(keys, key)) { - return undefined; - } - - return ReflectGetOwnPropertyDescriptor(object, key) ?? - (object.prototype && - ReflectGetOwnPropertyDescriptor(object.prototype, key)) ?? - { - configurable: true, - enumerable: true, - value: object[key], - }; - }, - has(_target, key) { - return ArrayPrototypeIncludes(keys, key); - }, - ownKeys() { - return keys; - }, - }); - } - function defineEnumerableProps( Ctor, props, @@ -1097,14 +1062,18 @@ } [SymbolFor("Deno.privateCustomInspect")](inspect) { - return inspect(buildFilteredPropertyInspectObject(this, [ - ...EVENT_PROPS, - "message", - "filename", - "lineno", - "colno", - "error", - ])); + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof ErrorEvent, + keys: [ + ...EVENT_PROPS, + "message", + "filename", + "lineno", + "colno", + "error", + ], + })); } } @@ -1151,12 +1120,16 @@ } [SymbolFor("Deno.privateCustomInspect")](inspect) { - return inspect(buildFilteredPropertyInspectObject(this, [ - ...EVENT_PROPS, - "wasClean", - "code", - "reason", - ])); + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof CloseEvent, + keys: [ + ...EVENT_PROPS, + "wasClean", + "code", + "reason", + ], + })); } } @@ -1179,12 +1152,16 @@ } [SymbolFor("Deno.privateCustomInspect")](inspect) { - return inspect(buildFilteredPropertyInspectObject(this, [ - ...EVENT_PROPS, - "data", - "origin", - "lastEventId", - ])); + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof MessageEvent, + keys: [ + ...EVENT_PROPS, + "data", + "origin", + "lastEventId", + ], + })); } } @@ -1209,10 +1186,14 @@ } [SymbolFor("Deno.privateCustomInspect")](inspect) { - return inspect(buildFilteredPropertyInspectObject(this, [ - ...EVENT_PROPS, - "detail", - ])); + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof CustomEvent, + keys: [ + ...EVENT_PROPS, + "detail", + ], + })); } } @@ -1232,12 +1213,16 @@ } [SymbolFor("Deno.privateCustomInspect")](inspect) { - return inspect(buildFilteredPropertyInspectObject(this, [ - ...EVENT_PROPS, - "lengthComputable", - "loaded", - "total", - ])); + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof ProgressEvent, + keys: [ + ...EVENT_PROPS, + "lengthComputable", + "loaded", + "total", + ], + })); } } diff --git a/extensions/web/06_streams.js b/extensions/web/06_streams.js index 388b7b13cf..ff6c9d7d82 100644 --- a/extensions/web/06_streams.js +++ b/extensions/web/06_streams.js @@ -36,6 +36,7 @@ WeakMapPrototypeHas, WeakMapPrototypeSet, } = globalThis.__bootstrap.primordials; + const consoleInternal = window.__bootstrap.console; const { DOMException } = window.__bootstrap.domException; class AssertionError extends Error { @@ -3018,9 +3019,14 @@ } [Symbol.for("Deno.customInspect")](inspect) { - return `${this.constructor.name} ${ - inspect({ highWaterMark: this.highWaterMark, size: this.size }) - }`; + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof ByteLengthQueuingStrategy, + keys: [ + "highWaterMark", + "size", + ], + })); } get [Symbol.toStringTag]() { @@ -3069,9 +3075,14 @@ } [Symbol.for("Deno.customInspect")](inspect) { - return `${this.constructor.name} ${ - inspect({ highWaterMark: this.highWaterMark, size: this.size }) - }`; + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof CountQueuingStrategy, + keys: [ + "highWaterMark", + "size", + ], + })); } get [Symbol.toStringTag]() { @@ -3561,9 +3572,11 @@ } [Symbol.for("Deno.customInspect")](inspect) { - return `${this.constructor.name} ${ - inspect({ desiredSize: this.desiredSize }) - }`; + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof ReadableByteStreamController, + keys: ["desiredSize"], + })); } get [Symbol.toStringTag]() { @@ -3684,9 +3697,11 @@ } [Symbol.for("Deno.customInspect")](inspect) { - return `${this.constructor.name} ${ - inspect({ desiredSize: this.desiredSize }) - }`; + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof ReadableStreamDefaultController, + keys: ["desiredSize"], + })); } get [Symbol.toStringTag]() { @@ -3905,9 +3920,11 @@ } [Symbol.for("Deno.customInspect")](inspect) { - return `${this.constructor.name} ${ - inspect({ desiredSize: this.desiredSize }) - }`; + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof TransformStreamDefaultController, + keys: ["desiredSize"], + })); } get [Symbol.toStringTag]() { @@ -4182,13 +4199,15 @@ } [Symbol.for("Deno.customInspect")](inspect) { - return `${this.constructor.name} ${ - inspect({ - closed: this.closed, - desiredSize: this.desiredSize, - ready: this.ready, - }) - }`; + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof WritableStreamDefaultWriter, + keys: [ + "closed", + "desiredSize", + "ready", + ], + })); } get [Symbol.toStringTag]() { @@ -4240,7 +4259,11 @@ } [Symbol.for("Deno.customInspect")](inspect) { - return `${this.constructor.name} ${inspect({})}`; + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof WritableStreamDefaultController, + keys: [], + })); } get [Symbol.toStringTag]() { diff --git a/extensions/web/09_file.js b/extensions/web/09_file.js index a39b380a68..516e80adfc 100644 --- a/extensions/web/09_file.js +++ b/extensions/web/09_file.js @@ -34,6 +34,7 @@ TypeError, Uint8Array, } = window.__bootstrap.primordials; + const consoleInternal = window.__bootstrap.console; // TODO(lucacasonato): this needs to not be hardcoded and instead depend on // host os. @@ -362,7 +363,14 @@ } [SymbolFor("Deno.customInspect")](inspect) { - return `Blob ${inspect({ size: this.size, type: this.#type })}`; + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof Blob, + keys: [ + "size", + "type", + ], + })); } }