PaperMC / Paper

The most widely used, high performance Minecraft server that aims to fix gameplay and mechanics inconsistencies
https://papermc.io/
Other
9.9k stars 2.29k forks source link

Invalid move player packet received #8222

Closed CrazyWillBear closed 2 years ago

CrazyWillBear commented 2 years ago

Expected behavior

To join the server and not get kicked right away for "Invalid move player packet received"

Observed/Actual behavior

Sometimes, when players join, they get a "Invalid move player packet received". Apparently other servers on 1.19~ are experiencing the same thing.

Steps/models to reproduce

Just start a 1.19.1 Paper server, and connect to it several times until you get the error. (To best reproduce: get in a boat, then disconnect and reconnect without leaving the boat; disconnect while standing on top of a boat).

Plugin and Datapack List

Plugins (18): Announcement, BungeeGuard, ChatCo, CommandWhitelist, DeathMessages, GrimAC, Kill, LuckPerms, MyCommand, OpenWorldStats, PlaceholderAPI, ProtocolLib, ServerSidePatches, TabList, ViaBackwards, ViaVersion, WelcomeMsg, WorldEdit

Paper version

This server is running Paper version git-Paper-101 (MC: 1.19.1) (Implementing API version 1.19.1-R0.1-SNAPSHOT) (Git: ceef4b9) You are running the latest version Previous version: git-Paper-86 (MC: 1.19.1)

Other

When I use GrimAC, it happens more often, however without any plugins I still get the error.

jonaas13 commented 2 years ago

Same issues here. I'm running a bungeecord(waterfall) network. What's weird is that on some of my servers it does not seems to happen. The hub is completely broken atm, skyblock server is partly broken and doesn't let bedrock clients join (geyser+floodgate), minigames server, kitpvp and smp seems to be working fine. KitPVP and SMP were created on 1.19, although minigames was created on 1.18. Hub and RPG are older. They share mostly the same plugins although some plugins unique ones. Couldn't replicate it unless I copied my existing plugin data. I've been stuck on this for around 1.5 week now.

MWHunter commented 2 years ago

When I use GrimAC, it happens more often, however without any plugins I still get the error.

Grim does send packet teleports which might cause some issues. By itself, there shouldn't be issues as grim sends only negative teleport ID's, which won't conflict with the vanilla range of 0 to infinity. The kick occurs when a plugin, or something, sends the same teleport ID twice in a row.

CrazyWillBear commented 2 years ago

The kick occurs when a plugin, or something, sends the same teleport ID twice in a row.

The issue happens with or without Grim, but Grim just increases the likelihood of the error happening. I don't have any other plugins that teleport players upon spawn or that would.

jonaas13 commented 2 years ago

Could have something to do with the Teleportation API update?https://github.com/PaperMC/Paper/commit/5deafd1969b3c521af9cabb46822ecb01e6ce498#diff-5111166204f17a48d89f6e4688c9b0e8e5ccf85351e6f10affd4a586c70049c2

It's around the same time problems started appearing, I think plugins have some influence but I don't think they're the culprit. Although I'm not sure if it's something to be fixed inside paper or inside the plugins.

CrazyWillBear commented 2 years ago

So I think I may have fixed it or at least DRASTICALLY reduced the amount of times it occurs (it hasn't happened to me). I set up my BungeeCord proxy to wait 1 second before teleporting players between servers. For some reason this is working even though it shouldn't be a problem. I also uninstalled ProtocolLib but I don't think that was related to the problem, just giving all info I can.

jonaas13 commented 2 years ago

So I think I may have fixed it or at least DRASTICALLY reduced the amount of times it occurs (it hasn't happened to me). I set up my BungeeCord proxy to wait 1 second before teleporting players between servers. For some reason this is working even though it shouldn't be a problem. I also uninstalled ProtocolLib but I don't think that was related to the problem, just giving all info I can.

I agree with this. I found on the skyblock server that when you would wait 1-5 seconds before using /warp /tp /spawn etc it has less chance of kicking you although it still can occur. Protocollib

CrazyWillBear commented 2 years ago

Yea so although this may not be an issue with Paper, I'm gonna leave the issue open so the devs can make sure before closing.

jonaas13 commented 2 years ago

For me it was an issue with the mirrorskin function of Denizen. Contacted creator of denizen, he said it was an issue with spigot or possibly bungeecord. Please re-open issue.

CrazyWillBear commented 2 years ago

Sorry had to reopen, it is definitely a possible issue with Paper or at least the server jar itself and requires a patch. I haven't had this happen on any other version (1.19.x) and through testing it's either an issue with the server jar or BungeeCord.

MWHunter commented 2 years ago

Yeah it’s a new 1.19 feature by the wonderful netcode developers from Mojang. Unsure what exactly this feature solves as ignoring the invalid teleport accept can be done as nothing bad will happen.

electronicboy commented 2 years ago

if a plugin is inducing this, this would pretty much 100% be on them to resolve it or at least generally diagnose what is going on, they're at least sending mangled data somewhere; Only concern I have over this limitation is down to proxies and the potential risks around server switching, otherwise, not our issue

mcmonkey4eva commented 2 years ago

(re electronicboy above) It's been blamed on multiple separate plugins that I've seen, and not in any clearly overlapping way. The user above that mentions Denizen, normally we would assume Denizen is at fault because Denizen mucks with networking internals, but most of the other reports don't even have Denizen at all, so seems unlikely to actually be Denizen at fault. I've also seen it blamed on Citizens a bunch, but there's no logical link at all between Citizens and this bug, and there have been several reports that don't involve Citizens at all. The initial few reporters in this thread aren't running any plugins I maintain, so I don't think there's much I can do to track it down myself at least.

Also for what it helps: I've seen reports on both 1.19(.0) and 1.19.1, and have seen both Paper and Purpur. To my awareness I haven't seen it on base Spigot offhand, but also nobody runs Spigot 1.19.x so who knows.


EDIT: Also have had a user report that the issue goes away when they remove Depenizen from their server - Depenizen is an addon to Denizen that doesn't do much in itself other than add a massive dependency list (it provides links between Denizen scripts and those plugins) which might imply the reason the random plugins getting blamed isn't actually those plugins - rather it's some effect on the plugin load order caused by altering the dependency list. Maybe there's a circular dependency somewhere that relates to the issue?

electronicboy commented 2 years ago

Theres no real changes to the logic that I can see that would induce that, I did shove out a build which would log what was going on, and like, at least within everything I could see to realistically mutate that value, no calls where made, yet the field was being set as expected, it would just randomly be null at some point;

ofc, testing this is going to be a headache, I know billy tested somebody else's server download and couldn't reproduce it, so at least to me it's clearly environmentally induced, not sure if there is a sane way to monitor all attempts to mutate a field in a reobf'd server; as I said somewhere else, if somebody is able to manually build a jar, rename the await teleportation pos field, that should break the reobfuscation of that field and anything screwing with it should show up

jonaas13 commented 2 years ago

I was able to reproduce it with 1.19/1.19.1 + essentials+spawn+chat, citizens, denizen. Not sure if essentials had any influence haven't tested without. Create an npc, any name/skin. (Citizens) Add /mirrorskin and/or /mirrorname (denizen) I got kicked almost immediately "invalid player move packet received" Npc was close to spawn if it matters.

I've also seen reports of the "Images" plugin and others so it may be a conflict between plugins or something that's changed somewhere in the server(.jar) builds

PaintsplattersMC commented 2 years ago

I have opened this issue on citizens github. I cannot reproduce this error with only 1 of either citizens or denizen installed.

seems the 2 cause some kind of conflict

Maitlans commented 2 years ago

Issue only happens on my end when Denizen and Citizens2 is installed on the server, once Denizen is removed everything works fine.

OnionsBoot commented 2 years ago

Hi, I'm getting the Invalid move player packet received kick issue as well.

I do not run and never have ran Denizen. I run Citizens2, FastAsyncWorldEdit, BetonQuest, EssentialsX+Spawn+Chat, Worldguard, Lands, on Purpur.

The issue only happens a couple time a day, when players use the essentialsX /warp command to warp to a specific location. This location is protected by Lands claims (which doesn't have any dependencies) and was loaded in via FastAsyncWorldEdit (this is why I mention FastAsyncWorldEdit, just in case there's some overlap in a dependency or something FAWE uses that might be used in Denizen or Citizens as well?). The players are kicked and given the "Invalid Move Player Packet Received" message. They are able to rejoin immediately.

So I don't think this is a Denizen specific issue. But Citizens2 and EssentialsX are definitely present here.

Additional (maybe useless) info - I had GrimAC installed in the past. It's been uninstalled for a while now, though.

mcmonkey4eva commented 2 years ago

Update from investigation of the Denizen side of things: the reason servers with Denizen have this issue has been traced: https://github.com/CitizensDev/Citizens2/issues/2836#issuecomment-1207156851

Servers not running Denizen are likely running into other plugins that perform similar network interception to Denizen.

tl;dr of the keymost point: ServerGamePacketListenerImpl#internalTeleport is modified by Paper from private to public, which is breakingly incompatible with standard network intercept tools based on Spigot. The options are A: Build plugins to require Paper (not a valid option for me until an actual full hardfork happens) B: Paper removes the incompatible change (from past experience yall will probably refuse) C: Build an alternative network intercept toolkit, potentially based on dynamic codegen. I'm already going to look into this for Denizen purposes, but all other relevant plugins would have to correct for this as well.

Also if you check the issue post linked above, I detailed some other reasons plugins might cause the error, and a spot even Paper's own API might.


EDIT: I have implemented option C, dynamic codegen... the code for which is utterly cursed but it works! https://github.com/DenizenScript/Denizen/blob/dev/plugin/src/main/java/com/denizenscript/denizen/utilities/packets/NetworkInterceptCodeGen.java

The portion of this issue sourced from Denizen is now resolved. Replications not involving Denizen are still to-be-determined, depending on how the Paper team and/or relevant plugin devs feel about the tracing information I provided above and the recommended 3 options.

siulung201314 commented 2 years ago

I'm getting the Invalid move player packet received kick issue too. I run Citizens2, CMI and Reisdence on Purpur. When players use the teleport commands (eg. /back, /res tp), they may be kicked randomly and given the "Invalid Move Player Packet Received" message. However, they can rejoin immediately after being kicking. The issue only happens after updating the server from 1.18.2 to 1.19.

Owen1212055 commented 2 years ago

Seeing that nobody has experienced this issue in a while and that the underlying issue has been fixed in Denizen (used with Citizens it seems) this issue will be closed.

However, if you are still seeing this issue without any plugins please feel free to comment and this issue can be reopened. Thank you! 😄