Fix plugin message string encoding inconsistency (#3364)

bukkit/src/main/java/me/lucko/luckperms/bukkit/messaging/PluginMessageMessenger.java

@@ -26,15 +26,12 @@
 package me.lucko.luckperms.bukkit.messaging;
 
 import com.google.common.collect.Iterables;
-import com.google.common.io.ByteArrayDataInput;
-import com.google.common.io.ByteArrayDataOutput;
-import com.google.common.io.ByteStreams;
 
 import me.lucko.luckperms.bukkit.LPBukkitPlugin;
+import me.lucko.luckperms.common.messaging.pluginmsg.AbstractPluginMessageMessenger;
 
 import net.luckperms.api.messenger.IncomingMessageConsumer;
 import net.luckperms.api.messenger.Messenger;
-import net.luckperms.api.messenger.message.OutgoingMessage;
 
 import org.bukkit.entity.Player;
 import org.bukkit.plugin.messaging.PluginMessageListener;
@@ -46,15 +43,12 @@ import java.util.Collection;
 /**
  * An implementation of {@link Messenger} using the plugin messaging channels.
  */
-public class PluginMessageMessenger implements Messenger, PluginMessageListener {
+public class PluginMessageMessenger extends AbstractPluginMessageMessenger implements PluginMessageListener {
-    private static final String CHANNEL = "luckperms:update";
-
     private final LPBukkitPlugin plugin;
-    private final IncomingMessageConsumer consumer;
 
     public PluginMessageMessenger(LPBukkitPlugin plugin, IncomingMessageConsumer consumer) {
+        super(consumer);
         this.plugin = plugin;
-        this.consumer = consumer;
     }
 
     public void init() {
@@ -69,11 +63,7 @@ public class PluginMessageMessenger implements Messenger, PluginMessageListener
     }
 
     @Override
-    public void sendOutgoingMessage(@NonNull OutgoingMessage outgoingMessage) {
+    protected void sendOutgoingMessage(byte[] buf) {
-        ByteArrayDataOutput out = ByteStreams.newDataOutput();
-        out.writeUTF(outgoingMessage.asEncodedString());
-        byte[] data = out.toByteArray();
-
         new BukkitRunnable() {
             @Override
             public void run() {
@@ -83,21 +73,18 @@ public class PluginMessageMessenger implements Messenger, PluginMessageListener
                     return;
                 }
 
-                p.sendPluginMessage(PluginMessageMessenger.this.plugin.getLoader(), CHANNEL, data);
+                p.sendPluginMessage(PluginMessageMessenger.this.plugin.getLoader(), CHANNEL, buf);
                 cancel();
             }
         }.runTaskTimer(this.plugin.getLoader(), 1L, 100L);
     }
 
     @Override
-    public void onPluginMessageReceived(String s, @NonNull Player player, @NonNull byte[] bytes) {
+    public void onPluginMessageReceived(String channel, @NonNull Player player, byte @NonNull [] message) {
-        if (!s.equals(CHANNEL)) {
+        if (!channel.equals(CHANNEL)) {
             return;
         }
 
-        ByteArrayDataInput in = ByteStreams.newDataInput(bytes);
+        handleIncomingMessage(message);
-        String msg = in.readUTF();
-
-        this.consumer.consumeIncomingMessageAsString(msg);
     }
 }

bungee/src/main/java/me/lucko/luckperms/bungee/messaging/PluginMessageMessenger.java

@@ -25,15 +25,11 @@
 
 package me.lucko.luckperms.bungee.messaging;
 
-import com.google.common.io.ByteArrayDataInput;
-import com.google.common.io.ByteArrayDataOutput;
-import com.google.common.io.ByteStreams;
-
 import me.lucko.luckperms.bungee.LPBungeePlugin;
+import me.lucko.luckperms.common.messaging.pluginmsg.AbstractPluginMessageMessenger;
 
 import net.luckperms.api.messenger.IncomingMessageConsumer;
 import net.luckperms.api.messenger.Messenger;
-import net.luckperms.api.messenger.message.OutgoingMessage;
 import net.md_5.bungee.api.ProxyServer;
 import net.md_5.bungee.api.config.ServerInfo;
 import net.md_5.bungee.api.connection.ProxiedPlayer;
@@ -41,20 +37,15 @@ import net.md_5.bungee.api.event.PluginMessageEvent;
 import net.md_5.bungee.api.plugin.Listener;
 import net.md_5.bungee.event.EventHandler;
 
-import org.checkerframework.checker.nullness.qual.NonNull;
-
 /**
  * An implementation of {@link Messenger} using the plugin messaging channels.
  */
-public class PluginMessageMessenger implements Messenger, Listener {
+public class PluginMessageMessenger extends AbstractPluginMessageMessenger implements Listener {
-    private static final String CHANNEL = "luckperms:update";
-
     private final LPBungeePlugin plugin;
-    private final IncomingMessageConsumer consumer;
 
     public PluginMessageMessenger(LPBungeePlugin plugin, IncomingMessageConsumer consumer) {
+        super(consumer);
         this.plugin = plugin;
-        this.consumer = consumer;
     }
 
     public void init() {
@@ -70,19 +61,11 @@ public class PluginMessageMessenger implements Messenger, Listener {
         proxy.getPluginManager().unregisterListener(this);
     }
 
-    private void dispatchMessage(byte[] message) {
-        for (ServerInfo server : this.plugin.getBootstrap().getProxy().getServers().values()) {
-            server.sendData(CHANNEL, message, false);
-        }
-    }
-
     @Override
-    public void sendOutgoingMessage(@NonNull OutgoingMessage outgoingMessage) {
+    protected void sendOutgoingMessage(byte[] buf) {
-        ByteArrayDataOutput out = ByteStreams.newDataOutput();
+        for (ServerInfo server : this.plugin.getBootstrap().getProxy().getServers().values()) {
-        out.writeUTF(outgoingMessage.asEncodedString());
+            server.sendData(CHANNEL, buf, false);
-
+        }
-        byte[] message = out.toByteArray();
-        dispatchMessage(message);
     }
 
     @EventHandler
@@ -97,14 +80,11 @@ public class PluginMessageMessenger implements Messenger, Listener {
             return;
         }
 
-        byte[] data = e.getData();
+        byte[] buf = e.getData();
 
-        ByteArrayDataInput in = ByteStreams.newDataInput(data);
+        if (handleIncomingMessage(buf)) {
-        String msg = in.readUTF();
-
-        if (this.consumer.consumeIncomingMessageAsString(msg)) {
             // Forward to other servers
-            this.plugin.getBootstrap().getScheduler().executeAsync(() -> dispatchMessage(data));
+            this.plugin.getBootstrap().getScheduler().executeAsync(() -> sendOutgoingMessage(buf));
         }
     }
 }

common/src/main/java/me/lucko/luckperms/common/messaging/pluginmsg/AbstractPluginMessageMessenger.java

@@ -0,0 +1,80 @@
+/*
+ * This file is part of LuckPerms, licensed under the MIT License.
+ *
+ *  Copyright (c) lucko (Luck) <luck@lucko.me>
+ *  Copyright (c) contributors
+ *
+ *  Permission is hereby granted, free of charge, to any person obtaining a copy
+ *  of this software and associated documentation files (the "Software"), to deal
+ *  in the Software without restriction, including without limitation the rights
+ *  to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ *  copies of the Software, and to permit persons to whom the Software is
+ *  furnished to do so, subject to the following conditions:
+ *
+ *  The above copyright notice and this permission notice shall be included in all
+ *  copies or substantial portions of the Software.
+ *
+ *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ *  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ *  AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ *  LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ *  OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ *  SOFTWARE.
+ */
+
+package me.lucko.luckperms.common.messaging.pluginmsg;
+
+import com.google.common.io.ByteArrayDataInput;
+import com.google.common.io.ByteArrayDataOutput;
+import com.google.common.io.ByteStreams;
+
+import net.luckperms.api.messenger.IncomingMessageConsumer;
+import net.luckperms.api.messenger.Messenger;
+import net.luckperms.api.messenger.message.OutgoingMessage;
+
+import org.checkerframework.checker.nullness.qual.NonNull;
+
+/**
+ * Abstract implementation of {@link Messenger} using Minecraft's
+ * 'plugin messaging channels' packet.
+ *
+ * <p>The {@link OutgoingMessage#asEncodedString() encoded string} format
+ * is used to transmit messages. {@link java.io.DataOutput#writeUTF(String)} is
+ * used to encode the string into raw bytes.</p>
+ */
+public abstract class AbstractPluginMessageMessenger implements Messenger {
+
+    /**
+     * The identifier of the channel used by LuckPerms for all messages.
+     */
+    public static final String CHANNEL = "luckperms:update";
+
+    /**
+     * The {@link IncomingMessageConsumer} used by the messenger.
+     */
+    private final IncomingMessageConsumer consumer;
+
+    protected AbstractPluginMessageMessenger(IncomingMessageConsumer consumer) {
+        this.consumer = consumer;
+    }
+
+    @Override
+    public final void sendOutgoingMessage(@NonNull OutgoingMessage outgoingMessage) {
+        ByteArrayDataOutput dataOutput = ByteStreams.newDataOutput();
+        dataOutput.writeUTF(outgoingMessage.asEncodedString());
+
+        byte[] buf = dataOutput.toByteArray();
+
+        sendOutgoingMessage(buf);
+    }
+
+    protected abstract void sendOutgoingMessage(byte[] buf);
+
+    protected boolean handleIncomingMessage(byte[] buf) {
+        ByteArrayDataInput dataInput = ByteStreams.newDataInput(buf);
+        String decodedString = dataInput.readUTF();
+        return this.consumer.consumeIncomingMessageAsString(decodedString);
+    }
+
+}

fabric/src/main/java/me/lucko/luckperms/fabric/messaging/PluginMessageMessenger.java

@@ -27,6 +27,7 @@ package me.lucko.luckperms.fabric.messaging;
 
 import com.google.common.collect.Iterables;
 
+import me.lucko.luckperms.common.messaging.pluginmsg.AbstractPluginMessageMessenger;
 import me.lucko.luckperms.common.plugin.scheduler.SchedulerTask;
 import me.lucko.luckperms.fabric.LPFabricPlugin;
 
@@ -34,29 +35,24 @@ import net.fabricmc.fabric.api.networking.v1.PacketByteBufs;
 import net.fabricmc.fabric.api.networking.v1.PacketSender;
 import net.fabricmc.fabric.api.networking.v1.ServerPlayNetworking;
 import net.luckperms.api.messenger.IncomingMessageConsumer;
-import net.luckperms.api.messenger.Messenger;
-import net.luckperms.api.messenger.message.OutgoingMessage;
 import net.minecraft.network.PacketByteBuf;
 import net.minecraft.server.MinecraftServer;
 import net.minecraft.server.network.ServerPlayNetworkHandler;
 import net.minecraft.server.network.ServerPlayerEntity;
 import net.minecraft.util.Identifier;
 
-import org.checkerframework.checker.nullness.qual.NonNull;
-
 import java.util.Collection;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 
-public class PluginMessageMessenger implements Messenger, ServerPlayNetworking.PlayChannelHandler {
+public class PluginMessageMessenger extends AbstractPluginMessageMessenger implements ServerPlayNetworking.PlayChannelHandler {
-    private static final Identifier CHANNEL = new Identifier("luckperms", "update");
+    private static final Identifier CHANNEL = new Identifier(AbstractPluginMessageMessenger.CHANNEL);
 
     private final LPFabricPlugin plugin;
-    private final IncomingMessageConsumer consumer;
 
     public PluginMessageMessenger(LPFabricPlugin plugin, IncomingMessageConsumer consumer) {
+        super(consumer);
         this.plugin = plugin;
-        this.consumer = consumer;
     }
 
     public void init() {
@@ -69,7 +65,7 @@ public class PluginMessageMessenger implements Messenger, ServerPlayNetworking.P
     }
 
     @Override
-    public void sendOutgoingMessage(@NonNull OutgoingMessage outgoingMessage) {
+    protected void sendOutgoingMessage(byte[] buf) {
         AtomicReference<SchedulerTask> taskRef = new AtomicReference<>();
         SchedulerTask task = this.plugin.getBootstrap().getScheduler().asyncRepeating(() -> {
             MinecraftServer server = this.plugin.getBootstrap().getServer().orElse(null);
@@ -83,9 +79,9 @@ public class PluginMessageMessenger implements Messenger, ServerPlayNetworking.P
                 return;
             }
 
-            PacketByteBuf buf = PacketByteBufs.create();
+            PacketByteBuf packetBuf = PacketByteBufs.create();
-            buf.writeString(outgoingMessage.asEncodedString());
+            packetBuf.writeBytes(buf);
-            ServerPlayNetworking.send(p, CHANNEL, buf);
+            ServerPlayNetworking.send(p, CHANNEL, packetBuf);
 
             SchedulerTask t = taskRef.getAndSet(null);
             if (t != null) {
@@ -96,8 +92,10 @@ public class PluginMessageMessenger implements Messenger, ServerPlayNetworking.P
     }
 
     @Override
-    public void receive(MinecraftServer server, ServerPlayerEntity entity, ServerPlayNetworkHandler netHandler, PacketByteBuf buf, PacketSender packetSender) {
+    public void receive(MinecraftServer server, ServerPlayerEntity entity, ServerPlayNetworkHandler netHandler, PacketByteBuf packetBuf, PacketSender packetSender) {
-        String msg = buf.readString();
+        byte[] buf = new byte[packetBuf.readableBytes()];
-        this.consumer.consumeIncomingMessageAsString(msg);
+        packetBuf.readBytes(buf);
+
+        handleIncomingMessage(buf);
     }
 }

sponge/src/main/java/me/lucko/luckperms/sponge/messaging/PluginMessageMessenger.java

@@ -27,13 +27,12 @@ package me.lucko.luckperms.sponge.messaging;
 
 import com.google.common.collect.Iterables;
 
+import me.lucko.luckperms.common.messaging.pluginmsg.AbstractPluginMessageMessenger;
 import me.lucko.luckperms.sponge.LPSpongePlugin;
 
 import net.luckperms.api.messenger.IncomingMessageConsumer;
 import net.luckperms.api.messenger.Messenger;
-import net.luckperms.api.messenger.message.OutgoingMessage;
 
-import org.checkerframework.checker.nullness.qual.NonNull;
 import org.spongepowered.api.ResourceKey;
 import org.spongepowered.api.entity.living.player.server.ServerPlayer;
 import org.spongepowered.api.network.EngineConnectionSide;
@@ -41,6 +40,7 @@ import org.spongepowered.api.network.ServerSideConnection;
 import org.spongepowered.api.network.channel.ChannelBuf;
 import org.spongepowered.api.network.channel.raw.RawDataChannel;
 import org.spongepowered.api.network.channel.raw.play.RawPlayDataHandler;
+import org.spongepowered.api.scheduler.ScheduledTask;
 import org.spongepowered.api.scheduler.Task;
 
 import java.util.Collection;
@@ -49,17 +49,16 @@ import java.util.concurrent.TimeUnit;
 /**
  * An implementation of {@link Messenger} using the plugin messaging channels.
  */
-public class PluginMessageMessenger implements Messenger, RawPlayDataHandler<ServerSideConnection> {
+public class PluginMessageMessenger extends AbstractPluginMessageMessenger implements RawPlayDataHandler<ServerSideConnection> {
-    private static final ResourceKey CHANNEL = ResourceKey.of("luckperms", "update");
+    private static final ResourceKey CHANNEL = ResourceKey.resolve(AbstractPluginMessageMessenger.CHANNEL);
 
     private final LPSpongePlugin plugin;
-    private final IncomingMessageConsumer consumer;
 
     private RawDataChannel channel = null;
 
     public PluginMessageMessenger(LPSpongePlugin plugin, IncomingMessageConsumer consumer) {
+        super(consumer);
         this.plugin = plugin;
-        this.consumer = consumer;
     }
 
     public void init() {
@@ -75,32 +74,39 @@ public class PluginMessageMessenger implements Messenger, RawPlayDataHandler<Ser
     }
 
     @Override
-    public void sendOutgoingMessage(@NonNull OutgoingMessage outgoingMessage) {
+    protected void sendOutgoingMessage(byte[] buf) {
-        Task t = Task.builder()
+        if (!this.plugin.getBootstrap().getGame().isServerAvailable()) {
+            return;
+        }
+
+        Task task = Task.builder()
                 .interval(10, TimeUnit.SECONDS)
-                .execute(task -> {
+                .execute(t -> sendOutgoingMessage(buf, t))
-                    if (!this.plugin.getBootstrap().getGame().isServerAvailable()) {
-                        return;
-                    }
-
-                    Collection<ServerPlayer> players = this.plugin.getBootstrap().getGame().server().onlinePlayers();
-                    ServerPlayer p = Iterables.getFirst(players, null);
-                    if (p == null) {
-                        return;
-                    }
-
-                    this.channel.play().sendTo(p, buf -> buf.writeUTF(outgoingMessage.asEncodedString()));
-                    task.cancel();
-                })
                 .plugin(this.plugin.getBootstrap().getPluginContainer())
                 .build();
 
-        this.plugin.getBootstrap().getScheduler().getSyncScheduler().submit(t);
+        this.plugin.getBootstrap().getScheduler().getSyncScheduler().submit(task);
+    }
+
+    private void sendOutgoingMessage(byte[] buf, ScheduledTask scheduledTask) {
+        if (!this.plugin.getBootstrap().getGame().isServerAvailable()) {
+            scheduledTask.cancel();
+            return;
+        }
+
+        Collection<ServerPlayer> players = this.plugin.getBootstrap().getGame().server().onlinePlayers();
+        ServerPlayer p = Iterables.getFirst(players, null);
+        if (p == null) {
+            return;
+        }
+
+        this.channel.play().sendTo(p, channelBuf -> channelBuf.writeBytes(buf));
+        scheduledTask.cancel();
     }
 
     @Override
-    public void handlePayload(ChannelBuf buf, ServerSideConnection connection) {
+    public void handlePayload(ChannelBuf channelBuf, ServerSideConnection connection) {
-        String msg = buf.readUTF();
+        byte[] buf = channelBuf.readBytes(channelBuf.available());
-        this.consumer.consumeIncomingMessageAsString(msg);
+        handleIncomingMessage(buf);
     }
 }

velocity/src/main/java/me/lucko/luckperms/velocity/messaging/PluginMessageMessenger.java

@@ -25,9 +25,6 @@
 
 package me.lucko.luckperms.velocity.messaging;
 
-import com.google.common.io.ByteArrayDataInput;
-import com.google.common.io.ByteArrayDataOutput;
-import com.google.common.io.ByteStreams;
 import com.velocitypowered.api.event.Subscribe;
 import com.velocitypowered.api.event.connection.PluginMessageEvent;
 import com.velocitypowered.api.event.connection.PluginMessageEvent.ForwardResult;
@@ -37,26 +34,23 @@ import com.velocitypowered.api.proxy.messages.ChannelIdentifier;
 import com.velocitypowered.api.proxy.messages.MinecraftChannelIdentifier;
 import com.velocitypowered.api.proxy.server.RegisteredServer;
 
+import me.lucko.luckperms.common.messaging.pluginmsg.AbstractPluginMessageMessenger;
 import me.lucko.luckperms.velocity.LPVelocityPlugin;
 
 import net.luckperms.api.messenger.IncomingMessageConsumer;
 import net.luckperms.api.messenger.Messenger;
-import net.luckperms.api.messenger.message.OutgoingMessage;
-
-import org.checkerframework.checker.nullness.qual.NonNull;
 
 /**
  * An implementation of {@link Messenger} using the plugin messaging channels.
  */
-public class PluginMessageMessenger implements Messenger {
+public class PluginMessageMessenger extends AbstractPluginMessageMessenger {
-    private static final ChannelIdentifier CHANNEL = MinecraftChannelIdentifier.create("luckperms", "update");
+    private static final ChannelIdentifier CHANNEL = MinecraftChannelIdentifier.from(AbstractPluginMessageMessenger.CHANNEL);
 
     private final LPVelocityPlugin plugin;
-    private final IncomingMessageConsumer consumer;
 
     public PluginMessageMessenger(LPVelocityPlugin plugin, IncomingMessageConsumer consumer) {
+        super(consumer);
         this.plugin = plugin;
-        this.consumer = consumer;
     }
 
     public void init() {
@@ -72,19 +66,11 @@ public class PluginMessageMessenger implements Messenger {
         proxy.getEventManager().unregisterListener(this.plugin.getBootstrap(), this);
     }
 
-    private void dispatchMessage(byte[] message) {
-        for (RegisteredServer server : this.plugin.getBootstrap().getProxy().getAllServers()) {
-            server.sendPluginMessage(CHANNEL, message);
-        }
-    }
-
     @Override
-    public void sendOutgoingMessage(@NonNull OutgoingMessage outgoingMessage) {
+    protected void sendOutgoingMessage(byte[] buf) {
-        ByteArrayDataOutput out = ByteStreams.newDataOutput();
+        for (RegisteredServer server : this.plugin.getBootstrap().getProxy().getAllServers()) {
-        out.writeUTF(outgoingMessage.asEncodedString());
+            server.sendPluginMessage(CHANNEL, buf);
-
+        }
-        byte[] message = out.toByteArray();
-        dispatchMessage(message);
     }
 
     @Subscribe
@@ -102,12 +88,11 @@ public class PluginMessageMessenger implements Messenger {
             return;
         }
 
-        ByteArrayDataInput in = e.dataAsDataStream();
+        byte[] buf = e.getData();
-        String msg = in.readUTF();
 
-        if (this.consumer.consumeIncomingMessageAsString(msg)) {
+        if (handleIncomingMessage(buf)) {
             // Forward to other servers
-            this.plugin.getBootstrap().getScheduler().executeAsync(() -> dispatchMessage(e.getData()));
+            this.plugin.getBootstrap().getScheduler().executeAsync(() -> sendOutgoingMessage(buf));
         }
     }
 }