iceiix / stevenarella

Multi-protocol Minecraft-compatible client written in Rust
Apache License 2.0
1.45k stars 59 forks source link

Favicon unzip thread panic while connecting to FTB beyond 1.11.0- Invalid byte (76, 10). #353

Closed skillman623 closed 4 years ago

skillman623 commented 4 years ago

I'm getting a panic and then the game locks up (sends to background cannot get to foreground again) The background still renders and spins, but the window cannot be selected from the taskbar.

thread '' panicked at 'called Result::unwrap() on an Err value: InvalidByte(76, 10)', src\screen\server_list.rs:269:40 stack backtrace: 0: backtrace::backtrace::trace_unsynchronized at C:\Users\VssAdministrator.cargo\registry\src\github.com-1ecc6299db9ec823\backtrace-0.3.46\src\backtrace\mod.rs:66 1: std::sys_common::backtrace::_print_fmt at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys_common\backtrace.rs:78 2: std::sys_common::backtrace::_print::{{impl}}::fmt at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys_common\backtrace.rs:59 3: core::fmt::write at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libcore\fmt\mod.rs:1069 4: std::io::Write::write_fmt at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\io\mod.rs:1504 5: std::sys_common::backtrace::_print at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys_common\backtrace.rs:62 6: std::sys_common::backtrace::print at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys_common\backtrace.rs:49 7: std::panicking::default_hook::{{closure}} at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\panicking.rs:198 8: std::panicking::default_hook at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\panicking.rs:218 9: std::panicking::rust_panic_with_hook at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\panicking.rs:511 10: std::panicking::begin_panic_handler at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\panicking.rs:419 11: core::panicking::panic_fmt at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libcore\panicking.rs:111 12: core::option::expect_none_failed at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libcore\option.rs:1268 13: core::result::Result<alloc::vec::Vec, base64::decode::DecodeError>::unwrap<alloc::vec::Vec,base64::decode::DecodeError> at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libcore\result.rs:1005 14: stevenarella::screen::server_list::{{impl}}::reload_server_list::{{closure}} at D:\Syncthing\Projects\Porting\stevenarella-forge113\src\screen\server_list.rs:269 note: Some details are omitted, run with RUST_BACKTRACE=full for a verbose backtrace.

Stevenarella_capture

If there is a way for me to enable full backtrace logging let me know and I can paste the full panic log.

This issue happens when compiling for debug on the latest commit (I compiled from source today).

iceiix commented 4 years ago

Crashing in src/screen/server_list.rs reload_server_list() decoding base64 data of the favicon image:

                        let favicon = if let Some(icon) = res.0.favicon {
                            let data_base64 = &icon["data:image/png;base64,".len()..];
                            let data = base64::decode(data_base64).unwrap(); // crash here
                            Some(image::load_from_memory(&data).unwrap())

https://docs.rs/base64/0.12.3/base64/enum.DecodeError.html

pub enum DecodeError {
    InvalidByte(usize, u8),

An invalid byte was found in the input. The offset and offending byte are provided.

In this case, offset 76 with the byte value 10 = newline.

I wonder if there is a \n at the end of the favicon that should be stripped. The code isn't very resilient against unexpected data, but it would be better to figure out why this occurs. Found this: https://wiki.vg/Server_List_Ping#Response

The favicon field is optional. The sample field must be set, but can be empty.

The favicon should be a PNG image that is Base64 encoded (without newlines: \n, new lines no longer work since 1.13) and prepended with data:image/png;base64,.

so I should strip newlines here before base64 decoding, for pre-1.13 compatibility

skillman623 commented 4 years ago

After changing the opt level to 3, the client will now connect. However, I'm still receiving the thread panic.

thread '' panicked at 'called Result::unwrap() on an Err value: InvalidByte(76, 10)', src\screen\server_list.rs:273:40 stack backtrace:

Icon loads

Icon does not load

skillman623 commented 4 years ago

Retesting with latest commit.

skillman623 commented 4 years ago

Okay I'm getting different thread panics this time For hypixel It seems like the protocol isn't implemented.

thread '' panicked at 'not implemented', protocol\src\format.rs:360:38 stack backtrace: 0: backtrace::backtrace::trace_unsynchronized at C:\Users\VssAdministrator.cargo\registry\src\github.com-1ecc6299db9ec823\backtrace-0.3.46\src\backtrace\mod.rs:66 1: std::sys_common::backtrace::_print_fmt at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys_common\backtrace.rs:78 2: std::sys_common::backtrace::_print::{{impl}}::fmt at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys_common\backtrace.rs:59 3: core::fmt::write at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libcore\fmt\mod.rs:1069 4: std::io::Write::write_fmt at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\io\mod.rs:1504 5: std::sys_common::backtrace::_print at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys_common\backtrace.rs:62 6: std::sys_common::backtrace::print at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys_common\backtrace.rs:49 7: std::panicking::default_hook::{{closure}} at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\panicking.rs:198 8: std::panicking::default_hook at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\panicking.rs:218 9: std::panicking::rust_panic_with_hook at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\panicking.rs:511 10: std::panicking::begin_panic<str*> at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libstd\panicking.rs:438 11: steven_protocol::format::convert_legacy at D:\Github\stevenarella\protocol\src\format.rs:360 12: stevenarella::screen::server_list::{{impl}}::reload_server_list::{{closure}} at D:\Github\stevenarella\src\screen\server_list.rs:266

For beyond I'm getting the same error as last time

thread '' panicked at 'called Result::unwrap() on an Err value: InvalidByte(76, 10)', src\screen\server_list.rs:269:40 stack backtrace: 0: backtrace::backtrace::trace_unsynchronized at C:\Users\VssAdministrator.cargo\registry\src\github.com-1ecc6299db9ec823\backtrace-0.3.46\src\backtrace\mod.rs:66 1: std::sys_common::backtrace::_print_fmt at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys_common\backtrace.rs:78 2: std::sys_common::backtrace::_print::{{impl}}::fmt at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys_common\backtrace.rs:59 3: core::fmt::write at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libcore\fmt\mod.rs:1069 4: std::io::Write::write_fmt at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\io\mod.rs:1504 5: std::sys_common::backtrace::_print at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys_common\backtrace.rs:62 6: std::sys_common::backtrace::print at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys_common\backtrace.rs:49 7: std::panicking::default_hook::{{closure}} at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\panicking.rs:198 8: std::panicking::default_hook at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\panicking.rs:218 9: std::panicking::rust_panic_with_hook at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\panicking.rs:511 10: std::panicking::begin_panic_handler at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\panicking.rs:419 11: core::panicking::panic_fmt at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libcore\panicking.rs:111 12: core::option::expect_none_failed at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libcore\option.rs:1268 13: core::result::Result<alloc::vec::Vec, base64::decode::DecodeError>::unwrap at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libcore\result.rs:1005 14: stevenarella::screen::server_list::{{impl}}::reload_server_list::{{closure}} at D:\Github\stevenarella\src\screen\server_list.rs:269

for the Vanilla server I get the same error as well

thread '' panicked at 'called Result::unwrap() on an Err value: InvalidByte(76, 10)', src\screen\server_list.rs:269:40 stack backtrace: 0: backtrace::backtrace::trace_unsynchronized at C:\Users\VssAdministrator.cargo\registry\src\github.com-1ecc6299db9ec823\backtrace-0.3.46\src\backtrace\mod.rs:66 1: std::sys_common::backtrace::_print_fmt at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys_common\backtrace.rs:78 2: std::sys_common::backtrace::_print::{{impl}}::fmt at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys_common\backtrace.rs:59 3: core::fmt::write at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libcore\fmt\mod.rs:1069 4: std::io::Write::write_fmt at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\io\mod.rs:1504 5: std::sys_common::backtrace::_print at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys_common\backtrace.rs:62 6: std::sys_common::backtrace::print at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys_common\backtrace.rs:49 7: std::panicking::default_hook::{{closure}} at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\panicking.rs:198 8: std::panicking::default_hook at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\panicking.rs:218 9: std::panicking::rust_panic_with_hook at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\panicking.rs:511 10: std::panicking::begin_panic_handler at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\panicking.rs:419 11: core::panicking::panic_fmt at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libcore\panicking.rs:111 12: core::option::expect_none_failed at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libcore\option.rs:1268 13: core::result::Result<alloc::vec::Vec, base64::decode::DecodeError>::unwrap at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libcore\result.rs:1005 14: stevenarella::screen::server_list::{{impl}}::reload_server_list::{{closure}} at D:\Github\stevenarella\src\screen\server_list.rs:269 note: Some details are omitted, run with RUST_BACKTRACE=full for a verbose backtrace.

skillman623 commented 4 years ago

Here are the icons that I'm using if it helps. FTB beyond server-icon Vanilla server-icon

iceiix commented 4 years ago

Thanks for the additional info. Testing with FTB Beyond I can repro the DecodeError but it is fixed with #354 and the icon FTB icon appears:

Screen Shot 2020-07-03 at 4 07 29 PM

Just merged this change into master, so InvalidByte(76, 10) should no longer occur.

However there are other errors, mc.hypixel.net: thread '' panicked at 'not implemented', protocol/src/format.rs:360:38

This is in the convert_legacy color decoding routine, for §-prefixed color codes:

                            match color_char {
                                '0' => modifier.color = Some(Color::Black),
                                '1' => modifier.color = Some(Color::DarkBlue),
                                '2' => modifier.color = Some(Color::DarkGreen),
                                '3' => modifier.color = Some(Color::DarkAqua),
                                '4' => modifier.color = Some(Color::DarkRed),
                                '5' => modifier.color = Some(Color::DarkPurple),
                                '6' => modifier.color = Some(Color::Gold),
                                '7' => modifier.color = Some(Color::Gray),
                                '8' => modifier.color = Some(Color::DarkGray),
                                '9' => modifier.color = Some(Color::Blue),
                                'a' => modifier.color = Some(Color::Green),
                                'b' => modifier.color = Some(Color::Aqua),
                                'c' => modifier.color = Some(Color::Red),
                                'd' => modifier.color = Some(Color::LightPurple),
                                'e' => modifier.color = Some(Color::Yellow),
                                'f' => modifier.color = Some(Color::White),
                                'k' => modifier.obfuscated = Some(true),
                                'l' => modifier.bold = Some(true),
                                'm' => modifier.strikethrough = Some(true),
                                'n' => modifier.underlined = Some(true),
                                'o' => modifier.italic = Some(true),
                                'r' => {}
                                _ => unimplemented!(), // crash here

documented on https://wiki.vg/Chat#Control_Sequences, all are supported. The server status text is:

§aHypixel Network  §c[1.8-1.16]
             §e§lSUMMER UPDATE §f§e§ §e§lSALE

"§ " (the escape character followed by space) isn't recognized, would probably be a good idea to handle this situation more gracefully (ignoring instead of panicing unimplemented)

iceiix commented 4 years ago

@skillman623 can you retest with latest master, at least as of this commit: https://github.com/iceiix/stevenarella/commit/eb703f077619b7e58bcf072a055d058b8adda05e - I believe both issues should be fixed (but there may be more)

skillman623 commented 4 years ago

Issue is resolved. Stevenarella_Server_Icon It seems that the remove server button no longer works. I'll open a new issue for that.