diff --git a/docs/cpp.md b/docs/cpp.md index 776e11d..cb1fd60 100644 --- a/docs/cpp.md +++ b/docs/cpp.md @@ -34,6 +34,8 @@ Callbacks are dispatched only when the corresponding `CALLBACK_ACTIVE_*` flag is Callbacks are disabled by default. +Callback delivery uses an immutable snapshot of the current event batch, callback table, and activation mask. While that batch is being replayed, Milo functions that mutate the same parser are ignored and nested `milo_parse()` calls return `0`. Calling `milo_destroy()` is deferred until replay ends and stops delivery of the remaining callbacks in that batch. Direct field writes during callbacks remain unsupported. + ## Constants The file `milo.h` defines several constants (`*` is used to denote a family prefix): diff --git a/docs/js.md b/docs/js.md index d43df97..244ce71 100644 --- a/docs/js.md +++ b/docs/js.md @@ -22,6 +22,8 @@ Callbacks are dispatched only when enabled with `setActiveCallbacks`. Callbacks are disabled by default. +Callback delivery uses an immutable snapshot of the current event batch and activation mask. While that batch is being replayed, operations that mutate the same parser are ignored and nested `parse()` calls return `0`. Calling `destroy()` is deferred until replay ends and stops delivery of the remaining callbacks in that batch. JavaScript callback exceptions are captured and rethrown only after Rust has cleaned up replay state. + ### Constants The module exports several constants (`*` is used to denote a family prefix): diff --git a/docs/rust.md b/docs/rust.md index 1ad19ae..2b8c830 100644 --- a/docs/rust.md +++ b/docs/rust.md @@ -20,6 +20,8 @@ Callbacks are dispatched only when the corresponding `CALLBACK_ACTIVE_*` flag is Callbacks are disabled by default. +Callback delivery uses an immutable snapshot of the current event batch, callback table, and activation mask. While that batch is being replayed, methods that mutate the same parser are ignored and nested `parse()` calls return `0`. FFI destruction is deferred until replay ends and stops delivery of the remaining callbacks in that batch. + ## Constants The crate exports several constants (`*` is used to denote a family prefix): diff --git a/macros/src/native.rs b/macros/src/native.rs index a10f57c..df820d1 100644 --- a/macros/src/native.rs +++ b/macros/src/native.rs @@ -18,9 +18,9 @@ pub fn generate_callbacks(callbacks: &[String]) -> TokenStream { if callback_name == "on_error" { quote! { #event_const => { - let at = unsafe { core::ptr::read_unaligned(self.events.add(cursor + 1) as *const u32) }.to_le() as usize; - if self.active_callbacks & #active_const != 0 { - (self.callbacks.#callback)(self, at, 0); + let at = unsafe { core::ptr::read_unaligned(events.as_ptr().add(cursor + 1) as *const u32) }.to_le() as usize; + if active_callbacks & #active_const != 0 { + (callbacks.#callback)(self, at, 0); } cursor += EVENT_ERROR_SIZE; } @@ -28,9 +28,9 @@ pub fn generate_callbacks(callbacks: &[String]) -> TokenStream { } else if callback_name == "on_headers" { quote! { #event_const => { - let at = unsafe { core::ptr::read_unaligned(self.events.add(cursor + 1) as *const u32) }.to_le() as usize; - if self.active_callbacks & #active_const != 0 { - (self.callbacks.#callback)(self, at, 0); + let at = unsafe { core::ptr::read_unaligned(events.as_ptr().add(cursor + 1) as *const u32) }.to_le() as usize; + if active_callbacks & #active_const != 0 { + (callbacks.#callback)(self, at, 0); } cursor += EVENT_METADATA_SIZE; } @@ -38,10 +38,10 @@ pub fn generate_callbacks(callbacks: &[String]) -> TokenStream { } else { quote! { #event_const => { - let at = unsafe { core::ptr::read_unaligned(self.events.add(cursor + 1) as *const u32) }.to_le() as usize; - let len = unsafe { core::ptr::read_unaligned(self.events.add(cursor + 5) as *const u32) }.to_le() as usize; - if self.active_callbacks & #active_const != 0 { - (self.callbacks.#callback)(self, at, len); + let at = unsafe { core::ptr::read_unaligned(events.as_ptr().add(cursor + 1) as *const u32) }.to_le() as usize; + let len = unsafe { core::ptr::read_unaligned(events.as_ptr().add(cursor + 5) as *const u32) }.to_le() as usize; + if active_callbacks & #active_const != 0 { + (callbacks.#callback)(self, at, len); } cursor += EVENT_RANGE_SIZE; } @@ -72,17 +72,36 @@ pub fn generate_callbacks(callbacks: &[String]) -> TokenStream { #[cfg(not(target_family = "wasm"))] impl Parser { #[inline] - fn invoke_callbacks(&mut self) { + fn invoke_callbacks(&mut self, event_len: usize) { + if self.callback_replay || event_len == 0 || event_len > EVENTS_BUFFER_SIZE { + return; + } + + let events = unsafe { core::slice::from_raw_parts(self.events, event_len) }.to_vec(); + let active_callbacks = self.active_callbacks; + let callbacks = self.callbacks.clone(); + let replay_flag = core::ptr::addr_of_mut!(self.callback_replay); + unsafe { core::ptr::write_volatile(replay_flag, true); } + + struct ReplayGuard(*mut bool); + impl Drop for ReplayGuard { + fn drop(&mut self) { unsafe { core::ptr::write_volatile(self.0, false); } } + } + let _guard = ReplayGuard(replay_flag); let mut cursor = 0usize; - loop { - let event_type = unsafe { *self.events.add(cursor) }; + while cursor < events.len() { + let event_type = events[cursor]; match event_type { EVENT_END => break, #(#replay_arms)* _ => break, } + + if unsafe { core::ptr::read_volatile(core::ptr::addr_of!(self.destroy_pending)) } { + break; + } } } } diff --git a/macros/src/parser_fields.rs b/macros/src/parser_fields.rs index c547583..9940b1f 100644 --- a/macros/src/parser_fields.rs +++ b/macros/src/parser_fields.rs @@ -46,6 +46,8 @@ struct ParserStub { unconsumed: WasmPointer, unconsumed_len: WasmUsize, error_description_len: u8, + callback_replay: bool, + destroy_pending: bool, events: WasmPointer, } diff --git a/macros/src/wasm.rs b/macros/src/wasm.rs index 8b8cc3e..a1902c9 100644 --- a/macros/src/wasm.rs +++ b/macros/src/wasm.rs @@ -24,10 +24,10 @@ pub fn generate_callbacks(callbacks: &[String]) -> TokenStream { quote! { #event_const => { - let at = unsafe { core::ptr::read_unaligned(self.events.add(cursor + 1) as *const u32) }.to_le() as usize; - let len = unsafe { core::ptr::read_unaligned(self.events.add(cursor + 5) as *const u32) }.to_le() as usize; - if self.active_callbacks & #active_const != 0 { - unsafe { #callback(self.ptr, at, len); } + let at = unsafe { core::ptr::read_unaligned(events.as_ptr().add(cursor + 1) as *const u32) }.to_le() as usize; + let len = unsafe { core::ptr::read_unaligned(events.as_ptr().add(cursor + 5) as *const u32) }.to_le() as usize; + if active_callbacks & #active_const != 0 { + unsafe { #callback(parser_ptr, at, len); } } cursor += EVENT_RANGE_SIZE; } @@ -65,34 +65,49 @@ pub fn generate_callbacks(callbacks: &[String]) -> TokenStream { #[cfg(target_family = "wasm")] impl Parser { #[inline] - fn invoke_callbacks(&mut self) { + fn invoke_callbacks(&mut self, event_len: usize) { + if self.callback_replay || event_len == 0 || event_len > EVENTS_BUFFER_SIZE { + return; + } + + let events = unsafe { core::slice::from_raw_parts(self.events, event_len) }.to_vec(); + let active_callbacks = self.active_callbacks; + let parser_ptr = self as *mut Parser as *mut c_void; + let replay_flag = core::ptr::addr_of_mut!(self.callback_replay); + unsafe { core::ptr::write_volatile(replay_flag, true); } + + struct ReplayGuard(*mut bool); + impl Drop for ReplayGuard { + fn drop(&mut self) { unsafe { core::ptr::write_volatile(self.0, false); } } + } + let _guard = ReplayGuard(replay_flag); let mut cursor = 0usize; - loop { - let event_type = unsafe { *self.events.add(cursor) }; + while cursor < events.len() { + let event_type = events[cursor]; match event_type { EVENT_END => break, EVENT_ERROR => { - let at = unsafe { core::ptr::read_unaligned(self.events.add(cursor + 1) as *const u32) }.to_le() as usize; - if self.active_callbacks & CALLBACK_ACTIVE_ON_ERROR != 0 { - unsafe { on_error(self.ptr, at, 0); } + let at = unsafe { core::ptr::read_unaligned(events.as_ptr().add(cursor + 1) as *const u32) }.to_le() as usize; + if active_callbacks & CALLBACK_ACTIVE_ON_ERROR != 0 { + unsafe { on_error(parser_ptr, at, 0); } } cursor += EVENT_ERROR_SIZE; } EVENT_HEADERS => { - let at = unsafe { core::ptr::read_unaligned(self.events.add(cursor + 1) as *const u32) }.to_le() as usize; - let method_or_status = unsafe { core::ptr::read_unaligned(self.events.add(cursor + 5) as *const u16) }.to_le() as u32; - let should_keep_alive = unsafe { *self.events.add(cursor + 7) } != 0; - let should_upgrade = unsafe { *self.events.add(cursor + 8) } != 0; - let has_trailers = unsafe { *self.events.add(cursor + 9) } != 0; - let body_kind = unsafe { *self.events.add(cursor + 10) }; - let content_length = unsafe { core::ptr::read_unaligned(self.events.add(cursor + 11) as *const u64) }.to_le() as f64; + let at = unsafe { core::ptr::read_unaligned(events.as_ptr().add(cursor + 1) as *const u32) }.to_le() as usize; + let method_or_status = unsafe { core::ptr::read_unaligned(events.as_ptr().add(cursor + 5) as *const u16) }.to_le() as u32; + let should_keep_alive = events[cursor + 7] != 0; + let should_upgrade = events[cursor + 8] != 0; + let has_trailers = events[cursor + 9] != 0; + let body_kind = events[cursor + 10]; + let content_length = unsafe { core::ptr::read_unaligned(events.as_ptr().add(cursor + 11) as *const u64) }.to_le() as f64; - if self.active_callbacks & CALLBACK_ACTIVE_ON_HEADERS != 0 { + if active_callbacks & CALLBACK_ACTIVE_ON_HEADERS != 0 { unsafe { on_headers( - self.ptr, + parser_ptr, at, method_or_status, should_keep_alive, @@ -108,6 +123,10 @@ pub fn generate_callbacks(callbacks: &[String]) -> TokenStream { #(#replay_arms)* _ => break, } + + if unsafe { core::ptr::read_volatile(core::ptr::addr_of!(self.destroy_pending)) } { + break; + } } } } diff --git a/parser/src/lib.rs b/parser/src/lib.rs index f5f0b50..b57ac18 100644 --- a/parser/src/lib.rs +++ b/parser/src/lib.rs @@ -70,6 +70,10 @@ pub struct Parser { pub unconsumed_len: usize, pub error_description_len: u8, + // Callback replay state. These fields are managed internally. + callback_replay: bool, + destroy_pending: bool, + // Event buffer. Keep this at the end of the struct for external readers. pub events: *mut c_uchar, } @@ -143,10 +147,35 @@ impl Parser { unconsumed: ptr::null(), unconsumed_len: 0, error_description_len: 0, + callback_replay: false, + destroy_pending: false, events: events_ptr, } } + #[inline(always)] + pub(crate) fn can_mutate(&self) -> bool { !self.callback_replay } + + #[inline(always)] + pub(crate) unsafe fn can_mutate_raw(parser: *const Parser) -> bool { + !unsafe { core::ptr::read_volatile(core::ptr::addr_of!((*parser).callback_replay)) } + } + + #[inline(always)] + pub(crate) unsafe fn request_destroy_raw(parser: *mut Parser) -> bool { + if unsafe { Self::can_mutate_raw(parser) } { + true + } else { + unsafe { core::ptr::write_volatile(core::ptr::addr_of_mut!((*parser).destroy_pending), true) }; + false + } + } + + #[inline(always)] + pub(crate) unsafe fn should_destroy_after_call_raw(parser: *const Parser) -> bool { + unsafe { Self::can_mutate_raw(parser) && core::ptr::read_volatile(core::ptr::addr_of!((*parser).destroy_pending)) } + } + /// Resets a parser. The second parameters specifies if to also reset the /// parsed counter. /// @@ -161,6 +190,10 @@ impl Parser { /// * debug /// * context pub fn reset(&mut self, keep_parsed: bool) { + if !self.can_mutate() { + return; + } + self.state = STATE_START; self.paused = false; @@ -199,6 +232,10 @@ impl Parser { /// Clears all values about the message in the parser. pub fn clear(&mut self) { + if !self.can_mutate() { + return; + } + self.is_connect = false; self.method = 0; self.status = 0; @@ -256,14 +293,26 @@ impl Parser { } /// Pauses the parser. It will have to be resumed via `resume`. - pub fn pause(&mut self) { self.paused = true; } + pub fn pause(&mut self) { + if self.can_mutate() { + self.paused = true; + } + } /// Resumes the parser. - pub fn resume(&mut self) { self.paused = false; } + pub fn resume(&mut self) { + if self.can_mutate() { + self.paused = false; + } + } /// Marks the parser as finished. Any new data received via `parse` will /// put the parser in the error state. pub fn finish(&mut self) { + if !self.can_mutate() { + return; + } + match self.state { // If the parser is one of the initial states, simply jump to finish STATE_START | STATE_REQUEST_LINE | STATE_STATUS_LINE | STATE_FINISH => { @@ -294,6 +343,10 @@ impl Parser { /// Marks parsing as failed and starts a standalone error event batch. #[inline(always)] pub fn fail(&mut self, code: u8, description: &str) { + if !self.can_mutate() { + return; + } + self.set_error(code, description); let mut event_cursor = 0usize; if (self.active_events | self.active_callbacks) & EVENT_ACTIVE_ON_ERROR != 0 { diff --git a/parser/src/native.rs b/parser/src/native.rs index 071802e..388244c 100644 --- a/parser/src/native.rs +++ b/parser/src/native.rs @@ -8,6 +8,12 @@ use std::ffi::{CString, c_char, c_uchar}; use crate::parse; use crate::{Callbacks, Errors, Events, Methods, Parser, States}; +unsafe fn with_mutable_parser(parser: *mut Parser, mutate: impl FnOnce(&mut Parser)) { + if unsafe { Parser::can_mutate_raw(parser) } { + mutate(unsafe { &mut *parser }); + } +} + #[repr(C)] pub struct CStringWithLength { pub ptr: *const c_uchar, @@ -70,39 +76,45 @@ pub extern "C" fn milo_destroy(parser: *mut Parser) { return; } - unsafe { - let _ = Box::from_raw(parser); + if unsafe { Parser::request_destroy_raw(parser) } { + unsafe { + let _ = Box::from_raw(parser); + } } } /// Parses a slice of characters. It returns the number of consumed characters. #[unsafe(no_mangle)] pub extern "C" fn milo_parse(parser: *mut Parser, data: *const c_uchar, limit: usize) -> usize { - unsafe { (*parser).parse(data, limit) } + if !unsafe { Parser::can_mutate_raw(parser) } { + return 0; + } + + let consumed = unsafe { (*parser).parse(data, limit) }; + if unsafe { Parser::should_destroy_after_call_raw(parser) } { + unsafe { + let _ = Box::from_raw(parser); + } + } + consumed } /// Sets the parser event bitmask. #[unsafe(no_mangle)] pub extern "C" fn milo_set_active_events(parser: *mut Parser, value: u64) { - unsafe { - (*parser).active_events = value; - } + unsafe { with_mutable_parser(parser, |parser| parser.active_events = value) } } /// Sets the maximum body payload consumed by a single parse invocation. #[unsafe(no_mangle)] pub extern "C" fn milo_set_max_body_payload(parser: *mut Parser, value: u64) { - unsafe { - (*parser).max_body_payload = value; - } + unsafe { with_mutable_parser(parser, |parser| parser.max_body_payload = value) } } /// Sets whether parsing should stop after headers have completed. #[unsafe(no_mangle)] pub extern "C" fn milo_set_suspend_after_headers(parser: *mut Parser, value: bool) { - unsafe { - (*parser).suspend_after_headers = value; - } + unsafe { with_mutable_parser(parser, |parser| parser.suspend_after_headers = value) } } /// Resets a parser. The second parameters specifies if to also reset the @@ -120,35 +132,37 @@ pub extern "C" fn milo_set_suspend_after_headers(parser: *mut Parser, value: boo /// * debug /// * context #[unsafe(no_mangle)] -pub extern "C" fn milo_reset(parser: *mut Parser, keep_parsed: bool) { unsafe { (*parser).reset(keep_parsed) } } +pub extern "C" fn milo_reset(parser: *mut Parser, keep_parsed: bool) { + unsafe { with_mutable_parser(parser, |parser| parser.reset(keep_parsed)) } +} /// Clears all values about the message in the parser. /// /// The autodetect and is_request fields are not cleared. #[unsafe(no_mangle)] -pub extern "C" fn milo_clear(parser: *mut Parser) { unsafe { (*parser).clear() } } +pub extern "C" fn milo_clear(parser: *mut Parser) { unsafe { with_mutable_parser(parser, Parser::clear) } } /// Pauses the parser. It will have to be resumed via `milo_resume`. #[unsafe(no_mangle)] -pub extern "C" fn milo_pause(parser: *mut Parser) { unsafe { (*parser).pause() } } +pub extern "C" fn milo_pause(parser: *mut Parser) { unsafe { with_mutable_parser(parser, Parser::pause) } } /// Resumes the parser. #[unsafe(no_mangle)] -pub extern "C" fn milo_resume(parser: *mut Parser) { unsafe { (*parser).resume() } } +pub extern "C" fn milo_resume(parser: *mut Parser) { unsafe { with_mutable_parser(parser, Parser::resume) } } /// Completes the current message without consuming more input. #[unsafe(no_mangle)] -pub extern "C" fn milo_complete(parser: *mut Parser) { unsafe { (*parser).complete() } } +pub extern "C" fn milo_complete(parser: *mut Parser) { unsafe { with_mutable_parser(parser, Parser::complete) } } /// Marks the parser as finished. Any new data received via `milo_parse` will /// put the parser in the error state. #[unsafe(no_mangle)] -pub extern "C" fn milo_finish(parser: *mut Parser) { unsafe { (*parser).finish() } } +pub extern "C" fn milo_finish(parser: *mut Parser) { unsafe { with_mutable_parser(parser, Parser::finish) } } /// Marks the parsing a failed, setting a error code and and error message. #[unsafe(no_mangle)] pub extern "C" fn milo_fail(parser: *mut Parser, code: u8, description: CStringWithLength) { - unsafe { (*parser).fail(code, description.into()) }; + unsafe { with_mutable_parser(parser, |parser| parser.fail(code, description.into())) } } /// Returns the current parser's state as string. diff --git a/parser/src/parse.rs b/parser/src/parse.rs index 80105a0..44afd1d 100644 --- a/parser/src/parse.rs +++ b/parser/src/parse.rs @@ -25,6 +25,10 @@ impl Parser { /// /// It returns the number of consumed characters. pub fn parse(&mut self, input: *const c_uchar, limit: usize) -> usize { + if !self.can_mutate() { + return 0; + } + let mut event_cursor = 0usize; let mut failed = false; let active_events = self.active_events | self.active_callbacks; @@ -1489,7 +1493,7 @@ impl Parser { } if self.active_callbacks != 0 { - self.invoke_callbacks(); + self.invoke_callbacks(event_cursor + EVENT_END_SIZE); } // Return the number of consumed bytes @@ -1502,6 +1506,10 @@ impl Parser { /// Milo after headers were parsed, then need Milo to perform the normal /// completion transition and emit completion events. pub fn complete(&mut self) { + if !self.can_mutate() { + return; + } + match self.state { STATE_BODY_DECISION | STATE_TUNNEL diff --git a/parser/src/wasm.rs b/parser/src/wasm.rs index 73d380c..3901061 100644 --- a/parser/src/wasm.rs +++ b/parser/src/wasm.rs @@ -3,6 +3,13 @@ use std::slice; use crate::Parser; +unsafe fn with_mutable_parser(parser: *mut c_void, mutate: impl FnOnce(&mut Parser)) { + let parser = parser as *mut Parser; + if unsafe { Parser::can_mutate_raw(parser) } { + mutate(unsafe { &mut *parser }); + } +} + #[cfg(any(debug_assertions, feature = "debug"))] pub fn debug(message: String) { unsafe { crate::logger(((message.as_ptr() as u64) << 32) + message.len() as u64) } } @@ -48,56 +55,67 @@ pub fn destroy(parser: *mut c_void) { return; } - unsafe { - let _ = Box::from_raw(parser as *mut Parser); + let parser = parser as *mut Parser; + if unsafe { Parser::request_destroy_raw(parser) } { + unsafe { + let _ = Box::from_raw(parser); + } } } /// Resets a parser. The second parameters specifies if to also reset the /// parsed counter. #[unsafe(no_mangle)] -pub fn reset(parser: *mut c_void, keep_parsed: bool) { unsafe { (*(parser as *mut Parser)).reset(keep_parsed) } } +pub fn reset(parser: *mut c_void, keep_parsed: bool) { + unsafe { with_mutable_parser(parser, |parser| parser.reset(keep_parsed)) } +} /// Clears all values in the parser. /// /// Persisted fields, unconsumed data and the position are not cleared. #[unsafe(no_mangle)] -pub fn clear(parser: *mut c_void) { unsafe { (*(parser as *mut Parser)).clear() } } +pub fn clear(parser: *mut c_void) { unsafe { with_mutable_parser(parser, Parser::clear) } } // Parses a slice of characters. It returns the number of consumed characters. #[unsafe(no_mangle)] pub fn parse(parser: *mut c_void, data: *const c_uchar, limit: usize) -> usize { - unsafe { (*(parser as *mut Parser)).parse(data, limit) } + let parser = parser as *mut Parser; + if !unsafe { Parser::can_mutate_raw(parser) } { + return 0; + } + + let consumed = unsafe { (*parser).parse(data, limit) }; + if unsafe { Parser::should_destroy_after_call_raw(parser) } { + unsafe { + let _ = Box::from_raw(parser); + } + } + consumed } /// Pauses the parser. It will have to be resumed via `resume`. #[unsafe(no_mangle)] -pub fn pause(parser: *mut c_void) { unsafe { (*(parser as *mut Parser)).pause() } } +pub fn pause(parser: *mut c_void) { unsafe { with_mutable_parser(parser, Parser::pause) } } /// Resumes the parser. #[unsafe(no_mangle)] -pub fn resume(parser: *mut c_void) { unsafe { (*(parser as *mut Parser)).resume() } } +pub fn resume(parser: *mut c_void) { unsafe { with_mutable_parser(parser, Parser::resume) } } /// Completes the current message without consuming more input. #[unsafe(no_mangle)] -pub fn complete(parser: *mut c_void) { unsafe { (*(parser as *mut Parser)).complete() } } +pub fn complete(parser: *mut c_void) { unsafe { with_mutable_parser(parser, Parser::complete) } } /// Marks the parser as finished. Any new data received via `parse` will /// put the parser in the error state. #[unsafe(no_mangle)] -pub fn finish(parser: *mut c_void) { unsafe { (*(parser as *mut Parser)).finish() } } +pub fn finish(parser: *mut c_void) { unsafe { with_mutable_parser(parser, Parser::finish) } } /// Marks the parser as failed. #[unsafe(no_mangle)] pub fn fail(parser: *mut c_void, code: u8, description_ptr: *const c_uchar, description_len: usize) { - unsafe { - (*(parser as *mut Parser)).fail( - code, - std::str::from_utf8_unchecked(slice::from_raw_parts(description_ptr, description_len)), - ); - } + let description = unsafe { std::str::from_utf8_unchecked(slice::from_raw_parts(description_ptr, description_len)) }; + unsafe { with_mutable_parser(parser, |parser| parser.fail(code, description)) } } - // Generates a getter. // pub fn wasm_getter(input: TokenStream) -> TokenStream { // let snake_matcher = Regex::new(r"([A-Z])").unwrap(); @@ -263,101 +281,74 @@ pub fn has_trailers(parser: *const c_void) -> bool { unsafe { (*(parser as *cons /// internal use. #[unsafe(no_mangle)] pub fn get_error_description_raw(parser: *mut c_void) -> u64 { - let parser = unsafe { &(*(parser as *const Parser)) }; - - let ptr = parser.error_description.as_ptr() as u64; - let len = parser.error_description_len as u64; + let parser = parser as *const Parser; + let ptr = unsafe { core::ptr::addr_of!((*parser).error_description) as *const u8 as u64 }; + let len = unsafe { core::ptr::read_volatile(core::ptr::addr_of!((*parser).error_description_len)) as u64 }; (ptr << 32) + len } #[unsafe(no_mangle)] pub fn set_should_autodetect(parser: *mut c_void, value: bool) { - unsafe { - (*(parser as *mut Parser)).autodetect = value; - } + unsafe { with_mutable_parser(parser, |parser| parser.autodetect = value) } } #[unsafe(no_mangle)] pub fn set_is_request(parser: *mut c_void, value: bool) { - unsafe { - (*(parser as *mut Parser)).is_request = value; - } + unsafe { with_mutable_parser(parser, |parser| parser.is_request = value) } } #[unsafe(no_mangle)] pub fn set_should_manage_unconsumed(parser: *mut c_void, value: bool) { - unsafe { - (*(parser as *mut Parser)).manage_unconsumed = value; - } + unsafe { with_mutable_parser(parser, |parser| parser.manage_unconsumed = value) } } #[unsafe(no_mangle)] pub fn set_should_suspend_after_headers(parser: *mut c_void, value: bool) { - unsafe { - (*(parser as *mut Parser)).suspend_after_headers = value; - } + unsafe { with_mutable_parser(parser, |parser| parser.suspend_after_headers = value) } } #[unsafe(no_mangle)] pub fn set_max_start_line_length(parser: *mut c_void, value: usize) { - unsafe { - (*(parser as *mut Parser)).max_start_line_length = value; - } + unsafe { with_mutable_parser(parser, |parser| parser.max_start_line_length = value) } } #[unsafe(no_mangle)] pub fn set_max_header_length(parser: *mut c_void, value: usize) { - unsafe { - (*(parser as *mut Parser)).max_header_length = value; - } + unsafe { with_mutable_parser(parser, |parser| parser.max_header_length = value) } } #[unsafe(no_mangle)] pub fn set_max_body_payload(parser: *mut c_void, value: u64) { - unsafe { - (*(parser as *mut Parser)).max_body_payload = value; - } + unsafe { with_mutable_parser(parser, |parser| parser.max_body_payload = value) } } #[unsafe(no_mangle)] pub fn set_should_continue_without_data(parser: *mut c_void, value: bool) { - unsafe { - (*(parser as *mut Parser)).continue_without_data = value; - } + unsafe { with_mutable_parser(parser, |parser| parser.continue_without_data = value) } } #[unsafe(no_mangle)] pub fn set_should_skip_body(parser: *mut c_void, value: bool) { - unsafe { - (*(parser as *mut Parser)).skip_body = value; - } + unsafe { with_mutable_parser(parser, |parser| parser.skip_body = value) } } #[unsafe(no_mangle)] pub fn set_is_connect(parser: *mut c_void, value: bool) { - unsafe { - (*(parser as *mut Parser)).is_connect = value; - } + unsafe { with_mutable_parser(parser, |parser| parser.is_connect = value) } } #[unsafe(no_mangle)] pub fn set_debug(parser: *mut c_void, value: bool) { - unsafe { - (*(parser as *mut Parser)).debug = value; - } + unsafe { with_mutable_parser(parser, |parser| parser.debug = value) } } #[unsafe(no_mangle)] pub fn set_active_callbacks(parser: *mut c_void, value: u64) { - unsafe { - (*(parser as *mut Parser)).active_callbacks = value; - } + unsafe { with_mutable_parser(parser, |parser| parser.active_callbacks = value) } } #[unsafe(no_mangle)] pub fn set_active_events(parser: *mut c_void, value: u64) { - unsafe { - (*(parser as *mut Parser)).active_events = value; - } + unsafe { with_mutable_parser(parser, |parser| parser.active_events = value) } } diff --git a/parser/tests/callback_safety.rs b/parser/tests/callback_safety.rs new file mode 100644 index 0000000..35c8861 --- /dev/null +++ b/parser/tests/callback_safety.rs @@ -0,0 +1,93 @@ +use core::ffi::c_void; +use std::sync::atomic::{AtomicBool, Ordering}; + +use milo_parser::{ + CALLBACK_ACTIVE_ON_MESSAGE_START, CALLBACK_ACTIVE_ON_REQUEST, ERROR_NONE, ERROR_UNEXPECTED_CHARACTER, Parser, + STATE_START, milo_create, milo_destroy, milo_parse, +}; + +#[derive(Default)] +struct ReentryContext { + nested_consumed: usize, + mutation_rejected: bool, + message_start_called: bool, +} + +fn noop(_parser: &mut Parser, _at: usize, _len: usize) {} + +fn attempt_reentry(parser: &mut Parser, _at: usize, _len: usize) { + let state = parser.state; + let parsed = parser.parsed; + let error_code = parser.error_code; + let nested = b"GET /nested HTTP/1.1\r\n\r\n"; + + let nested_consumed = parser.parse(nested.as_ptr(), nested.len()); + parser.pause(); + parser.reset(false); + parser.clear(); + parser.complete(); + parser.finish(); + parser.fail(ERROR_UNEXPECTED_CHARACTER, "callback mutation"); + + let context = unsafe { &mut *(parser.context as *mut ReentryContext) }; + context.nested_consumed = nested_consumed; + context.mutation_rejected = + parser.state == state && parser.parsed == parsed && parser.error_code == error_code && !parser.paused; + + // The current callback batch uses immutable callback and activation snapshots. + parser.active_callbacks = 0; + parser.callbacks.on_message_start = noop; +} + +fn record_message_start(parser: &mut Parser, _at: usize, _len: usize) { + let context = unsafe { &mut *(parser.context as *mut ReentryContext) }; + context.message_start_called = true; +} + +#[test] +fn callback_reentry_and_mutation_are_rejected() { + let mut parser = Parser::new(); + parser.autodetect = false; + parser.is_request = true; + let context = Box::new(ReentryContext::default()); + parser.context = Box::into_raw(context) as *mut c_void; + parser.active_callbacks = CALLBACK_ACTIVE_ON_REQUEST | CALLBACK_ACTIVE_ON_MESSAGE_START; + parser.callbacks.on_request = attempt_reentry; + parser.callbacks.on_message_start = record_message_start; + + let message = b"GET / HTTP/1.1\r\n\r\n"; + assert_eq!(parser.parse(message.as_ptr(), message.len()), message.len()); + assert_eq!(parser.state, STATE_START); + assert_eq!(parser.error_code, ERROR_NONE); + + let context = unsafe { Box::from_raw(parser.context as *mut ReentryContext) }; + assert_eq!(context.nested_consumed, 0); + assert!(context.mutation_rejected); + assert!(context.message_start_called); + parser.context = core::ptr::null_mut(); +} + +static CALLBACK_AFTER_DESTROY: AtomicBool = AtomicBool::new(false); + +fn destroy_during_callback(parser: &mut Parser, _at: usize, _len: usize) { milo_destroy(parser as *mut Parser); } + +fn callback_after_destroy(_parser: &mut Parser, _at: usize, _len: usize) { + CALLBACK_AFTER_DESTROY.store(true, Ordering::SeqCst); +} + +#[test] +fn callback_destruction_is_deferred_and_stops_replay() { + CALLBACK_AFTER_DESTROY.store(false, Ordering::SeqCst); + let parser = milo_create(); + unsafe { + (*parser).autodetect = false; + (*parser).is_request = true; + (*parser).active_callbacks = CALLBACK_ACTIVE_ON_REQUEST | CALLBACK_ACTIVE_ON_MESSAGE_START; + (*parser).callbacks.on_request = destroy_during_callback; + (*parser).callbacks.on_message_start = callback_after_destroy; + } + + let message = b"GET / HTTP/1.1\r\n\r\n"; + assert_eq!(milo_parse(parser, message.as_ptr(), message.len()), message.len()); + assert!(!CALLBACK_AFTER_DESTROY.load(Ordering::SeqCst)); +} diff --git a/parser/wasm/src/template.js b/parser/wasm/src/template.js index 8c3d066..ef0d1ef 100644 --- a/parser/wasm/src/template.js +++ b/parser/wasm/src/template.js @@ -26,8 +26,44 @@ function destroy (parser) { this.destroy(parser) } -function parse (parser, data, limit) { - return this.parse(parser, data, limit) >>> 0 +function parse (context, parser, data, limit) { + const outermost = context.parseDepth === 0 + if (outermost) { + context.callbackError = undefined + } + context.parseDepth++ + + let result + let wasmError + try { + result = context.wasm.parse(parser, data, limit) >>> 0 + } catch (error) { + wasmError = { value: error } + } finally { + context.parseDepth-- + } + + if (outermost && context.callbackError !== undefined) { + const error = context.callbackError.value + context.callbackError = undefined + throw error + } + if (wasmError !== undefined) { + throw wasmError.value + } + return result +} + +function wrapCallback (context, callback) { + return (...args) => { + try { + callback(...args) + } catch (error) { + if (context.callbackError === undefined) { + context.callbackError = { value: error } + } + } + } } function fail (parser, code, description) { @@ -67,7 +103,7 @@ export function noop () {} export function setup (env = {}) { let { logger: logOption, ...instanceEnvironment } = env let logger = noop - const context = {} + const context = { callbackError: undefined, parseDepth: 0, wasm: undefined } if (logOption) { if (typeof logOption !== 'function') { @@ -77,6 +113,12 @@ export function setup (env = {}) { logger = log.bind(context, logOption) } + for (const [name, callback] of Object.entries(instanceEnvironment)) { + if (typeof callback === 'function') { + instanceEnvironment[name] = wrapCallback(context, callback) + } + } + // Create the WASM instance /* eslint-disable-next-line no-undef */ const instance = new WebAssembly.Instance(wasmModule, { @@ -89,6 +131,7 @@ export function setup (env = {}) { const wasm = instance.exports context.memory = wasm.memory + context.wasm = wasm const milo = { /* REPLACE: version */ @@ -101,7 +144,7 @@ export function setup (env = {}) { dealloc: dealloc.bind(wasm), create: create.bind(wasm), destroy: destroy.bind(wasm), - parse: parse.bind(wasm), + parse: parse.bind(undefined, context), fail: fail.bind(wasm), hasDebug: hasDebug.bind(wasm), clear: wasm.clear, diff --git a/parser/wasm/test/callback-safety.test.js b/parser/wasm/test/callback-safety.test.js new file mode 100644 index 0000000..0b593bd --- /dev/null +++ b/parser/wasm/test/callback-safety.test.js @@ -0,0 +1,110 @@ +import assert from 'node:assert/strict' +import { it } from 'node:test' +import { setup } from './helpers.js' + +function allocate (milo, input) { + const bytes = Buffer.from(input) + const ptr = milo.alloc(bytes.length) + new Uint8Array(milo.memory.buffer, ptr, bytes.length).set(bytes) + return { bytes, ptr } +} + +it('callback_reentry_and_mutation_are_rejected', t => { + let nestedConsumed + let mutationRejected = false + let messageStartCalled = false + + const milo = setup({ + on_request (parser) { + const state = milo.getState(parser) + const parsed = milo.getParsed(parser) + const errorCode = milo.getErrorCode(parser) + + nestedConsumed = milo.parse(parser, 0, 0) + milo.pause(parser) + milo.reset(parser, false) + milo.clear(parser) + milo.complete(parser) + milo.finish(parser) + milo.fail(parser, milo.ERROR_UNEXPECTED_CHARACTER, 'callback mutation') + milo.setIsRequest(parser, false) + + mutationRejected = + milo.getState(parser) === state && + milo.getParsed(parser) === parsed && + milo.getErrorCode(parser) === errorCode && + !milo.isPaused(parser) && + milo.isRequest(parser) + + // The current callback batch uses an immutable activation snapshot. + milo.setActiveCallbacks(parser, 0n) + }, + on_message_start () { + messageStartCalled = true + } + }) + + const parser = milo.create() + t.after(() => milo.destroy(parser)) + milo.setShouldAutodetect(parser, false) + milo.setIsRequest(parser, true) + milo.setActiveCallbacks(parser, milo.CALLBACK_ACTIVE_ON_REQUEST | milo.CALLBACK_ACTIVE_ON_MESSAGE_START) + const { bytes, ptr } = allocate(milo, 'GET / HTTP/1.1\r\n\r\n') + t.after(() => milo.dealloc(ptr, bytes.length)) + + assert.equal(milo.parse(parser, ptr, bytes.length), bytes.length) + assert.equal(nestedConsumed, 0) + assert.equal(mutationRejected, true) + assert.equal(messageStartCalled, true) + assert.equal(milo.getState(parser), milo.STATE_START) + assert.equal(milo.getErrorCode(parser), milo.ERROR_NONE) +}) + +it('callback_exceptions_are_rethrown_after_replay_cleanup', t => { + const expected = new Error('callback failed') + const milo = setup({ + on_request () { + throw expected + } + }) + + const parser = milo.create() + t.after(() => milo.destroy(parser)) + milo.setShouldAutodetect(parser, false) + milo.setIsRequest(parser, true) + milo.setActiveCallbacks(parser, milo.CALLBACK_ACTIVE_ON_REQUEST) + const { bytes, ptr } = allocate(milo, 'GET / HTTP/1.1\r\n\r\n') + t.after(() => milo.dealloc(ptr, bytes.length)) + + assert.throws( + () => milo.parse(parser, ptr, bytes.length), + error => error === expected + ) + + // The replay guard was cleared before the JavaScript exception was rethrown. + milo.setActiveCallbacks(parser, 0n) + assert.equal(milo.parse(parser, ptr, bytes.length), bytes.length) +}) + +it('callback_destruction_is_deferred_and_stops_replay', t => { + let callbackAfterDestroy = false + + const milo = setup({ + on_request (parser) { + milo.destroy(parser) + }, + on_message_start () { + callbackAfterDestroy = true + } + }) + + const parser = milo.create() + milo.setShouldAutodetect(parser, false) + milo.setIsRequest(parser, true) + milo.setActiveCallbacks(parser, milo.CALLBACK_ACTIVE_ON_REQUEST | milo.CALLBACK_ACTIVE_ON_MESSAGE_START) + const { bytes, ptr } = allocate(milo, 'GET / HTTP/1.1\r\n\r\n') + t.after(() => milo.dealloc(ptr, bytes.length)) + + assert.equal(milo.parse(parser, ptr, bytes.length), bytes.length) + assert.equal(callbackAfterDestroy, false) +})