diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index 2dbefc463781..0904138b9439 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -572,6 +572,10 @@ pub struct TablePrinter<'a> { tabular_data: JSValue, properties: JSValue, + /// Rows come from the iterator protocol (Map, Set, generators and other + /// iterables). Otherwise they are the own enumerable properties, like + /// Node's `Object.keys`: that includes arrays, so a hole or a `length` + /// an object merely reports never becomes a row. is_iterable: bool, jstype: jsc::JSType, @@ -658,7 +662,7 @@ impl<'a> TablePrinter<'a> { global_object, tabular_data, properties, - is_iterable: tabular_data.is_iterable(global_object)?, + is_iterable: tabular_data.is_non_array_iterable(global_object)?, jstype: tabular_data.js_type(), value_formatter: { // `Formatter` has a `Drop` impl, so struct-update diff --git a/src/jsc/JSValue.rs b/src/jsc/JSValue.rs index 09e31b639806..2e509fd5821b 100644 --- a/src/jsc/JSValue.rs +++ b/src/jsc/JSValue.rs @@ -2426,6 +2426,12 @@ impl JSValue { pub fn is_iterable(self, global: &JSGlobalObject) -> JsResult { host_fn::from_js_host_call_generic(global, || JSC__JSValue__isIterable(self, global)) } + /// [`is_iterable`](Self::is_iterable), but false for an array (through a + /// Proxy too) and for anything that iterates with the intrinsic Array + /// iterator, such as an `arguments` object. + pub fn is_non_array_iterable(self, global: &JSGlobalObject) -> JsResult { + crate::cpp::Bun__JSValue__isNonArrayIterable(self, global) + } /// `JSValue.forEach` — invoke `callback` for each iterable element. pub fn for_each( self, diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index bb17cb537ced..ef78e06e215c 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -4251,6 +4251,38 @@ bool JSC__JSValue__isIterable(JSC::EncodedJSValue JSValue, JSC::JSGlobalObject* return JSC::hasIteratorMethod(global, JSC::JSValue::decode(JSValue)); } +// Like hasIteratorMethod, but false for an array (IsArray, so a Proxy around +// an array too) and for anything that iterates with the intrinsic Array +// iterator (Array.prototype.values): an `arguments` object, a plain +// array-like. That iterator trusts the `length` the object reports and never +// checks that an element exists, so a caller that wants one step per existing +// element walks the object's own properties instead. +extern "C" [[ZIG_EXPORT(check_slow)]] bool Bun__JSValue__isNonArrayIterable(JSC::EncodedJSValue encodedValue, JSC::JSGlobalObject* globalObject) +{ + auto& vm = JSC::getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + + JSC::JSValue value = JSC::JSValue::decode(encodedValue); + if (!value.isObject()) + return false; + bool isArray = JSC::isArray(globalObject, value); + RETURN_IF_EXCEPTION(scope, false); + if (isArray) + return false; + + JSC::CallData callData; + JSC::JSValue method = asObject(value)->getMethod(globalObject, callData, vm.propertyNames->iteratorSymbol, "Symbol.iterator property should be callable"_s); + RETURN_IF_EXCEPTION(scope, false); + if (method.isUndefined()) + return false; + + auto* function = dynamicDowncast(method); + if (function && function == function->globalObject()->arrayProtoValuesFunctionConcurrently()) + return false; + + return true; +} + void JSC__JSValue__forEach(JSC::EncodedJSValue JSValue0, JSC::JSGlobalObject* arg1, void* ctx, void (*ArgFn3)(JSC::VM* arg0, JSC::JSGlobalObject* arg1, void* arg2, JSC::EncodedJSValue JSValue3)) { JSC::forEachInIterable( diff --git a/test/js/bun/console/console-table.test.ts b/test/js/bun/console/console-table.test.ts index 32fee910127d..38a663e2d1dc 100644 --- a/test/js/bun/console/console-table.test.ts +++ b/test/js/bun/console/console-table.test.ts @@ -363,3 +363,86 @@ console.log("calls=" + calls);`, expect({ stdout, stderr, exitCode }).toEqual({ stdout: box("1") + "calls=1\n", stderr: "", exitCode: 0 }); }); }); + +// The rows of an array or array-like are the elements that exist, like Node's +// console.table (Object.keys). The Array iterator instead trusts whatever +// `length` the object reports and yields `undefined` for every missing index, +// so a Proxy that lies about `length`, or a sparse array, would produce one +// row per reported index (up to 2^32 - 1 of them). +describe("console.table rows come from the elements that exist, not from length", () => { + const table = (...lines: string[]) => + `┌───┬───┐\n│ │ a │\n├───┼───┤\n${lines.map(l => l + "\n").join("")}└───┴───┘\n`; + + test("a Proxy that reports a length far beyond its elements", () => { + const proxy = new Proxy([{ a: 1 }, { a: 2 }], { + get(t, p, r) { + return p === "length" ? 1_000 : Reflect.get(t, p, r); + }, + }); + expect(Bun.inspect.table(proxy)).toBe(table("│ 0 │ 1 │", "│ 1 │ 2 │")); + }); + + test("a Proxy around an array still reads its cells through the get trap", () => { + const proxy = new Proxy([{ a: 1 }], { + get(t, p, r) { + return p === "0" ? { a: 2 } : Reflect.get(t, p, r); + }, + }); + expect(Bun.inspect.table(proxy)).toBe(table("│ 0 │ 2 │")); + }); + + test("a sparse array shows only the indices that hold an element", () => { + const sparse = [{ a: 1 }, , { a: 2 }]; + sparse.length = 1_000; + expect(Bun.inspect.table(sparse)).toBe(table("│ 0 │ 1 │", "│ 2 │ 2 │")); + }); + + test("an array with only holes is an empty table", () => { + expect(Bun.inspect.table(new Array(1_000))).toBe(Bun.inspect.table([])); + }); + + test("a properties filter applies to the elements that exist", () => { + const sparse = [{ a: 1, b: 2 }, , { a: 3, b: 4 }]; + expect(Bun.inspect.table(sparse, ["a"])).toBe(table("│ 0 │ 1 │", "│ 2 │ 3 │")); + }); + + // Object.freeze moves every element of an array into its sparse map. + test("a frozen array with a hole", () => { + const frozen = Object.freeze([{ a: 1 }, , { a: 2 }]); + expect(Bun.inspect.table(frozen)).toBe(table("│ 0 │ 1 │", "│ 2 │ 2 │")); + }); + + test("an array is walked like an object: its named properties are rows too", () => { + const arr = Object.assign([{ a: 1 }], { foo: { a: 2 } }); + expect(Bun.inspect.table(arr)).toBe(Bun.inspect.table({ 0: { a: 1 }, foo: { a: 2 } })); + expect(Bun.inspect.table(new Proxy(arr, {}))).toBe(Bun.inspect.table(arr)); + }); + + test("an Array subclass with its own iterator is still walked by its elements", () => { + class Rows extends Array { + *[Symbol.iterator]() { + yield { a: "from the iterator" }; + } + } + expect(Bun.inspect.table(Rows.from([{ a: 1 }]))).toBe(table("│ 0 │ 1 │")); + }); + + test("console.table", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const proxy = new Proxy([{ a: 1 }, { a: 2 }], { + get(t, p, r) { + return p === "length" ? 1_000 : Reflect.get(t, p, r); + }, +}); +console.table(proxy);`, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: table("│ 0 │ 1 │", "│ 1 │ 2 │"), stderr: "", exitCode: 0 }); + }); +});