lore: return an error on a malformed IPC message header#16
Open
glitch-ux wants to merge 1 commit into
Open
Conversation
blocking_read_v1_message read the 5-byte V1 header off the local IPC socket and unwrapped V1Header::from_bytes, panicking when the header's serialization-type byte was not a known value. The enclosing function already returns Result<_, MessageError> and from_bytes produces a descriptive error, so propagate it with `?` instead of panicking. Add a regression test that drives a header with an invalid serialization-type byte through blocking_read_v1_message and asserts it returns an error. Fixes EpicGames#15 Signed-off-by: glitch-ux <glitch-ux@users.noreply.github.com>
Collaborator
|
Makes sense, but please avoid adding extra The PR intake process will do proper user attribution in the Lore revision that will result from this being applied - the revision will carry information that you submitted this change. |
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
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.
What
Make
blocking_read_v1_messagereturn a recoverable error on a malformed V1 IPCmessage header instead of panicking, and add a regression test.
Why
blocking_read_v1_message(lore/src/remote/message.rs) reads the 5-byte V1header off the local IPC socket (the
loreCLI ↔ locallore service) andunwrapped the parsed header:
V1Header::from_bytesreturnsErr(MessageError)for a header whoseserialization-type byte isn't a known value, but the caller
.unwrap()s it, soa corrupted/partial frame — or a version-mismatched local service — panics the
process instead of surfacing that descriptive error. The enclosing function
already returns
Result<Option<(V1Header, Message)>, MessageError>and everyother read in it uses
?, so the recoverable path already exists; the.unwrap()simply bypassed it.Fixes #15.
How
V1Header::from_bytes(&header_bytes).unwrap()→V1Header::from_bytes(&header_bytes)?.The error type already matches
MessageError, so no mapping is needed — thedescriptive error (
"Message received with invalid serialization type: …") nowreaches the caller. No public API, wire-protocol, or on-disk-format change.
Testing
errors_on_invalid_serialization_type_in_headertolore/tests/remote_message.rs: it feeds a header with an invalidserialization-type byte (
0xff) throughblocking_read_v1_messageand assertsit returns
Err. Before this change that path panics at theunwrap.cargo test -p lore --test remote_message→ 3 passed.cargo clippy -p lore --all-targets -- -D warnings --no-deps→ clean.cargo +nightly fmt --all --check→ clean.AI assistance
This change was prepared with the help of an AI coding assistant (Claude Code),
in keeping with the project's AI assistance policy. The diff was built and
verified locally as shown in Testing, and is submitted under my
responsibility.