ppy / osu

rhythm is just a *click* away!
https://osu.ppy.sh
MIT License
15.38k stars 2.29k forks source link

Crash while viewing changelogs on the "lazer" stream #13993

Closed RoboMico closed 3 years ago

RoboMico commented 3 years ago

Describe the bug:

  1. Open osu!, click Changelog on the top bar;

  2. Go to the "Lazer" version;

  3. Wait a few seconds and game crashes.

Screenshots or videos showing encountered issue:

https://user-images.githubusercontent.com/59791306/126767466-021c7825-9bf2-445e-b9c4-a82776cb5609.mp4

Sorry that I can't record audio due to the device limitation.

osu!lazer version:

2021.720.0-lazer, running on MagicUI 3.1.1 (based on Android 10) using Honor ViewPad 6.

Logs:

Logs.zip

peppy commented 3 years ago

Broken by the following comment content:

My tablet doesn't work :(
It's a Huion 420 and it's apparently incompatible with OpenTablet Driver. The warning I get is: "DeviceInUseException: Device is currently in use by another kernel module. To fix this issue, please follow the instructions from https://github.com/OpenTabletDriver/OpenTabletDriver/wiki/Linux-FAQ#arg    umentoutofrangeexception-value-0-15" and it repeats 4 times on the notification before logging subsequent warnings. 
Checking the logs, it looks for other Huion tablets before sending the notification (e.g.
 "2021-07-23 03:52:33 [verbose]: Detect: Searching for tablet 'Huion WH1409 V2'
 20 2021-07-23 03:52:33 [error]: DeviceInUseException: Device is currently in use by another kernel module. To fix this     issue, please follow the instructions from https://github.com/OpenTabletDriver/OpenTabletDriver/wiki/Linux-FAQ#arg    umentoutofrangeexception-value-0-15") 
I use an Arch based installation of Linux and the tablet runs perfectly with Digimend kernel driver, with area configuration, pen pressure, etc. On osu!lazer the cursor disappears until I set it to "Borderless" instead of "Fullscreen" and even after it shows up, it goes to the bottom left corner as soon as a map starts. 
I have honestly 0 idea of whats going on at this point.

Fault is either in MessageFormatter.FormatText or LinkFlowContainer.AddLinks. Will require a framework side fix.

I've hidden the comment for now to alleviate the crashes.

bdach commented 3 years ago

The issue is caused by the pasted log lines. In particular, the log timestamps (03:52:33) are being parsed as editor timestamps. And the timestamp regex is quite greedy - it'll consume the rest of the line as long as there's no dash in it:

https://github.com/ppy/osu/blob/8eb42614060bee5e0ec217bf24485b516b8f96f1/osu.Game/Online/Chat/MessageFormatter.cs#L45-L46

The crash happens because that line has a nested link in it and LinkFlowContainer obviously fails to deal with links nested in links (but I don't fault it for not being able to).

I'd say this is a blatant misuse of MessageFormatter - it was intended for chat and should not be used to parse comments for links. The way forward is probably actually using the markdown representation of comments from web?

leomick commented 3 years ago

I've done the same thing on fireos (based on android) and windows 10 and it did not crash for me

bdach commented 3 years ago

That's because the comment in question was hidden. Please read this thread.

smoogipoo commented 3 years ago

Using markdown sounds like the way to go. Maybe the recent wiki markdown structures can be used with relative ease.

gagahpangeran commented 3 years ago

There's slightly different behaviour for image rendering in wiki page and comment markdown. The image in comment has this minimum and maximum height value in web.

https://github.com/ppy/osu-web/blob/6dc5188ef1a15ca90b53500aa883c01cfe2956a2/resources/assets/less/bem/osu-md.less#L31-L35

Maybe I can work on this and hide the image for the meantime.

smoogipoo commented 3 years ago

The bigger issue is that the API doesn't yet provide the message in markdown format. I mentioned in #osu-web but also https://github.com/ppy/osu-web/issues/7922.

bdach commented 3 years ago

I'm slightly confused. The string that caused the crash was in a user comment, and not a changelog entry...?

smoogipoo commented 3 years ago

You're right, I misread.

gagahpangeran commented 3 years ago

@smoogipoo are you working on this?

If not, I can work on this and open the PR tonight.

smoogipoo commented 3 years ago

Go for it.