Make geyser connectiontest command more foolproof (#3951)

Co-authored-by: Camotoy <20743703+Camotoy@users.noreply.github.com>

core/src/main/java/org/geysermc/geyser/command/defaults/ConnectionTestCommand.java

@@ -26,8 +26,8 @@
 package org.geysermc.geyser.command.defaults;
 
 import com.fasterxml.jackson.databind.JsonNode;
-import org.geysermc.geyser.api.util.PlatformType;
 import org.geysermc.geyser.GeyserImpl;
+import org.geysermc.geyser.api.util.PlatformType;
 import org.geysermc.geyser.command.GeyserCommand;
 import org.geysermc.geyser.command.GeyserCommandSource;
 import org.geysermc.geyser.session.GeyserSession;
@@ -36,11 +36,20 @@ import org.geysermc.geyser.util.LoopbackUtil;
 import org.geysermc.geyser.util.WebUtils;
 import org.jetbrains.annotations.Nullable;
 
+import java.util.Random;
 import java.util.concurrent.CompletableFuture;
 
 public class ConnectionTestCommand extends GeyserCommand {
+    /*
+     * The MOTD is temporarily changed during the connection test.
+     * This allows us to check if we are pinging the correct Geyser instance
+     */
+    public static String CONNECTION_TEST_MOTD = null;
+
     private final GeyserImpl geyser;
 
+    private final Random random = new Random();
+
     public ConnectionTestCommand(GeyserImpl geyser, String name, String description, String permission) {
         super(name, description, permission);
         this.geyser = geyser;
@@ -55,23 +64,61 @@ public class ConnectionTestCommand extends GeyserCommand {
         }
 
         if (args.length == 0) {
-            sender.sendMessage("Provide the Bedrock server IP you are trying to connect with. Example: `test.geysermc.org:19132`");
+            sender.sendMessage("Provide the server IP and port you are trying to test Bedrock connections for. Example: `test.geysermc.org:19132`");
             return;
         }
 
+        // Replace "<" and ">" symbols if they are present to avoid the common issue of people including them
+        String[] fullAddress = args[0].replace("<", "").replace(">", "").split(":", 2);
+
         // Still allow people to not supply a port and fallback to 19132
-        String[] fullAddress = args[0].split(":", 2);
         int port;
         if (fullAddress.length == 2) {
+            try {
                 port = Integer.parseInt(fullAddress[1]);
+            } catch (NumberFormatException e) {
+                // can occur if e.g. "/geyser connectiontest <ip>:<port> is ran
+                sender.sendMessage("Not a valid port! Specify a valid numeric port.");
+                return;
+            }
         } else {
             port = 19132;
         }
+        String ip = fullAddress[0];
+
+        // Issue: people commonly checking placeholders
+        if (ip.equals("ip")) {
+            sender.sendMessage(ip + " is not a valid IP, and instead a placeholder. Please specify the IP to check.");
+            return;
+        }
+
+        // Issue: checking 0.0.0.0 won't work
+        if (ip.equals("0.0.0.0")) {
+            sender.sendMessage("Please specify the IP that you would connect with. 0.0.0.0 in the config tells Geyser to the listen on the server's IPv4.");
+            return;
+        }
+
+        // Issue: people testing local ip
+        if (ip.equals("localhost") || ip.startsWith("127.") || ip.startsWith("10.") || ip.startsWith("192.168.")) {
+            sender.sendMessage("This tool checks if connections from other networks are possible, so you cannot check a local IP.");
+            return;
+        }
 
         // Issue: do the ports not line up?
         if (port != geyser.getConfig().getBedrock().port()) {
-            sender.sendMessage("The port you supplied (" + port + ") does not match the port supplied in Geyser's configuration ("
+            if (fullAddress.length == 2) {
-                    + geyser.getConfig().getBedrock().port() + "). You can change it under `bedrock` `port`.");
+                sender.sendMessage("The port you are testing with (" + port + ") is not the same as you set in your Geyser configuration ("
+                    + geyser.getConfig().getBedrock().port() + ")");
+                sender.sendMessage("Re-run the command with the port in the config, or change the `bedrock` `port` in the config.");
+                if (geyser.getConfig().getBedrock().isCloneRemotePort()) {
+                    sender.sendMessage("You have `clone-remote-port` enabled. This option ignores the `bedrock` `port` in the config, and uses the Java server port instead.");
+                }
+            } else {
+                sender.sendMessage("You did not specify the port to check (add it with \":<port>\"), " +
+                        "and the default port 19132 does not match the port in your Geyser configuration ("
+                        + geyser.getConfig().getBedrock().port() + ")!");
+                sender.sendMessage("Re-run the command with that port, or change the port in the config under `bedrock` `port`.");
+            }
         }
 
         // Issue: is the `bedrock` `address` in the config different?
@@ -79,7 +126,7 @@ public class ConnectionTestCommand extends GeyserCommand {
             sender.sendMessage("The address specified in `bedrock` `address` is not \"0.0.0.0\" - this may cause issues unless this is deliberate and intentional.");
         }
 
-        // Issue: did someone turn on enable-proxy-protocol and they didn't mean it?
+        // Issue: did someone turn on enable-proxy-protocol, and they didn't mean it?
         if (geyser.getConfig().getBedrock().isEnableProxyProtocol()) {
             sender.sendMessage("You have the `enable-proxy-protocol` setting enabled. " +
                     "Unless you're deliberately using additional software that REQUIRES this setting, you may not need it enabled.");
@@ -88,7 +135,6 @@ public class ConnectionTestCommand extends GeyserCommand {
         CompletableFuture.runAsync(() -> {
             try {
                 // Issue: SRV record?
-                String ip = fullAddress[0];
                 String[] record = WebUtils.findSrvRecord(geyser, ip);
                 if (record != null && !ip.equals(record[3]) && !record[2].equals(String.valueOf(port))) {
                     sender.sendMessage("Bedrock Edition does not support SRV records. Try connecting to your server using the address " + record[3] + " and the port " + record[2]
@@ -102,20 +148,50 @@ public class ConnectionTestCommand extends GeyserCommand {
                             "See here for steps on how to resolve: " + "https://wiki.geysermc.org/geyser/fixing-unable-to-connect-to-world/#using-geyser-on-the-same-computer");
                 }
 
-                // mcsrvstatus will likely be replaced in the future with our own service where we can also test
+                // Generate some random, unique bits that another server wouldn't provide
-                // around the OVH workaround without worrying about caching
+                byte[] randomBytes = new byte[2];
-                JsonNode output = WebUtils.getJson("https://api.mcsrvstat.us/bedrock/2/" + args[0]);
+                this.random.nextBytes(randomBytes);
+                StringBuilder randomStr = new StringBuilder();
+                for (byte b : randomBytes) {
+                    randomStr.append(Integer.toHexString(b));
+                }
+                String connectionTestMotd = "Geyser Connection Test " + randomStr;
+                CONNECTION_TEST_MOTD = connectionTestMotd;
 
-                long cacheTime = output.get("debug").get("cachetime").asLong();
+                sender.sendMessage("Testing server connection now. Please wait...");
-                String when;
+                JsonNode output;
-                if (cacheTime == 0) {
+                try {
-                    when = "now";
+                    output = WebUtils.getJson("https://checker.geysermc.org/ping?hostname=" + ip + "&port=" + port);
-                } else {
+                } finally {
-                    when = ((System.currentTimeMillis() / 1000L) - cacheTime) + " seconds ago";
+                    CONNECTION_TEST_MOTD = null;
                 }
 
-                if (output.get("online").asBoolean()) {
+                JsonNode cache = output.get("cache");
-                    sender.sendMessage("Your server is likely online as of " + when + "!");
+                String when;
+                if (cache.get("fromCache").asBoolean()) {
+                    when = cache.get("secondsSince").asInt() + " seconds ago";
+                } else {
+                    when = "now";
+                }
+
+                if (output.get("success").asBoolean()) {
+                    JsonNode ping = output.get("ping");
+                    JsonNode pong = ping.get("pong");
+                    String remoteMotd = pong.get("motd").asText();
+                    if (!connectionTestMotd.equals(remoteMotd)) {
+                        sender.sendMessage("The MOTD did not match when we pinged the server (we got '" + remoteMotd + "'). " +
+                                "Did you supply the correct IP and port of your server?");
+                        sendLinks(sender);
+                        return;
+                    }
+
+                    if (ping.get("tcpFirst").asBoolean()) {
+                        sender.sendMessage("Your server hardware likely has some sort of firewall preventing people from joining easily. See https://geysermc.link/ovh-firewall for more information.");
+                        sendLinks(sender);
+                        return;
+                    }
+
+                    sender.sendMessage("Your server is likely online and working as of " + when + "!");
                     sendLinks(sender);
                     return;
                 }
@@ -123,16 +199,16 @@ public class ConnectionTestCommand extends GeyserCommand {
                 sender.sendMessage("Your server is likely unreachable from outside the network as of " + when + ".");
                 sendLinks(sender);
             } catch (Exception e) {
-                sender.sendMessage("Error while trying to check your connection!");
+                sender.sendMessage("An error occurred while trying to check your connection! Check the console for more information.");
                 geyser.getLogger().error("Error while trying to check your connection!", e);
             }
         });
     }
 
     private void sendLinks(GeyserCommandSource sender) {
-        sender.sendMessage("If you still have issues, check to see if your hosting provider has a specific setup: " +
+        sender.sendMessage("If you still face issues, check the setup guide for instructions: " +
-                "https://wiki.geysermc.org/geyser/supported-hosting-providers/" + ", see this page: "
+                "https://wiki.geysermc.org/geyser/setup/");
-                + "https://wiki.geysermc.org/geyser/fixing-unable-to-connect-to-world/" + ", or contact us on our Discord: " + "https://discord.gg/geysermc");
+        sender.sendMessage("If that does not work, see " + "https://wiki.geysermc.org/geyser/fixing-unable-to-connect-to-world/" + ", or contact us on Discord: " + "https://discord.gg/geysermc");
     }
 
     @Override

core/src/main/java/org/geysermc/geyser/network/netty/GeyserServer.java

@@ -47,6 +47,7 @@ import org.cloudburstmc.netty.channel.raknet.config.RakChannelOption;
 import org.cloudburstmc.netty.handler.codec.raknet.server.RakServerOfflineHandler;
 import org.cloudburstmc.protocol.bedrock.BedrockPong;
 import org.geysermc.geyser.GeyserImpl;
+import org.geysermc.geyser.command.defaults.ConnectionTestCommand;
 import org.geysermc.geyser.configuration.GeyserConfiguration;
 import org.geysermc.geyser.event.type.GeyserBedrockPingEventImpl;
 import org.geysermc.geyser.network.CIDRMatcher;
@@ -254,6 +255,12 @@ public final class GeyserServer {
             pong.subMotd(GeyserImpl.NAME);
         }
 
+        if (ConnectionTestCommand.CONNECTION_TEST_MOTD != null) {
+            // Force-override as we are testing the connection and want to verify we are connecting to the right server through the MOTD
+            pong.motd(ConnectionTestCommand.CONNECTION_TEST_MOTD);
+            pong.subMotd(GeyserImpl.NAME);
+        }
+
         // The ping will not appear if the MOTD + sub-MOTD is of a certain length.
         // We don't know why, though
         byte[] motdArray = pong.motd().getBytes(StandardCharsets.UTF_8);

core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/entity/player/BedrockActionTranslator.java

@@ -285,7 +285,7 @@ public class BedrockActionTranslator extends PacketTranslator<PlayerActionPacket
                 entity.setOnGround(false); // Increase block break time while jumping
                 break;
             case MISSED_SWING:
-                // TODO Re-evaluate after pre-1.20.10 is supported?
+                // TODO Re-evaluate after pre-1.20.10 is no longer supported?
                 if (session.getArmAnimationTicks() == -1) {
                     session.sendDownstreamPacket(new ServerboundSwingPacket(Hand.MAIN_HAND));
                     session.activateArmAnimationTicking();