search

abraker95 / ultimate_osu_analyzer

Python rewrite of my old osu analyzer that aims to be a lot more useful
MIT License
28 stars 1 forks source link

osrparse throws OverflowError #32

Closed MaxKruse closed 4 years ago

MaxKruse commented 4 years ago

Fresh install of program. Run program. Drag and drop map file. Drag and drop replay file ->

abraker95 commented 4 years ago

@MaxKruse is it a specific replay that does this or does this happen to all replays? I have not come across this error, so helping me replicate this is an important first step to fixing this.

Also sorry it took so long to reply. Wasn't getting any notifications for issues for some reason.

MaxKruse commented 4 years ago

Example map: https://osu.ppy.sh/b/1106518 with replay file https://ripple.moe/web/replays/13523698

This crashes and is a random map i have tested.

MaxKruse commented 4 years ago

I have following assumptions:

Wrong python version used (i tested with both 3.6 through 3.8) Wrong requirements.txt versions specified Environment differences (which i cannot list, it can be litarlly anything)

abraker95 commented 4 years ago

@MaxKruse Try now I pushed a fix.

Rough description of what went on:

The crash is indeed specific to that particular replay file. I was able to import the replay into osu, and after exporting the replay from osu via F2 I am able to open the newly exported replay for whatever reason.

Examining the original file from ripple and the one exported from osu! I thought that the one from ripple has its life bar graph data corrupted. I compared the two files in a hex editor (ripple on left and exported on right)

A technical description of what went wrong:

You can see a 0B highlighted blue on the right. That indicates the start of a ULEB128 string. Without it, the data would be interpreted as a regular string. A ULEB128 string contains a section that says how long the string is and a section of the actual data. That byte is not present in the ripple replay, so osr parser interpreted it as a regular string. The actual issue was with my reimplementation of the string parsing. I read an extra byte for regular strings when I didn't need to, so 0 length strings broke parsing while the ULEB128 string specified the size correctly to begin with.

MaxKruse commented 4 years ago

all good now!

abraker95 commented 4 years ago

I'll need to reopen this because my fix broke parsing for LeaF - I (Maddy) [Terror] replay_0.osr replay found in unit_tests/replays. I investigated, and I am almost convinced the ripple replay is corrupt. Almost because osu! can open it for reasons I am not sure.

I looked at the binary data once more. This is how it should be if timestamp is to be read correctly:

The pic shows replay hash to be 31 bytes long, however that's not what happens because replay hash is 32 bytes long as indicated by the 0x20. That makes everything shift forward one byte after replay hash

Data comparison. Top is ripple, bottom is osu! export: A seen when comparing to osu! exported replay which works, if trying to have timestamp read correctly in ripple replay, score data before it would be incorrectly read. So I can only conclude the replay file is corrupt.

MaxKruse commented 4 years ago

Assuming the sourecode of osu!ftw, a replay editor, ( http://en.pudn.com/Download/item/id/2893887.html replay reader file) it can both read normal and ripple replays. im short on time at this moment, but will try to give you a delta on what is different to your implementation

abraker95 commented 4 years ago

Ok that helped me figure some stuff out. The replay parser reads a shitbyte that determines whether to read life bar data or not. If it's 0x0B, a ULEB128 string, then it's read. If it's 0x00, then it's not read.

I also noticed that the Leaf - I replay I have in unit_tests has game version data zero'd out. I dunno if that's a version specific legacy specific thing, ppy's server side replay processing screwed up the file a bit, or something else, but since it can technically be read, I made an exception by reading another byte in life bar data parsing if game version is zero'd out and there is no life bar data.

I don't know if that exception will break on some other replay where game version is zero'd out, but I guess I'll find out when I come across another such case.

For now everything should be working.