search

hohav / peppi

Rust parser for Slippi SSBM replay files
MIT License
37 stars 9 forks source link

Skipping frames on streaming replay panics. #51

Closed Kered13 closed 7 months ago

Kered13 commented 1 year ago

This bug is similar to #19, however this bug effects perfectly valid replays that should parse. The issue is on the same line, de.rs:982. For streaming replays length is 0, which guarantees an underflow and panic. The solution is straightforward: If length is 0, then do not skip directly to GameEnd, instead keep processing events normally, but just don't parse the frame events.

I also noticed that there is no unit testing for streaming replays at all, this should be fixed. I have attached a streaming replay that can be used for unit testing. It is identical to game.slp, except the length field has been modified to 0.

streaming_game.zip

NickCondron commented 1 year ago

41 addresses this. It also adds a unit test for this case. It removes the panic, but it will return a Result::Error if you attempt to skip frames on a corrupt/in-progress replay. In case the raw length is 0, it might be better to skip frame events as they're read like you suggest. However, that would be a more involved change to the code and is a subtle change in semantics of the skip. Maybe your suggestion is better I'll have to think about it.

hohav commented 7 months ago

Closed via #41.