From b7d2e27013f3bd40486f903d01063cda7d3bd9e3 Mon Sep 17 00:00:00 2001 From: robobun Date: Mon, 20 Jul 2026 08:19:27 +0000 Subject: [PATCH 1/2] sql: drop redundant clamps and a zero-fill of a zero-initialized buffer All four are porting artifacts from the Zig rewrite (#30412) where the original code initialized with 'undefined' or had looser slice contracts. In Rust they are provably dead: - PreparedStatement.rs: [0u8; N] is already zero-initialized, so .fill(0) on a sub-slice is a no-op. NewWriter::write_null_bitmap already omits it. - HandshakeV10.rs: after flooring auth_plugin_data_len to 21, subtracting 8 gives at least 13, so .max(13) is a no-op. Folded into one .max(21). - CommandTag.rs: strings::index_of_char returns an index strictly within the slice (highway asserts haystack[result] == needle), so idx+1 <= len and .min(len) on idx+1 is a no-op. - NewReader.rs (postgres): StackReader::read(count) returns exactly count bytes on Ok (src/sql/shared/StackReader.rs:94), so the length re-check is unreachable. from_be_slice already bounds-checks via [..SIZE]. --- src/sql/mysql/protocol/HandshakeV10.rs | 7 ++----- src/sql/mysql/protocol/PreparedStatement.rs | 1 - src/sql/postgres/CommandTag.rs | 6 +++--- src/sql/postgres/protocol/NewReader.rs | 6 +----- 4 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/sql/mysql/protocol/HandshakeV10.rs b/src/sql/mysql/protocol/HandshakeV10.rs index d0e9030a725b..d6fbb92ca8a4 100644 --- a/src/sql/mysql/protocol/HandshakeV10.rs +++ b/src/sql/mysql/protocol/HandshakeV10.rs @@ -76,16 +76,13 @@ impl HandshakeV10 { ); // Length of auth plugin data - let mut auth_plugin_data_len = reader.int::()?; - if auth_plugin_data_len < 21 { - auth_plugin_data_len = 21; - } + let auth_plugin_data_len = reader.int::()?.max(21); // Skip reserved bytes reader.skip(10); // Auth plugin data part 2 - let remaining_auth_len = (auth_plugin_data_len - 8).max(13); + let remaining_auth_len = auth_plugin_data_len - 8; let auth_data_2 = reader.read(remaining_auth_len as usize)?; self.auth_plugin_data_part_2 = Box::<[u8]>::from(auth_data_2.slice()); diff --git a/src/sql/mysql/protocol/PreparedStatement.rs b/src/sql/mysql/protocol/PreparedStatement.rs index 5c947b264a9e..d8db349deee4 100644 --- a/src/sql/mysql/protocol/PreparedStatement.rs +++ b/src/sql/mysql/protocol/PreparedStatement.rs @@ -53,7 +53,6 @@ impl<'a> Execute<'a> { let mut null_bitmap_buf = [0u8; MYSQL_MAX_PARAMS]; let bitmap_bytes = self.params.len.div_ceil(8); let null_bitmap = &mut null_bitmap_buf[0..bitmap_bytes]; - null_bitmap.fill(0); for i in 0..self.params.len { if (self.params.is_null)(self.params.ctx, i) { diff --git a/src/sql/postgres/CommandTag.rs b/src/sql/postgres/CommandTag.rs index 9fb4fcacd150..dcc70e5e67d0 100644 --- a/src/sql/postgres/CommandTag.rs +++ b/src/sql/postgres/CommandTag.rs @@ -82,12 +82,12 @@ impl<'a> CommandTag<'a> { let number: u64 = 'brk: { match cmd { KnownCommand::Insert => { - let mut remaining = &tag[(first_space_index + 1).min(tag.len())..]; + let mut remaining = &tag[first_space_index + 1..]; let Some(second_space) = strings::index_of_char(remaining, b' ') else { return CommandTag::Other(tag); }; let second_space = second_space as usize; - remaining = &remaining[(second_space + 1).min(remaining.len())..]; + remaining = &remaining[second_space + 1..]; // Postgres wire is pure base-10 ASCII so radix-0/`_`/sign // widening is unreachable. match bun_core::fmt::parse_int::(remaining, 0) { @@ -103,7 +103,7 @@ impl<'a> CommandTag<'a> { } } _ => { - let after_tag = &tag[(first_space_index + 1).min(tag.len())..]; + let after_tag = &tag[first_space_index + 1..]; match bun_core::fmt::parse_int::(after_tag, 0) { Ok(n) => break 'brk n, Err(err) => { diff --git a/src/sql/postgres/protocol/NewReader.rs b/src/sql/postgres/protocol/NewReader.rs index 0167ea77a39b..c1af503038f5 100644 --- a/src/sql/postgres/protocol/NewReader.rs +++ b/src/sql/postgres/protocol/NewReader.rs @@ -126,11 +126,7 @@ impl NewReaderWrap { pub fn int(&mut self) -> Result { let data = self.read(Int::SIZE)?; - let slice = data.slice(); - if slice.len() < Int::SIZE { - return Err(AnyPostgresError::ShortRead); - } - Ok(Int::from_be_slice(&slice[0..Int::SIZE])) + Ok(Int::from_be_slice(data.slice())) } pub fn peek_int(&self) -> Option { From bd890278ae552b350478656cef9b60d98c1114bc Mon Sep 17 00:00:00 2001 From: robobun Date: Mon, 20 Jul 2026 08:44:31 +0000 Subject: [PATCH 2/2] sql(postgres): document the exact-count-or-ShortRead contract on ReaderContext::read int() relies on it; state it on the trait method so future implementors (there are two today, StackReader and the sql_jsc live-socket Reader) do not silently violate it. --- src/sql/postgres/protocol/NewReader.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/sql/postgres/protocol/NewReader.rs b/src/sql/postgres/protocol/NewReader.rs index c1af503038f5..4eb5d51a4cbb 100644 --- a/src/sql/postgres/protocol/NewReader.rs +++ b/src/sql/postgres/protocol/NewReader.rs @@ -9,6 +9,9 @@ pub trait ReaderContext { fn peek(&self) -> &[u8]; fn skip(&mut self, count: usize); fn ensure_length(&mut self, count: usize) -> bool; + /// On `Ok`, the returned slice is exactly `count` bytes; otherwise + /// `Err(ShortRead)`. Callers rely on this: `int()` passes the + /// result straight to `from_be_slice` without re-checking length. fn read(&mut self, count: usize) -> Result; fn read_z(&mut self) -> Result; }