Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix panic for corrupt replays #41

Merged
merged 2 commits into from
Feb 25, 2023
Merged

Fix panic for corrupt replays #41

merged 2 commits into from
Feb 25, 2023

Conversation

NickCondron
Copy link
Contributor

Fixes #19

@@ -972,7 +972,13 @@ pub fn deserialize<R: Read, H: Handlers>(
while (raw_len == 0 || bytes_read < raw_len) && last_event != Some(Event::GameEnd) {
if skip_frames && last_event == Some(Event::GameStart) {
// Skip to GameEnd, which we assume is the last event in the stream!
let skip = raw_len - bytes_read - payload_sizes[&(Event::GameEnd as u8)] as usize - 1;
let end_offset: usize = payload_sizes[&(Event::GameEnd as u8)] as usize + 1;
if raw_len == 0 || raw_len - bytes_read < end_offset {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can just be if skip < 0 if you move it below let skip = ....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The panic actually happens on the subtraction because the corrupt replay has raw_len as 0.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=8e5f0273ee8001e570ea133c02e3446f

The alternative is to do a bunch of ugly casts to/from usize, but I don't think that would make sense here.

peppi/tests/peppi.rs Outdated Show resolved Hide resolved
@NickCondron
Copy link
Contributor Author

  • Rebased
  • Added corrupt replay to exclusions for round_trip
  • Slightly modified tests/common/mod.rs to make testing for errors easier

@hohav hohav merged commit 416d459 into hohav:main Feb 25, 2023
@NickCondron NickCondron deleted the skip-panic branch February 26, 2023 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parsing replay files can panic in debug mode.
2 participants