Fixed receiving "send" messages - #235
Merged
Merged
Conversation
- Fixed receiving "send" messages - Updated examples
Contributor
Author
|
This fixes #210 |
jhasheng
pushed a commit
to jhasheng/frida-rust
that referenced
this pull request
May 20, 2026
The merge in 5fe8f72 picked up parts of frida#235 (the `payload.get(N)` accessors in Exports::call, and the type change from `SendPayload` to `serde_json::Value`) but dropped two hunks: 1. The deletion of the now-dead `SendPayload` struct. 2. The accessor rewrite inside `call_on_message`, which still read `msg.payload.r#type`. After the type change this no longer compiled (E0609). Re-apply both hunks so the file matches what frida#235 intended: - Remove `SendPayload` (no remaining references in-tree). - Match the rpc envelope via `msg.payload.get(0).and_then(...)`, consistent with the same pattern at lines 367-368.
s1341
pushed a commit
that referenced
this pull request
May 20, 2026
…cript_from_bytes (#239) * fix(script): route on_message through CallbackHandler.script_handler (#189) Pre-fix, `handle_message` passed `*mut CallbackHandler` as the `g_signal_connect_data` user_data, but `call_on_message::<I>` re-cast that pointer to `*mut I` and dereferenced it: let handler: &mut I = &mut *(user_data as *mut I); handler.on_message(formatted_msg, data_vec); This reads I-sized bytes from the start of `CallbackHandler` (the `channel: (Sender, Receiver)` tuple) and treats them as `I`. It is undefined behavior, masked only when `I` is a ZST (e.g. the doc example's `struct Handler;`). Symptoms reported in #189: - `Arc<Mutex<_>>` handlers crash inside atomic_load. - Field reads return garbage / refcounts spike (e.g. 1624879470736). - Poisoned-mutex errors. For a handler whose fields include a `parking_lot`/`std::sync::Mutex`, the spurious atomic op on what frida-core treats as `Sender` internals can also deadlock `script.load_sync` outright if the JS script body issues a synchronous `send()` — the bogus SRWLOCK address looks held and `AcquireSRWLockExclusive` never returns. Fix: `user_data` is already `*mut CallbackHandler`, and `handle_message` stores the boxed trait object in `cb.script_handler`. Use that field instead of the type-pun. The `I` type parameter is kept (and given a `PhantomData::<I>` no-op) so monomorphization still emits a distinct fn pointer per handler type, preserving the original ABI for any external code that relied on it. Regression test (downstream): a `ScriptHandler` whose state includes a `u64` counter must increment cleanly from 1, 2, 3, ... rather than starting at a garbage value and corrupting adjacent memory. * feat(session): wrap compile_script + create_script_from_bytes frida-core has shipped a bytecode path for years: frida_session_compile_script_sync(session, source, opts) -> GBytes frida_session_create_script_from_bytes_sync(session, bytes, opts) -> FridaScript The Python and Node bindings expose both. frida-rust 0.17.2 only wraps the source-string path through `create_script`, leaving callers to either drop into frida-sys themselves or recompile on every load. This adds two thin methods on `Session`: fn compile_script(&self, source, &mut opts) -> Result<Vec<u8>> fn create_script_from_bytes<'b>(&self, bytes, &mut opts) -> Result<Script<'b>> `compile_script` copies the returned GBytes into an owned `Vec<u8>` and `g_bytes_unref`s the original. `create_script_from_bytes` wraps `g_bytes_new` / `g_bytes_unref` around the C call. Bytecode is runtime-specific — callers must compile and load with matching `ScriptOption::set_runtime`. Documented inline. Use cases this unblocks: - Startup speedup: skip the JS parse pass at every attach. - Soft script protection: ship .bin artifacts instead of readable JS. - Self-attach `compile_script` workflows so build pipelines can emit bytecode without spinning up an extra process. * docs(examples): add compile_script roundtrip example Demonstrates the new Session::compile_script and Session::create_script_from_bytes pair from 6a92b72: source JS -> compile_script -> Vec<u8> bytecode -> write to compiled.bin (ship-as-artifact) -> read back -> create_script_from_bytes -> Script -> load Attaches to self (pid=0) so it runs out of the box with no external target, and pins ScriptRuntime::QJS on both ends to make the runtime-specific-bytecode constraint explicit. * test(session): add integration tests for compile_script + create_script_from_bytes Covers three regression surfaces for 6a92b72: 1. compile_script returns a non-empty Vec<u8> and the result loads back through create_script_from_bytes (full QJS roundtrip via attach-to-self / pid=0). 2. compile_script with an interior-NUL source returns Error::CStringFailed without reaching the C side. 3. Both new methods work with bare ScriptOption::default(), matching the contract of create_script. CI already runs 'cargo test --features=auto-download' on the frida crate across Linux/macOS/Windows x x86/x86_64, so these run on every PR without workflow changes. * fix(script): complete dropped hunks from the #235 merge The merge in 5fe8f72 picked up parts of #235 (the `payload.get(N)` accessors in Exports::call, and the type change from `SendPayload` to `serde_json::Value`) but dropped two hunks: 1. The deletion of the now-dead `SendPayload` struct. 2. The accessor rewrite inside `call_on_message`, which still read `msg.payload.r#type`. After the type change this no longer compiled (E0609). Re-apply both hunks so the file matches what #235 intended: - Remove `SendPayload` (no remaining references in-tree). - Match the rpc envelope via `msg.payload.get(0).and_then(...)`, consistent with the same pattern at lines 367-368. * test(session): serialize frida self-attach tests via process-wide mutex cargo test runs tests in parallel by default. Two threads doing concurrent device.attach(0) / detach() inside the same process corrupts frida-core's agent state — on Windows the test binary exits with STATUS_STACK_BUFFER_OVERRUN (0xc0000409); on other platforms it would be similarly undefined. Add a static Mutex<()> that every #[test] in this file acquires for the full attach -> detach span. unwrap_or_else(into_inner) recovers a poisoned lock so a panic in one test does not cascade and turn the rest of the suite red. --------- Co-authored-by: yw <junjunus@hotmail.com> Co-authored-by: yw <yw@yw>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Send messages were not being handled properly.