Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/bun_core/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1691,6 +1691,15 @@ pub mod io {
panic!("io::Write::written_len: writer does not track bytes written");
}

/// Whether this sink has hit an output cap and is discarding writes.
/// Byte-budgeted sinks override this; producers can poll it to stop
/// generating output early. Writes after truncation still return
/// `Ok`, so infallible-sink callers need no changes.
Comment thread
robobun marked this conversation as resolved.
Resolved
#[inline]
fn is_truncated(&self) -> bool {
false
}
Comment thread
robobun marked this conversation as resolved.
Resolved

// ── provided helpers ────────────────────────────────────────────────

/// Write a single byte.
Expand Down Expand Up @@ -1789,6 +1798,10 @@ pub mod io {
fn written_len(&self) -> usize {
(**self).written_len()
}
#[inline]
fn is_truncated(&self) -> bool {
(**self).is_truncated()
}
}

impl<W: Write + ?Sized> Write for Box<W> {
Expand All @@ -1804,6 +1817,10 @@ pub mod io {
fn written_len(&self) -> usize {
(**self).written_len()
}
#[inline]
fn is_truncated(&self) -> bool {
(**self).is_truncated()
}
}

/// In-memory growable sink.
Expand Down
71 changes: 70 additions & 1 deletion src/jsc/ConsoleObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1608,6 +1608,12 @@
/// printed as a string. Set true in the error printer so that
/// `ShellError` prints a more readable message.
pub(crate) format_buffer_as_text: bool,
/// Byte budget for one `ZigFormatter` (`Display`) render of a value.
/// Wide object graphs (e.g. DOM trees) can otherwise expand to
/// gigabytes — past `WTF::String::MaxLength` the resulting error
/// message cannot even be materialized as a JS string. Output is cut
/// at the budget with a truncation marker. `usize::MAX` = unlimited.
Comment thread
robobun marked this conversation as resolved.
Resolved
pub max_output_bytes: usize,
}

impl<'a> Formatter<'a> {
Expand Down Expand Up @@ -1638,6 +1644,7 @@
can_throw_stack_overflow: false,
error_display_level: ErrorDisplayLevel::Full,
format_buffer_as_text: false,
max_output_bytes: usize::MAX,
}
}

Expand Down Expand Up @@ -1673,6 +1680,7 @@
can_throw_stack_overflow: self.can_throw_stack_overflow,
error_display_level: self.error_display_level,
format_buffer_as_text: self.format_buffer_as_text,
max_output_bytes: self.max_output_bytes,
}
}

Expand Down Expand Up @@ -1758,6 +1766,48 @@
}
}

/// Byte sink that forwards to `inner` until a byte budget is exhausted,
/// then silently discards further writes and reports `is_truncated()`.
/// It never fails because of the cap, so the formatter's infallible-write
/// call sites (`.expect("unreachable")`) hold; `print_as_prelude` polls
/// `is_truncated()` and sets `Formatter.failed`, which is what stops the
/// walk over the value graph shortly after the cap.
Comment thread
robobun marked this conversation as resolved.
Resolved
struct TruncatingWriter<'a> {
inner: &'a mut dyn bun_io::Write,
remaining: usize,
truncated: bool,
}

impl bun_io::Write for TruncatingWriter<'_> {
fn write_all(&mut self, buf: &[u8]) -> bun_io::Result<()> {
if self.truncated {
return Ok(());
}
if buf.len() <= self.remaining {
self.remaining -= buf.len();
return self.inner.write_all(buf);
}
// Keep the prefix that fits, backed off to a UTF-8 boundary so
// the fmt bridge doesn't emit a replacement char for a split
// code point.
Comment thread
robobun marked this conversation as resolved.
Resolved
let mut take = self.remaining;
while take > 0 && buf[take] & 0b1100_0000 == 0b1000_0000 {
take -= 1;
}
self.remaining = 0;
self.truncated = true;
if take > 0 {
self.inner.write_all(&buf[..take])?;
}
Ok(())
}

#[inline]
fn is_truncated(&self) -> bool {
self.truncated
}
}
Comment thread
robobun marked this conversation as resolved.
Resolved

impl core::fmt::Display for ZigFormatter<'_, '_> {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
let formatter: &mut Formatter<'_> = self
Expand Down Expand Up @@ -3216,6 +3266,12 @@
if self.failed {
return Ok(false);
}
if writer_.is_truncated() {
// A byte-budgeted sink hit its cap; stop walking the value
// graph instead of rendering into a discarding writer.
Comment thread
robobun marked this conversation as resolved.
Resolved
self.failed = true;
return Ok(false);
}
if self.global_this.has_exception() {
return Err(jsc::JsError::Thrown);
}
Expand Down Expand Up @@ -5628,9 +5684,22 @@
let one = [value];
self.remaining_values = bun_ptr::RawSlice::new(&one);
let global = self.global_this;
// Honor `max_output_bytes`: cap the rendered output and finish
// with a truncation marker when the budget is hit.
Comment thread
robobun marked this conversation as resolved.
Resolved
let mut sink = TruncatingWriter {
inner: &mut *writer,
remaining: self.max_output_bytes,
truncated: false,
};
let result = Tag::get(value, global)
.and_then(|tag| self.format::<ENABLE_ANSI_COLORS>(tag, writer, value, global));
.and_then(|tag| self.format::<ENABLE_ANSI_COLORS>(tag, &mut sink, value, global));
let truncated = sink.truncated;
drop(sink);

Check failure on line 5697 in src/jsc/ConsoleObject.rs

View workflow job for this annotation

GitHub Actions / cargo clippy

call to `std::mem::drop` with a value that does not implement `Drop`. Dropping such a type only extends its contained lifetimes
self.remaining_values = bun_ptr::RawSlice::EMPTY;
if truncated && result.is_ok() {
self.failed = false;
let _ = writer.write_all(b"... [value truncated]");
}
result
}
}
Expand Down
12 changes: 11 additions & 1 deletion src/jsc/JSGlobalObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,17 @@ impl JSGlobalObject {
pub fn throw(&self, args: Arguments<'_>) -> JsError {
let instance = self.create_error_instance(args);
if instance.is_empty() {
debug_assert!(self.has_exception());
if !self.has_exception() {
// Message creation can fail without leaving an exception
// pending (e.g. the rendered message exceeds the maximum JS
// string length). Returning `Thrown` with no pending
// exception would silently swallow the throw, so throw a
// fallback instead.
Comment thread
robobun marked this conversation as resolved.
Resolved
return self.throw_value(
BunString::static_("error message is too long to display")
.to_error_instance(self),
);
}
Comment thread
robobun marked this conversation as resolved.
Resolved
return JsError::Thrown;
}
self.throw_value(instance)
Expand Down
2 changes: 2 additions & 0 deletions src/runtime/test_runner/ScopeFunctions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ impl ScopeFunctions {
let [array] = frame.arguments_as_array::<1>();
if array.is_undefined_or_null() || !array.is_array() {
let mut formatter = bun_jsc::ConsoleObject::Formatter::new(global);
formatter.max_output_bytes = crate::test_runner::expect::MAX_MATCHER_OUTPUT_BYTES;
return Err(global.throw(format_args!("Expected array, got {}", array.to_fmt(&mut formatter))));
}

Expand Down Expand Up @@ -169,6 +170,7 @@ fn call_as_function(global: &JSGlobalObject, frame: &CallFrame) -> JsResult<JSVa
if !this.each.is_empty() {
if this.each.is_undefined_or_null() || !this.each.is_array() {
let mut formatter = bun_jsc::ConsoleObject::Formatter::new(global);
formatter.max_output_bytes = crate::test_runner::expect::MAX_MATCHER_OUTPUT_BYTES;
return Err(global.throw(format_args!("Expected array, got {}", this.each.to_fmt(&mut formatter))));
}
let mut iter = this.each.array_iterator(global)?;
Expand Down
4 changes: 3 additions & 1 deletion src/runtime/test_runner/expect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use super::bun_test::{self};
use super::diff_format::DiffFormatter;
use super::execution::ExpectAssertions;
use super::jest::Jest;
use super::expect::{JSValueTestExt, FormatterTestExt, make_formatter};
use super::expect::{JSValueTestExt, FormatterTestExt, make_formatter, MAX_MATCHER_OUTPUT_BYTES};
use crate::expect_throw as throw;

use bun_jsc::js_error_to_write_error;
Expand Down Expand Up @@ -1141,6 +1141,7 @@ impl Expect {
// TODO: print diff with properties from propertyMatchers
let signature = Self::get_signature(fn_name, "<green>propertyMatchers<r>", false);
let mut formatter = ConsoleObject::Formatter::new(global_this);
formatter.max_output_bytes = MAX_MATCHER_OUTPUT_BYTES;
Comment thread
robobun marked this conversation as resolved.
Resolved
return throw!(
self, global_this, signature,
"\n\nExpected <green>propertyMatchers<r> to match properties from received object\n\nReceived: {}\n",
Expand Down Expand Up @@ -1211,6 +1212,7 @@ impl Expect {
}
_ => {
let mut formatter = ConsoleObject::Formatter::new(global_this);
formatter.max_output_bytes = MAX_MATCHER_OUTPUT_BYTES;
global_this.throw(format_args!("Failed to snapshot value: {}", value.to_fmt(&mut formatter)))
}
});
Expand Down
16 changes: 14 additions & 2 deletions src/runtime/test_runner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,12 @@ pub mod expect {
#[derive(Copy, Clone, PartialEq, Eq)]
pub enum BigIntCompare { LessThan, Equal, GreaterThan, Undefined }

/// Byte cap for each value rendered into a matcher failure message.
/// Wide object graphs (e.g. a DOM tree) can expand to gigabytes, and past
/// `WTF::String::MaxLength` the assertion error cannot be created at all,
/// losing the throw (#37310).
Comment thread
robobun marked this conversation as resolved.
Resolved
pub(crate) const MAX_MATCHER_OUTPUT_BYTES: usize = 1024 * 1024;

/// `super::make_formatter(global_this)`
/// is the universal matcher pattern; `Formatter` has no `Default` (it
/// borrows `global_this`), so provide the constructor every matcher
Expand All @@ -284,6 +290,7 @@ pub mod expect {
pub(crate) fn make_formatter(global: &JSGlobalObject) -> Formatter<'_> {
let mut f = Formatter::new(global);
f.quote_strings = true;
f.max_output_bytes = MAX_MATCHER_OUTPUT_BYTES;
Comment thread
robobun marked this conversation as resolved.
Resolved
f
}
Comment thread
robobun marked this conversation as resolved.
Resolved

Expand Down Expand Up @@ -418,13 +425,18 @@ pub mod expect {
/// Builder-style `.with_quote_strings(bool)` shim — `bun_jsc::Formatter`
/// exposes `quote_strings` as a public field, not a chained setter. A
/// handful of matcher modules write
/// `Formatter::new(g).with_quote_strings(true)`.
/// `Formatter::new(g).with_quote_strings(true)`; it is the other spelling
/// of [`make_formatter`], so it also applies the matcher output cap.
Comment thread
robobun marked this conversation as resolved.
Resolved
pub trait FormatterTestExt: Sized {
fn with_quote_strings(self, b: bool) -> Self;
}
impl<'a> FormatterTestExt for Formatter<'a> {
#[inline]
fn with_quote_strings(mut self, b: bool) -> Self { self.quote_strings = b; self }
fn with_quote_strings(mut self, b: bool) -> Self {
self.quote_strings = b;
self.max_output_bytes = MAX_MATCHER_OUTPUT_BYTES;
self
}
}

// ── matcher modules ───────────────────────────────────────────────
Expand Down
88 changes: 88 additions & 0 deletions test/js/bun/test/expect-failure-message-truncation.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { expect, test } from "bun:test";

// https://github.com/oven-sh/bun/issues/37310
// Rendering the received value into a matcher failure message used to be
// unbounded: a wide object graph (e.g. a happy-dom tree) expanded to
// gigabytes, and past JSC's maximum string length the assertion error could
// not be created at all, so the failed assertion returned without throwing.

function tree(depth: number, fanout: number): any {
const node: any = { tag: "div", className: `level-${depth}`, children: [] };
if (depth > 0) {
for (let i = 0; i < fanout; i++) node.children.push(tree(depth - 1, fanout));
}
return node;
}

function messageOf(fn: () => void): string {
let err: Error | undefined;
try {
fn();
} catch (e: any) {
err = e;
}
expect(err).toBeDefined();
return err!.message;
}

test("failure message for a huge object graph is truncated and still throws", () => {
const root = tree(4, 14); // ~41k nodes, renders several MB untruncated
const message = messageOf(() => expect(root).toBeNull());
expect(message.length).toBeLessThan(2 * 1024 * 1024);
expect(message).toContain("[value truncated]");
});

test("failure message for a huge string is truncated and still throws", () => {
// Strings render through a different path than object properties; this
// used to escape the whole string into the message.
const s = Buffer.alloc(2_000_000, "x").toString();
const message = messageOf(() => expect(s).toBeNull());
expect(message.length).toBeLessThan(2 * 1024 * 1024);
expect(message).toContain("[value truncated]");
});

test("failure message for a huge Map is truncated and still throws", () => {
const big = Buffer.alloc(1024, "v").toString();
const m = new Map<string, string>();
for (let i = 0; i < 2_000; i++) m.set("key" + i, big); // ~2MB rendered untruncated
const message = messageOf(() => expect(m).toBeNull());
expect(message.length).toBeLessThan(2 * 1024 * 1024);
expect(message).toContain("[value truncated]");
});

test("toThrow failure message for a huge thrown value is truncated", () => {
// toThrow builds its formatter through a different helper than toBeNull;
// both must apply the cap.
const root = tree(4, 14);
const message = messageOf(() =>
expect(() => {
throw root;
}).toThrow("nope"),
);
expect(message.length).toBeLessThan(4 * 1024 * 1024);
expect(message).toContain("[value truncated]");
});

test("a getter throwing after the cap is hit does not replace the assertion error", () => {
// Element 0 exhausts the byte budget; element 1's tag classification then
// invokes a throwing getter. The pending exception must not surface in
// place of the assertion error.
const bad = {};
Object.defineProperty(bad, "$$typeof", {
get() {
throw new Error("boom");
},
});
const arr = [Buffer.alloc(2_000_000, "x").toString(), bad];
const message = messageOf(() => expect(arr).toBeNull());
expect(message).not.toBe("boom");
// The signature may carry ANSI codes in CI, so match a contiguous token.
expect(message).toContain("toBeNull");
expect(message.length).toBeLessThan(2 * 1024 * 1024);
});

test("failure message for a small value is not truncated", () => {
const message = messageOf(() => expect({ a: 1, b: [2, 3] }).toBeNull());
expect(message).not.toContain("[value truncated]");
expect(message).toContain("a: 1");
});
Loading