Pinger's MOTD Placeholder doesn't update as the MOTD updates

[hitsu420]

Hey
So, my problem is, that the %pinger_motd% doesn't update/just gets the server.properties' motd.
Imagine, you want to have the motd be the motd set by a plugin, for example BedwarsRel, which sets it to something like <status> <players>/<maxplayers> e.g. ready 0/10.
Well, unfortunetely that doesn't work for some reason. (In my case it just returns A Minecraft Server)
Please fix, thanks!

[M4T14SG4M3R]

3 years and still not fixed :C

[LawMixer]

This is still happening. %pinger_motd% returns whatever is in the server.config file in your Minecraft server.

[Andre601]

Pretty sure this can't be fixed at all.
All the expansion does is ping the server. Whatever is returned by that ping is what the expansion uses.

Not sure if the MC client sends some specific data that makes it receive diffrent data, including a different MOTD, but it could be exactly that to be honest here.

Either way, if you understand a bit of Java code, look at this section here:
https://github.com/PlaceholderAPI/Pinger-Expansion/blob/master/src/main/java/com/extendedclip/papi/expansion/pinger/PingerExpansion.java#L240

This is where the expansion retrieves the data from the pinged server, extracts the data from it, converts it to a String, splits it up into sections and then uses each section for the different options (MOTD, Players, etc)

If you're pinging a MC server that is part of a MC network AND the MOTD is handled by a BungeeCord plugin could the issue be that you ping the server directly, therefore bypassing the proxy and getting whatever the actual server has.

Either way, I have my doubts this issue is actually an issue with the expansion and that it could be fixed. If it was fixable, it probs would've been by now.

[0dinD]

EDIT: I was debugging using a fresh build from the source code, not the eCloud JAR. See this comment for why it matters: #2 (comment)

---

I think it is an issue with this expansion, I stepped through the code with a debugger and found that the expansion runs into an ArrayIndexOutOfBoundsException here:

Pinger-Expansion/src/main/java/com/extendedclip/papi/expansion/pinger/PingerExpansion.java

Line 308 in 34d9492

setPlayersOnline(Integer.parseInt(data[1]));

The exception is just swallowed here:

Pinger-Expansion/src/main/java/com/extendedclip/papi/expansion/pinger/PingerExpansion.java

Lines 63 to 64 in 34d9492

	} catch (Exception exception) {
	}

The ArrayIndexOutOfBoundsException was because of the fact that the data array only contained a single element, since the & delimiter was not found in the string. So then I wondered, why is the code trying to split on the & delimiter anyway? From what I could see by looking at the string in the debugger, the delimiter is supposed to be §, and this is also indicated here: https://wiki.vg/Server_List_Ping. I tried switching to the § delimiter instead, which seems to have solved the issue.

Why was the delimiter ever &? Is there something I'm missing? Maybe someone did a search-and-replace for § -> & when dealing with the formatting codes in the online/offline templates, and accidentally introduced the bug?

[0dinD]

In addition to splitting on §, it's also important that we check § as the first character, not &.

I think the current code is trying to support two different ping formats. If the packet starts with §, the code is trying to read according to this format: https://wiki.vg/Server_List_Ping#1.4_to_1.5, which is the same as the 1.6 format in terms of reading the response. Otherwise, it tries to read according to this format: https://wiki.vg/Server_List_Ping#Beta_1.8_to_1.3, which is why it has to split on §. Not sure why we need to support Beta 1.8 - 1.3 servers, lol. It seems like modern Mincraft servers still support the legacy ping request formats, so I guess it's fine to use them.

I probably have time tomorrow to do some more testing to make sure everything really works now, and then I could submit a PR to fix this issue.

[0dinD]

Huh, okay, this issue is weirder than I originally thought. I've now done some more testing, and the problem is definitely that § is supposed to be used instead of & when handling the ping packets. But, to my surprise (after a bunch of debugging), I can't reproduce this issue when using the Pinger expansion from eCloud, only when building the source code from this repository. It turns out that there have been changes here in the source code which have not been built and released on eCloud. As you can see here: https://api.extendedclip.com/expansions/pinger/versions/ , Pinger was uploaded to eCloud in 2017, but if you look at the history of this git repository, you can see that the source code was not uploaded until 2018/2021. By decompiling and analyzing the Pinger JAR from eCloud, I have found that the diff between the eCloud version and the source version of Pinger boils down to this:

diff --git a/src/main/java/com/extendedclip/papi/expansion/pinger/PingerExpansion.java b/src/main/java/com/extendedclip/papi/expansion/pinger/PingerExpansion.java
index 9a13260..c15b926 100644
--- a/src/main/java/com/extendedclip/papi/expansion/pinger/PingerExpansion.java
+++ b/src/main/java/com/extendedclip/papi/expansion/pinger/PingerExpansion.java
@@ -294,7 +294,7 @@ public class PingerExpansion extends PlaceholderExpansion implements Cacheable,
                     return false;
                 }
                 String string = new String(chars);
-                if (string.startsWith("&")) {
+                if (string.startsWith("§")) {
                     String[] data = string.split("\000");
                     setPingVersion(Integer.parseInt(data[0].substring(1)));
                     setProtocolVersion(Integer.parseInt(data[1]));
@@ -303,7 +303,7 @@ public class PingerExpansion extends PlaceholderExpansion implements Cacheable,
                     setPlayersOnline(Integer.parseInt(data[4]));
                     setMaxPlayers(Integer.parseInt(data[5]));
                 } else {
-                    String[] data = string.split("&");
+                    String[] data = string.split("§");
                     setMotd(data[0]);
                     setPlayersOnline(Integer.parseInt(data[1]));
                     setMaxPlayers(Integer.parseInt(data[2]));

That is to say, the old eCloud version actually correctly uses the § character instead of &. So the mistake must have been introduced when uploading the source code.

But okay, if the eCloud version correctly uses the § character, what's the problem? Why are people still experiencing this issue? Well, it turns out that there's a bug on older versions of Spigot, on Mincraft 1.8 - 1.12, which was fixed in 1.13.2. Specifically, Spigot (on these old versions) would (if the server is pinged using one of the legacy ping formats) return the MOTD from the server.properties file instead of the one returned by plugin event handlers, which is the actual cause of this issue. It means that if a plugin changes the MOTD via an event handler, it will only be shown for clients that use the "new" Minecraft 1.7+ ping format: https://wiki.vg/Server_List_Ping#Current_.281.7.2B.29. And because (as I described in the previous comment) Pinger uses the legacy ping format, it means that Spigot servers for Mincraft 1.8 - 1.12 will return the MOTD from the server.properties file to Pinger, not the MOTD from plugin event handlers.

The bug was fixed in Spigot for Minecraft 1.13.2+ in this commit: https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/commits/a3c2ec03148f9f38d4d27d045b1afee2fc6ff173

---

So, what's the conclusion?

1. This issue is caused by Spigot, and will only occur on servers for Minecraft 1.8 - 1.12, on newer servers it can't be reproduced. Also, the issue will only happen if the legacy 1.6, 1.4-1.5 or Beta 1.8 - 1.3 ping packet format is used. The Pinger code could be changed to send the ping request using the 1.7+ format, which would resolve the problem on "all" Minecraft versions, 1.7+.
2. There is an unrelated issue in the source code of this git repository where someone seems to have accidentally replaced a few § characters with &. This means that Pinger won't work correctly on any Minecraft version if built from this source code. However, most people are using the old eCloud JAR, which was built and published without this defect, so that's why Pinger has been working for most people on newer Mincraft versions.

[0dinD]

I have now provided a fix for the packet decoding in #8, as well as some code cleanup and comments to explain what the current code is doing.

As far as fixing this issue goes (again, it only happens on Minecraft 1.8 - 1.12), the main solution I see is to change the code to use the Minecraft 1.7+ ping request format, which is not affected by the Spigot MOTD bug. I am not interested in working on this, especially since there is already an alternative plugin (based on, or at least inspired by Pinger, it seems), which uses the 1.7+ ping format to fix this issue: https://github.com/HappyAreaBean/ServerPinger-PAPI-Expansion. It uses this library to send the ping requests: https://github.com/lucaazalim/minecraft-server-ping.

Or actually, I'm not sure, that library might be using Query: https://wiki.vg/Query, which should also work.

On second thought, I don't think they're using Query, it looks like it's just the 1.7+ server ping list request.