Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent invalid status responses with empty/dummy player UUIDs #3831

Merged
merged 1 commit into from Apr 12, 2023

Conversation

stephan-gh
Copy link
Contributor

Right now SpongeGameProfile.toMcProfile() converts all SpongeAPI GameProfiles with empty UUIDs (= new UUID(0, 0)) to authlib GameProfiles with a null UUID.

This is fine in most cases but problematic for the sample/player list in the status response. Empty UUIDs can be used there to create dummy entries, to display custom messages in the server list instead of the online players. (See e.g. https://github.com/Minecrell/ServerListPlus/wiki/Status-Configuration#player-hover-messages)

The Minecraft client completely rejects status responses that contain GameProfiles with null UUIDs and/or null names. This is why it is important that the empty UUID is not converted to a null UUID.

[Client thread/ERROR]: Can't ping localhost: Internal Exception: io.netty.handler.codec.DecoderException: java.lang.IllegalArgumentException: Invalid UUID string:

Prevent sending invalid responses to the client by making sure both UUID and name are non-null in the player samples of the status response.

This fixes a regression in API 8 when using the ServerListPlus plugin on Sponge. The problem was likely introduced in some GameProfile refactoring (API 7 worked fine).

There is similar code in Paper to avoid this problem: https://github.com/PaperMC/Paper/blob/ad3e1bc12104f1f8ea8bcf9248a606133bfc7780/patches/server/0182-Implement-extended-PaperServerListPingEvent.patch#L122-L128

I would appreciate if you could pick/merge this into all newer branches as well, not sure how Sponge development works nowadays :)

stephan-gh added a commit to Minecrell/ServerListPlus that referenced this pull request Feb 27, 2023
Prefer fixing this in Sponge instead: SpongePowered/Sponge#3831
Use the previous SLP build on Sponge until this is merged.
Right now SpongeGameProfile.toMcProfile() converts all SpongeAPI GameProfiles
with empty UUIDs (= new UUID(0, 0)) to authlib GameProfiles with a null UUID.

This is fine in most cases but problematic for the sample/player list in
the status response. Empty UUIDs can be used there to create dummy entries,
to display custom messages in the server list instead of the online players.
(See e.g. https://github.com/Minecrell/ServerListPlus/wiki/Status-Configuration#player-hover-messages)

The Minecraft client completely rejects status responses that contain
GameProfiles with null UUIDs and/or null names. This is why it is
important that the empty UUID is not converted to a null UUID.

Prevent sending invalid responses to the client by making sure both
UUID and name are non-null in the player samples of the status response.

This fixes a regression in API 8 when using the ServerListPlus plugin
on Sponge. The problem was likely introduced in some GameProfile refactoring
(API 7 worked fine).
@ImMorpheus ImMorpheus added type: bug Something isn't working version: 1.16 (u) API: 8 labels Apr 2, 2023
@ImMorpheus ImMorpheus merged commit 8fbc8c9 into SpongePowered:api-8 Apr 12, 2023
@ImMorpheus
Copy link
Contributor

ImMorpheus commented Apr 12, 2023

Thanks!

I've merged your changes in the other branches but afaics api 10 doesn't seem to be affected. The mojang->sponge->mojang gameprofile conversion is gone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working version: 1.16 (u) API: 8
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants