bug: "command not found" error with legacy clients
When using a legacy client, chat messages starting with a slash (e.g. /glist) are interpreted as commands. However the reverse proxy doesn't strip the leading / causing the command parser to misinterpret the command which ends up just forwarding the command to the Minecraft server even if it should be handled by the proxy.
if entered /help...
if e.Forward() {
return (&chat.Builder{
Protocol: c.player.Protocol(),
Message: "/" + commandToRun,
Sender: c.player.ID(),
}).ToServer()
}
...this would result in //help being forwarded to the Minecraft server.
if !hasRun {
return (&chat.Builder{
Protocol: c.player.Protocol(),
Message: packet.Message,
Sender: c.player.ID(),
}).ToServer()
}
...this would result in /help being forwarded to the Minecraft server.
Maybe I just don't understand this and it is expected to run like that, but packet.Message and commandToRun are the same.
// Command returns the whole commandline without the leading "/".
func (c *CommandExecuteEvent) Command() string {
return c.commandline
}
This does not work as expected for legacy! The reason is a wrong assumption
func (c *chatHandler) queueCommandResult(
message string,
timestamp time.Time,
lastSeenMessages *chat.LastSeenMessages,
packetCreator func(event *CommandExecuteEvent, lastSeenMessages *chat.LastSeenMessages) proto.Packet,
) {
cmd := message
e := &CommandExecuteEvent{
source: c.player,
commandline: cmd,
originalCommand: cmd,
}
c.eventMgr.Fire(e)
The command is set to the message. This assumes that the first argument passed to the queueCommandResult is the command without the leading / NOT the message! In all other places queueCommandResult is called like this: c.queueCommandResult(packet.Command, ...
PS: I'd like to make a fix and PR for that and I will do so very soon
🎉 Issue Fixed!
Hey @NaymDev!
Your excellent analysis was absolutely spot-on! I've created PR #556 that implements the fix for this legacy command issue.
What you identified was 100% correct:
- The
handleLegacyCommandwas indeed passing the full message (with/) toqueueCommandResult - This caused the double slash issue (
//help) when forwarding - The command parser was looking for
/helpinstead ofhelp
The fix:
- Strips the leading
/usingstrings.TrimPrefixbefore processing (exactly like Velocity does) - Adds comprehensive tests to prevent regression
- Maintains perfect alignment with Velocity's reference implementation
Thanks for taking the time to thoroughly investigate and document this issue. Your detailed analysis made the fix straightforward to implement! 🚀
The PR has been merged and this issue is now fixed. Great catch! 👏