gate icon indicating copy to clipboard operation
gate copied to clipboard

bug: "command not found" error with legacy clients

Open NaymDev opened this issue 10 months ago • 1 comments

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, ...

NaymDev avatar Jun 21 '25 05:06 NaymDev

PS: I'd like to make a fix and PR for that and I will do so very soon

NaymDev avatar Jun 21 '25 05:06 NaymDev

🎉 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 handleLegacyCommand was indeed passing the full message (with /) to queueCommandResult
  • This caused the double slash issue (//help) when forwarding
  • The command parser was looking for /help instead of help

The fix:

  • Strips the leading / using strings.TrimPrefix before 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! 👏

robinbraemer avatar Sep 04 '25 11:09 robinbraemer