Conversation
Summary: In `InMemoryAgentBusState::read_filtered_entries`, slicing `&bus[start_idx..end_idx]` calculated `end_idx = (end_position as usize).min(bus.len())` without checking if `start_idx < bus.len()`. When a consumer reads beyond the tail of a bus or reads an empty bus with non-zero bounds (e.g. `start_position: 5, end_position: 10` on a bus with 2 entries), `start_idx > end_idx`, causing Rust to panic at runtime with `slice index starts at 5 but ends at 2`. Fix this by: 1. Rejecting negative `start_position` early. 2. Guarding `if start_idx >= bus.len()` to return `Ok((vec![], end_position))` matching the defensive behavior of `fetch_entries`. 3. Making header access safe on max_entries truncation with `.map(|h| h.log_position + 1).unwrap_or(end_position)`. 4. Adding direct unit tests for `InMemoryAgentBusState` covering empty bus, beyond-tail, negative start, and pagination. 5. Adding an AgentBus cross-implementation conformance scenario `run_test_read_next_beyond_tail` in `agentbus_tests`. 6. Removing unused `BusId` imports in `bus/core/src/channeled_agentbus.rs` and `logact/commit_service/tests/src/fixtures/grpc_bus_id_encoding.rs` to ensure clean zero-warning builds. Test Plan: - cargo test -p agentbus_simple (4/4 passed) - cargo test -p agentbus_tests --test sim_tests -- run_test_read_next_beyond_tail (14/14 passed across all fixtures) - cargo check --workspace (0 errors, 0 warnings) - cargo fmt --all -- --check (0 formatting diffs)
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
1 participant
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.
Summary
Fixes #3.
In
InMemoryAgentBusState::read_filtered_entries, slicing&bus[start_idx..end_idx]computedend_idx = (end_position as usize).min(bus.len())without verifying thatstart_idx < bus.len().When a consumer reads beyond the tail of a populated bus or reads an empty bus with non-zero bounds (e.g.
start_position: 5, end_position: 10on a bus with 2 entries),start_idx (5) > end_idx (2), causing Rust to panic at runtime:Changes
start_positionearly, and guardif start_idx >= bus.len() { return Ok((vec![], end_position)); }prior to slicing, matching the defensive behavior offetch_entries.expectpanic on entry truncation by using.map(|h| h.log_position + 1).unwrap_or(end_position).agentbus_simple(in_memory_agentbus_state.rs) covering empty bus reads, beyond-tail reads, negative start rejection, and pagination.run_test_read_next_beyond_tailinagentbus_tests(test_scenarios.rs) to verify boundary and beyond-tail behavior across all 14+ AgentBus test fixtures.BusIdimports inbus/core/src/channeled_agentbus.rsandlogact/commit_service/tests/src/fixtures/grpc_bus_id_encoding.rsto ensure clean, warning-free builds under-D warnings.Test Plan
cargo test -p agentbus_simple(4/4 passed)cargo test -p agentbus_tests --test sim_tests -- run_test_read_next_beyond_tail(14/14 passed across all fixtures)cargo check --workspace(0 errors, 0 warnings)cargo fmt --all -- --check(0 formatting diffs)