Dinnerbone / mcstatus

A Python class for checking the status of an enabled Minecraft server
http://dinnerbone.com/minecraft/tools/status/
1.11k stars 146 forks source link

Key existence for COLOR_MAP is not being checked #194

Closed humayunj closed 2 years ago

humayunj commented 2 years ago

Mcstatus is not able to parse the response from play.creativefun.net. I haven't analyzed deeply but when pinging, OSError: Received invalid ping response packet. exception is raised and when using status parameter from cli OSError: Server did not respond with any information! is raised. Server does respond but mcstatus is not able to parse it.

Using mcstatus 8.0.0 and python 3.9.2

To reproduce mcstatus play.creativefun.net ping

Edit:

After the little bit of digging, setting tries = 1 for status I discovered that the cause is the KeyError exception in pinger.py around line 246:

if entry.get("color"): description += "§" + COLOR_MAP[entry["color"]] The value for entry["color"] was #b3eeff but the COLOR_MAP doesn't have this key, the code doesn't check for the key existence, nor assign the default value.

kevinkjt2000 commented 2 years ago

I did a pre-release a moment ago 9.0.0.dev1 that includes a fix. Please give it a whirl :lollipop:

ItsDrike commented 2 years ago

I did a pre-release a moment ago 9.0.0.dev1 that includes a fix. Please give it a whirl lollipop

While the cleanup PR introduced a new logic for color parsing and this wouldn't be an issue in it anymore, is that really what we want? In the new logic, we simply ignore all colors which weren't defined in the mapping, but that may be a bit weird and an exception may actually make more sense if the color is invalid, that said, the exception should be clear about what went wrong, the original exception raised here obviously wasn't very helpful.

kevinkjt2000 commented 2 years ago

For any who come across this later, #204 is tracking that last comment since it will overhaul styling outputs