From 2b94d7445eb7ae80a51a775be0c0d3f62e16e45a Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Thu, 11 Jun 2026 15:03:03 -0700 Subject: [PATCH 1/2] sql: consolidate per-driver StackReader into shared/StackReader.rs The MySQL and Postgres StackReader types were near-identical cursor-over-buffer wrappers. This moves the struct + read methods to shared/StackReader.rs with two small traits (ShortRead, WrapReader) so each driver supplies its own error variant and NewReader wrapper. The per-driver StackReader.rs files become trait impls + a re-export. Also drops the dead NewReaderOf type alias and decode_allocator (which was identical to decode) from mysql NewReader.rs. --- src/sql/lib.rs | 5 +- src/sql/mysql/protocol/NewReader.rs | 9 -- src/sql/mysql/protocol/StackReader.rs | 106 +++------------------ src/sql/postgres/protocol/StackReader.rs | 94 ++++-------------- src/sql/shared/StackReader.rs | 116 +++++++++++++++++++++++ 5 files changed, 151 insertions(+), 179 deletions(-) create mode 100644 src/sql/shared/StackReader.rs diff --git a/src/sql/lib.rs b/src/sql/lib.rs index a43503328f8f..5666182df0bf 100644 --- a/src/sql/lib.rs +++ b/src/sql/lib.rs @@ -11,6 +11,8 @@ pub mod shared { pub mod query_status; #[path = "SQLQueryResultMode.rs"] pub mod sql_query_result_mode; + #[path = "StackReader.rs"] + pub mod stack_reader; #[path = "StatementStatus.rs"] pub mod statement_status; @@ -18,6 +20,7 @@ pub mod shared { pub use connection_flags::ConnectionFlags; pub use data::Data; pub use sql_query_result_mode::SQLQueryResultMode; + pub use stack_reader::StackReader; } pub mod mysql { @@ -107,7 +110,7 @@ pub mod mysql { pub use handshake_response41::HandshakeResponse41; pub use handshake_v10::HandshakeV10; pub use local_infile_request::LocalInfileRequest; - pub use new_reader::{Decode, NewReader, NewReaderOf, ReadableInt, ReaderContext}; + pub use new_reader::{Decode, NewReader, ReadableInt, ReaderContext}; pub use new_writer::{NewWriter, NewWriterWrap, Packet, WriterContext, write_wrap}; pub use ok_packet::OKPacket; pub use packet_header::PacketHeader; diff --git a/src/sql/mysql/protocol/NewReader.rs b/src/sql/mysql/protocol/NewReader.rs index f4790d47c6af..81e5c1d96e95 100644 --- a/src/sql/mysql/protocol/NewReader.rs +++ b/src/sql/mysql/protocol/NewReader.rs @@ -122,8 +122,6 @@ impl NewReader { /// MySQL's u24/i24 are NOT routed through this trait — see `int_u24`/`int_i24`. pub use bun_core::NativeEndianInt as ReadableInt; -pub type NewReaderOf = NewReader; - impl From for NewReader { fn from(wrapped: C) -> Self { Self { wrapped } @@ -142,11 +140,4 @@ pub trait Decode: Sized { ) -> Result<(), AnyMySQLError> { self.decode_internal(context.into()) } - - fn decode_allocator( - &mut self, - context: impl Into>, - ) -> Result<(), AnyMySQLError> { - self.decode_internal(context.into()) - } } diff --git a/src/sql/mysql/protocol/StackReader.rs b/src/sql/mysql/protocol/StackReader.rs index d996366948db..57415004c5d2 100644 --- a/src/sql/mysql/protocol/StackReader.rs +++ b/src/sql/mysql/protocol/StackReader.rs @@ -1,116 +1,40 @@ -use core::cell::Cell; - -use bun_core::strings; - use super::any_mysql_error::Error as AnyMySQLError; use super::new_reader::{NewReader, ReaderContext}; use crate::shared::data::Data; +use crate::shared::stack_reader::{ShortRead, WrapReader}; -#[derive(Clone, Copy)] -pub struct StackReader<'a> { - pub buffer: &'a [u8], - pub offset: &'a Cell, - pub message_start: &'a Cell, -} - -impl<'a> StackReader<'a> { - pub fn mark_message_start(&self) { - self.message_start.set(self.offset.get()); - } - - pub fn set_offset_from_start(&self, offset: usize) { - self.offset.set(self.message_start.get() + offset); - } - - pub fn ensure_capacity(&self, length: usize) -> bool { - self.offset - .get() - .checked_add(length) - .is_some_and(|end| self.buffer.len() >= end) - } - - pub fn init( - buffer: &'a [u8], - offset: &'a Cell, - message_start: &'a Cell, - ) -> NewReader> { - NewReader { - wrapped: StackReader { - buffer, - offset, - message_start, - }, - } - } +pub use crate::shared::stack_reader::StackReader; - pub fn peek(&self) -> &'a [u8] { - &self.buffer[self.offset.get()..] - } - - pub fn skip(&self, count: isize) { - if count < 0 { - let abs_count = count.unsigned_abs(); - if abs_count > self.offset.get() { - self.offset.set(0); - return; - } - self.offset.set(self.offset.get() - abs_count); - return; - } - - let ucount: usize = usize::try_from(count).expect("int cast"); - if self.offset.get() + ucount > self.buffer.len() { - self.offset.set(self.buffer.len()); - return; - } - - self.offset.set(self.offset.get() + ucount); - } - - pub fn read(&self, count: usize) -> Result { - let offset = self.offset.get(); - if !self.ensure_capacity(count) { - return Err(AnyMySQLError::ShortRead); - } - - self.skip(isize::try_from(count).expect("int cast")); - Ok(Data::Temporary(bun_ptr::RawSlice::new( - &self.buffer[offset..self.offset.get()], - ))) - } - - pub fn read_z(&self) -> Result { - let remaining = self.peek(); - if let Some(zero) = strings::index_of_char(remaining, 0) { - let zero = zero as usize; - self.skip(isize::try_from(zero + 1).expect("int cast")); - return Ok(Data::Temporary(bun_ptr::RawSlice::new(&remaining[0..zero]))); - } +impl ShortRead for AnyMySQLError { + const SHORT_READ: Self = AnyMySQLError::ShortRead; +} - Err(AnyMySQLError::ShortRead) +impl<'a> WrapReader<'a> for NewReader> { + fn wrap(reader: StackReader<'a>) -> Self { + NewReader { wrapped: reader } } } impl<'a> ReaderContext for StackReader<'a> { fn mark_message_start(self) { - Self::mark_message_start(&self) + StackReader::mark_message_start(&self) } fn peek(&self) -> &[u8] { - Self::peek(self) + StackReader::peek(self) } fn skip(self, count: isize) { - Self::skip(&self, count) + StackReader::skip(&self, count) } fn ensure_capacity(self, count: usize) -> bool { - Self::ensure_capacity(&self, count) + StackReader::ensure_capacity(&self, count) } fn read(self, count: usize) -> Result { - Self::read(&self, count) + StackReader::read(&self, count) } fn read_z(self) -> Result { - Self::read_z(&self) + StackReader::read_z(&self) } fn set_offset_from_start(self, offset: usize) { - Self::set_offset_from_start(&self, offset) + StackReader::set_offset_from_start(&self, offset) } } diff --git a/src/sql/postgres/protocol/StackReader.rs b/src/sql/postgres/protocol/StackReader.rs index 411f746af09e..9babc82d3e35 100644 --- a/src/sql/postgres/protocol/StackReader.rs +++ b/src/sql/postgres/protocol/StackReader.rs @@ -1,101 +1,39 @@ use crate::postgres::any_postgres_error::AnyPostgresError; use crate::postgres::protocol::new_reader::{NewReader, ReaderContext}; use crate::shared::data::Data; -use bun_core::strings; +use crate::shared::stack_reader::{ShortRead, WrapReader}; -pub struct StackReader<'a> { - pub buffer: &'a [u8], - pub offset: &'a mut usize, - pub message_start: &'a mut usize, -} - -impl<'a> StackReader<'a> { - pub fn mark_message_start(&mut self) { - *self.message_start = *self.offset; - } - - pub fn ensure_length(&self, length: usize) -> bool { - self.buffer.len() >= (*self.offset + length) - } - - pub fn init( - buffer: &'a [u8], - offset: &'a mut usize, - message_start: &'a mut usize, - ) -> NewReader> { - NewReader { - wrapped: StackReader { - buffer, - offset, - message_start, - }, - } - } - - pub fn peek(&self) -> &[u8] { - &self.buffer[*self.offset..] - } +pub use crate::shared::stack_reader::StackReader; - pub fn skip(&mut self, count: usize) { - if *self.offset + count > self.buffer.len() { - *self.offset = self.buffer.len(); - return; - } - - *self.offset += count; - } - - pub fn ensure_capacity(&self, count: usize) -> bool { - self.buffer.len() >= (*self.offset + count) - } - - pub fn read(&mut self, count: usize) -> Result { - let offset = *self.offset; - if !self.ensure_capacity(count) { - return Err(AnyPostgresError::ShortRead); - } - - self.skip(count); - // Copy the &'a [u8] out before slicing so the returned Data borrows 'a, - // not &mut self. - let buffer: &'a [u8] = self.buffer; - Ok(Data::Temporary(bun_ptr::RawSlice::new( - &buffer[offset..*self.offset], - ))) - } - - pub fn read_z(&mut self) -> Result { - // Inline `peek()` so `remaining` borrows 'a (via the Copy &'a [u8]) - // instead of &self, allowing `self.skip()` below. - let buffer: &'a [u8] = self.buffer; - let remaining = &buffer[*self.offset..]; - if let Some(zero) = strings::index_of_char(remaining, 0) { - let zero = zero as usize; - self.skip(zero + 1); - return Ok(Data::Temporary(bun_ptr::RawSlice::new(&remaining[0..zero]))); - } +impl ShortRead for AnyPostgresError { + const SHORT_READ: Self = AnyPostgresError::ShortRead; +} - Err(AnyPostgresError::ShortRead) +impl<'a> WrapReader<'a> for NewReader> { + fn wrap(reader: StackReader<'a>) -> Self { + NewReader { wrapped: reader } } } impl<'a> ReaderContext for StackReader<'a> { fn mark_message_start(&mut self) { - Self::mark_message_start(self) + StackReader::mark_message_start(self) } fn peek(&self) -> &[u8] { - Self::peek(self) + StackReader::peek(self) } fn skip(&mut self, count: usize) { - Self::skip(self, count) + // The shared reader's signed skip clamps to the buffer end, matching + // the old unsigned behavior even when `count` exceeds `isize::MAX`. + StackReader::skip(self, isize::try_from(count).unwrap_or(isize::MAX)) } fn ensure_length(&mut self, count: usize) -> bool { - Self::ensure_length(self, count) + StackReader::ensure_capacity(self, count) } fn read(&mut self, count: usize) -> Result { - Self::read(self, count) + StackReader::read(self, count) } fn read_z(&mut self) -> Result { - Self::read_z(self) + StackReader::read_z(self) } } diff --git a/src/sql/shared/StackReader.rs b/src/sql/shared/StackReader.rs new file mode 100644 index 000000000000..1919622b9bc1 --- /dev/null +++ b/src/sql/shared/StackReader.rs @@ -0,0 +1,116 @@ +use core::cell::Cell; + +use bun_core::strings; + +use super::data::Data; + +/// Supplies the protocol error enum's "buffer exhausted" variant for +/// [`StackReader`]'s fallible reads. +pub trait ShortRead { + const SHORT_READ: Self; +} + +/// Wraps a [`StackReader`] in the protocol's reader type ([`StackReader::init`]). +pub trait WrapReader<'a>: Sized { + fn wrap(reader: StackReader<'a>) -> Self; +} + +/// Accepts either `&Cell` or `&mut usize` as a cursor slot in +/// [`StackReader::init`]. +pub trait IntoCursor<'a> { + fn into_cursor(self) -> &'a Cell; +} + +impl<'a> IntoCursor<'a> for &'a Cell { + fn into_cursor(self) -> &'a Cell { + self + } +} + +impl<'a> IntoCursor<'a> for &'a mut usize { + fn into_cursor(self) -> &'a Cell { + Cell::from_mut(self) + } +} + +/// Cursor over a borrowed wire buffer. `Cell`-based so copies share the +/// offset and callers can read the cursor back after a short read. +#[derive(Clone, Copy)] +pub struct StackReader<'a> { + pub buffer: &'a [u8], + pub offset: &'a Cell, + pub message_start: &'a Cell, +} + +impl<'a> StackReader<'a> { + pub fn init>( + buffer: &'a [u8], + offset: impl IntoCursor<'a>, + message_start: impl IntoCursor<'a>, + ) -> R { + R::wrap(StackReader { + buffer, + offset: offset.into_cursor(), + message_start: message_start.into_cursor(), + }) + } + + pub fn mark_message_start(&self) { + self.message_start.set(self.offset.get()); + } + + pub fn set_offset_from_start(&self, offset: usize) { + self.offset.set(self.message_start.get() + offset); + } + + pub fn ensure_capacity(&self, length: usize) -> bool { + self.offset + .get() + .checked_add(length) + .is_some_and(|end| self.buffer.len() >= end) + } + + pub fn peek(&self) -> &'a [u8] { + &self.buffer[self.offset.get()..] + } + + /// Clamps to `[0, buffer.len()]` in both directions. + pub fn skip(&self, count: isize) { + let offset = self.offset.get(); + if count < 0 { + self.offset.set(offset.saturating_sub(count.unsigned_abs())); + return; + } + + let ucount = count.unsigned_abs(); + if offset + ucount > self.buffer.len() { + self.offset.set(self.buffer.len()); + return; + } + + self.offset.set(offset + ucount); + } + + pub fn read(&self, count: usize) -> Result { + let offset = self.offset.get(); + if !self.ensure_capacity(count) { + return Err(E::SHORT_READ); + } + + self.offset.set(offset + count); + Ok(Data::Temporary(bun_ptr::RawSlice::new( + &self.buffer[offset..offset + count], + ))) + } + + pub fn read_z(&self) -> Result { + let remaining = self.peek(); + if let Some(zero) = strings::index_of_char(remaining, 0) { + let zero = zero as usize; + self.skip(isize::try_from(zero + 1).expect("int cast")); + return Ok(Data::Temporary(bun_ptr::RawSlice::new(&remaining[0..zero]))); + } + + Err(E::SHORT_READ) + } +} From 5e04e035848c1e232f9c6bf8b59d1af36a3785a2 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Thu, 11 Jun 2026 15:06:18 -0700 Subject: [PATCH 2/2] sql: drop stale non-Copy claim from postgres NewReader blanket-impl comment --- src/sql/postgres/protocol/NewReader.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/sql/postgres/protocol/NewReader.rs b/src/sql/postgres/protocol/NewReader.rs index 60769fa0f2b4..f58855aab3a2 100644 --- a/src/sql/postgres/protocol/NewReader.rs +++ b/src/sql/postgres/protocol/NewReader.rs @@ -36,8 +36,7 @@ macro_rules! impl_protocol_int { } impl_protocol_int!(u8, i8, u16, i16, u32, i32, u64, i64); -// Blanket impl so `NewReaderWrap<&mut C>` works — the inner `Context` is -// non-`Copy` (holds `&mut usize`), so callers reborrow instead. +// Blanket impl so `NewReaderWrap<&mut C>` works for callers that reborrow. impl ReaderContext for &mut C { #[inline] fn mark_message_start(&mut self) {