postgres: skip 16KB memset in parse_array scratch buffer

[robobun]

Problem

The Zig parseArray (src/sql_jsc/postgres/DataCell.zig:80) declares:

var stack_buffer: [16 * 1024]u8 = undefined;

an uninitialized scratch passed to unescapePostgresString, which writes every returned byte before reading (so undefined is sound). The Rust port declared:

let mut stack_buffer = [0u8; 16 * 1024];

which emits a 16 KiB memset on every parse_array entry, including every recursive call for nested sub-arrays. For a result set with N rows each holding a text[][] of K inner arrays, that is N × (K + 1) × 16 KiB of zero-fill the Zig build never did.

Fix

Change unescape_postgres_string to accept &mut [MaybeUninit<u8>] and return &[u8] over the written prefix; declare the stack buffer as:

let mut stack_buffer = [MaybeUninit::<u8>::uninit(); 16 * 1024];

matching the Zig = undefined semantics. The >16 KiB heap fallback switches from vec![0u8; n] to Box::new_uninit_slice(n). Same pattern as the getdents AlignedBuf in src/sys/lib.rs.

Verification

Behavior is unchanged. Against a local postgres, all of these produce byte-identical output before/after:

• quoted text[] with \", \\\\, \t escapes
• nested text[][] (recursion)
• json[] / jsonb[] (the other stack_buffer branch)
• 1000-byte element (stack path) and 20000-byte element (heap fallback path)

cargo check -p bun_sql_jsc and clippy are clean.

[robobun]

This is a perf-only change: [0u8; 16*1024] → [MaybeUninit::<u8>::uninit(); 16*1024] removes a 16 KiB memset with zero behavioral difference (the Zig reference at DataCell.zig:80 uses = undefined; unescape_postgres_string writes every returned byte before reading it).

There is no test that can fail on main and pass here, because output is byte-identical. Verified locally against postgres that quoted text[] with escapes, nested text[][], json[], a 1000-byte stack-path element, and a 20000-byte heap-fallback element all produce identical results on main vs this branch. The two added tests in sql.test.ts cover the >16 KiB heap fallback and nested recursion paths that were previously untested.

CI: the diff is green on every lane that ran sql.test.ts (including debian-13-x64-asan). The red on builds 62409 and 62505 is three macOS test-bun jobs that Expired waiting for an agent, plus unrelated flaky retries in update_interactive_install.test.ts (Windows), serve-body-leak.test.ts, and spawn-pipe-leak.test.ts. None touch postgres or this diff.

Ready for a maintainer to review/merge.

[coderabbitai]

Warning

Review limit reached

@robobun, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 3 minutes and 9 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d993a476-40ed-4d74-a20d-4e13c996bd7c

📥 Commits

Reviewing files that changed from the base of the PR and between e0acad3 and 24c7a9c.

📒 Files selected for processing (2)

• src/sql_jsc/postgres/DataCell.rs
• test/js/sql/sql.test.ts

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[robobun]

^{Updated 9:31 AM PT - Jun 15th, 2026}

❌ @robobun, your commit 24c7a9c has 4 failures in Build #62505 (All Failures):

• test/cli/install/bun-add.test.ts - code 1 on 🍎 26 aarch64
• test/js/bun/io/fetch/fetch-abort-slow-connect.test.ts - code 1 on 🍎 14 aarch64
• test/js/node/http/node-http-backpressure-max.test.ts - code 1 on 🍎 14 x64
• test/js/bun/terminal/terminal.test.ts - code 1 on 🍎 14 x64

---

🧪   To try this PR locally:

bunx bun-pr 32281

That installs a local version of the PR into your bun-32281 executable, so you can run:

bun-32281 --bun

[claude]

The MaybeUninit/unsafe reasoning looks sound to me and the refactored escape loop is behavior-identical, but since this adds a new unsafe block on a hot path that parses server-supplied bytes, I'd like a human to double-check the soundness argument before merge.

Extended reasoning...

Overview

This PR eliminates a 16 KiB memset on every parse_array call (and every recursive sub-array) in the postgres text-array parser by switching the scratch buffer from [0u8; 16*1024] to [MaybeUninit<u8>::uninit(); 16*1024]. unescape_postgres_string is updated to take &mut [MaybeUninit<u8>], write each output byte via .write(), and return a &[u8] over the initialized prefix via an unsafe from_raw_parts. The two heap-fallback paths switch from vec![0u8; n] to Box::new_uninit_slice(n). Two new integration tests in test/js/sql/sql.test.ts exercise the >16 KiB heap fallback and nested text[][] recursion with quoted/escaped elements.

Security risks

The buffer holds postgres wire-format bytes (server-supplied). The new unsafe block's invariant — every buffer[0..out_index] element is initialized because out_index only increments immediately after a .write(), and out_index <= buffer.len() via the top-of-loop check — appears correct, and the returned slice is immediately copied via BunString::clone_utf8 so there's no lifetime hazard. I don't see a way to read uninitialized memory or write OOB. Still, this is new unsafe on attacker-influenceable input, which is exactly the kind of thing a second pair of eyes should confirm.

Level of scrutiny

Medium-high. It's a perf-only change with no intended behavior delta, but it (a) introduces unsafe, (b) restructures the escape-sequence match from per-arm buffer[out_index] = ... assignments into a single byte expression then one .write() (I traced each arm including the \x hex path and the i += 2 ordering — equivalent), and (c) changes the return type from &mut [u8] to &[u8] (callers only read, so fine). None of these are mechanical.

Other factors

The bug hunter found nothing. CI shows build-cpp failures on musl targets for the first commit, which look unrelated to this Rust-only change (and the linker -no-pie lines are warnings, not errors), but the status hasn't been updated for the test-adding commit. Given the unsafe addition and the fact that this is an optimization rather than a fix, I'm deferring rather than approving.

[claude]

The safety argument looks sound and the refactor is clean, but this introduces a new unsafe block reinterpreting MaybeUninit memory in a parser handling network data — worth a quick human glance to confirm the invariant.

Extended reasoning...

Overview

This PR changes the 16 KiB scratch buffer in parse_array (src/sql_jsc/postgres/DataCell.rs) from zero-initialized [0u8; 16*1024] to [MaybeUninit::<u8>::uninit(); 16*1024], eliminating a per-call (and per-recursion) memset. unescape_postgres_string is reworked to take &mut [MaybeUninit<u8>], write each output byte via .write(), and return the initialized prefix via unsafe { slice::from_raw_parts(...) }. The >16 KiB heap fallback switches from vec![0u8; n] to Box::new_uninit_slice(n). Two integration tests are added covering the >16 KiB heap path with quoted elements and nested text[][] recursion.

Security risks

The new unsafe block constructs a &[u8] over a MaybeUninit buffer. The invariant — every element in [0..out_index] is written exactly once before out_index is incremented, and out_index <= buffer.len() via the top-of-loop bounds check — appears correct on inspection, including across all early-return error paths (which never increment out_index). The lifetime 'a ties the returned slice to the buffer borrow, so no use-after-scope. If the invariant were ever violated by a future edit, the consequence would be reading uninitialized stack/heap bytes into a JS string from postgres wire data, which is why I'd like a human to confirm.

Level of scrutiny

Medium. The change is small, mechanical, well-commented, and mirrors both the original Zig (= undefined) and an existing in-tree pattern (getdents AlignedBuf). But it adds unsafe to a hot path that parses untrusted-ish network input and whose correctness now depends on a loop invariant rather than the type system. That's the textbook case for a second reviewer.

Other factors

No CODEOWNERS cover this path. The bug-hunting system found nothing. The match-arm refactor (compute byte → single .write()) is behavior-identical to the old per-arm assignment. Return type narrowed from &mut [u8] to &[u8], which is fine since both call sites only pass the result to BunString::clone_utf8. New tests look correct and exercise both the stack and heap-fallback branches.

[robobun]

Stale PR review: keep open, rework.

The goal is still wanted. Main still zero-fills the scratch at src/sql_jsc/postgres/DataCell.rs:173 ([0u8; 16 * 1024], once per parse_array call and per nesting level) and the heap fallbacks at :256 and :279 (vec![0u8; len]). #39417 (merged 2026-08-17) removed the other zero-fills from that audit and left this site to this PR by name, and the review on #41442 (2026-09-06) says the same thing about 16 KB buffers in general.

The current diff is not the shape that lands. It conflicts with main, it gives no before/after numbers (.claude/docs/landing-prs.md:34), and it adds a &mut [MaybeUninit<u8>] signature plus unsafe { from_raw_parts } while keeping a 16 KiB stack array per recursion frame. The review on the sibling #32350 asked for "No unsafe. Make it idiomatic Rust.", and landing-prs.md:32 says multi-KB scratch comes from a shared pool or a reused buffer, never tens-of-KB stack frames.

Wanted shape: rebase over the crate::Error migration, delete stack_buffer and both needs_dynamic_buffer blocks, and thread one reusable Vec<u8> scratch through parse_array and its recursive calls (created once in from_bytes or held on the putter). unescape_postgres_string can take &mut Vec<u8>, clear() and reserve(input.len()), then push decoded bytes, since the output is never longer than the input. That removes the memset, the zeroed heap fallback, and the per-depth stack growth with no unsafe. Put array-select numbers (for example 100k rows of text[], int8[], text[][], plus a scalar control) in the body, and drop the tests that already pass on main.

[robobun]

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-06-15, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.