From 4f19bfde3da82ec4a321b225e0cdbaeaea741d90 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Thu, 16 Jul 2020 12:44:02 -0400 Subject: [PATCH] Fix various problems with GS4QueryHandler --- .../proxy/protocol/netty/GS4QueryHandler.java | 183 ++++++++---------- 1 file changed, 82 insertions(+), 101 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/GS4QueryHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/GS4QueryHandler.java index 6ca9d42e9..508cba159 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/GS4QueryHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/GS4QueryHandler.java @@ -9,6 +9,7 @@ import com.google.common.collect.ImmutableSet; import com.velocitypowered.api.event.query.ProxyQueryEvent; import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.api.plugin.PluginContainer; +import com.velocitypowered.api.plugin.PluginDescription; import com.velocitypowered.api.proxy.Player; import com.velocitypowered.api.proxy.server.QueryResponse; import com.velocitypowered.proxy.VelocityServer; @@ -19,6 +20,7 @@ import io.netty.channel.socket.DatagramPacket; import java.net.InetAddress; import java.nio.charset.StandardCharsets; import java.security.SecureRandom; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Iterator; @@ -33,8 +35,6 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; public class GS4QueryHandler extends SimpleChannelInboundHandler { - private static final Logger logger = LogManager.getLogger(GS4QueryHandler.class); - private static final short QUERY_MAGIC_FIRST = 0xFE; private static final short QUERY_MAGIC_SECOND = 0xFD; private static final byte QUERY_TYPE_HANDSHAKE = 0x09; @@ -59,10 +59,6 @@ public class GS4QueryHandler extends SimpleChannelInboundHandler .expireAfterWrite(30, TimeUnit.SECONDS) .build(); private final SecureRandom random; - - private volatile @MonotonicNonNull List pluginInformationList - = null; - private final VelocityServer server; public GS4QueryHandler(VelocityServer server) { @@ -93,95 +89,87 @@ public class GS4QueryHandler extends SimpleChannelInboundHandler ByteBuf queryMessage = msg.content(); InetAddress senderAddress = msg.sender().getAddress(); - // Allocate buffer for response - ByteBuf queryResponse = ctx.alloc().buffer(); - DatagramPacket responsePacket = new DatagramPacket(queryResponse, msg.sender()); + // Verify query packet magic + if (queryMessage.readUnsignedByte() != QUERY_MAGIC_FIRST + || queryMessage.readUnsignedByte() != QUERY_MAGIC_SECOND) { + return; + } - try { - // Verify query packet magic - if (queryMessage.readUnsignedByte() != QUERY_MAGIC_FIRST - || queryMessage.readUnsignedByte() != QUERY_MAGIC_SECOND) { - throw new IllegalStateException("Invalid query packet magic"); + // Read packet header + short type = queryMessage.readUnsignedByte(); + int sessionId = queryMessage.readInt(); + + switch (type) { + case QUERY_TYPE_HANDSHAKE: { + // Generate new challenge token and put it into the sessions cache + int challengeToken = random.nextInt(); + sessions.put(senderAddress, challengeToken); + + // Respond with challenge token + ByteBuf queryResponse = ctx.alloc().buffer(); + queryResponse.writeByte(QUERY_TYPE_HANDSHAKE); + queryResponse.writeInt(sessionId); + writeString(queryResponse, Integer.toString(challengeToken)); + + DatagramPacket responsePacket = new DatagramPacket(queryResponse, msg.sender()); + ctx.writeAndFlush(responsePacket, ctx.voidPromise()); + break; } - // Read packet header - short type = queryMessage.readUnsignedByte(); - int sessionId = queryMessage.readInt(); - - switch (type) { - case QUERY_TYPE_HANDSHAKE: { - // Generate new challenge token and put it into the sessions cache - int challengeToken = random.nextInt(); - sessions.put(senderAddress, challengeToken); - - // Respond with challenge token - queryResponse.writeByte(QUERY_TYPE_HANDSHAKE); - queryResponse.writeInt(sessionId); - writeString(queryResponse, Integer.toString(challengeToken)); - ctx.writeAndFlush(responsePacket, ctx.voidPromise()); - break; + case QUERY_TYPE_STAT: { + // Check if query was done with session previously generated using a handshake packet + int challengeToken = queryMessage.readInt(); + Integer session = sessions.getIfPresent(senderAddress); + if (session == null || session != challengeToken) { + return; } - case QUERY_TYPE_STAT: { - // Check if query was done with session previously generated using a handshake packet - int challengeToken = queryMessage.readInt(); - Integer session = sessions.getIfPresent(senderAddress); - if (session == null || session != challengeToken) { - throw new IllegalStateException("Invalid challenge token"); - } - - // Check which query response client expects - if (queryMessage.readableBytes() != 0 && queryMessage.readableBytes() != 4) { - throw new IllegalStateException("Invalid query packet"); - } - - // Build query response - QueryResponse response = createInitialResponse(); - - boolean isBasic = queryMessage.readableBytes() == 0; - - // Call event and write response - server.getEventManager() - .fire(new ProxyQueryEvent(isBasic ? BASIC : FULL, senderAddress, response)) - .whenCompleteAsync((event, exc) -> { - // Packet header - queryResponse.writeByte(QUERY_TYPE_STAT); - queryResponse.writeInt(sessionId); - - // Start writing the response - ResponseWriter responseWriter = new ResponseWriter(queryResponse, isBasic); - responseWriter.write("hostname", event.getResponse().getHostname()); - responseWriter.write("gametype", "SMP"); - - responseWriter.write("game_id", "MINECRAFT"); - responseWriter.write("version", event.getResponse().getGameVersion()); - responseWriter.writePlugins(event.getResponse().getProxyVersion(), - event.getResponse().getPlugins()); - - responseWriter.write("map", event.getResponse().getMap()); - responseWriter.write("numplayers", event.getResponse().getCurrentPlayers()); - responseWriter.write("maxplayers", event.getResponse().getMaxPlayers()); - responseWriter.write("hostport", event.getResponse().getProxyPort()); - responseWriter.write("hostip", event.getResponse().getProxyHost()); - - if (!responseWriter.isBasic) { - responseWriter.writePlayers(event.getResponse().getPlayers()); - } - - // Send the response - ctx.writeAndFlush(responsePacket, ctx.voidPromise()); - }, ctx.channel().eventLoop()); - - break; + // Check which query response client expects + if (queryMessage.readableBytes() != 0 && queryMessage.readableBytes() != 4) { + return; } - default: - throw new IllegalStateException("Invalid query type: " + type); + + // Build initial query response + QueryResponse response = createInitialResponse(); + boolean isBasic = queryMessage.isReadable(); + + // Call event and write response + server.getEventManager() + .fire(new ProxyQueryEvent(isBasic ? BASIC : FULL, senderAddress, response)) + .whenCompleteAsync((event, exc) -> { + // Packet header + ByteBuf queryResponse = ctx.alloc().buffer(); + queryResponse.writeByte(QUERY_TYPE_STAT); + queryResponse.writeInt(sessionId); + + // Start writing the response + ResponseWriter responseWriter = new ResponseWriter(queryResponse, isBasic); + responseWriter.write("hostname", event.getResponse().getHostname()); + responseWriter.write("gametype", "SMP"); + + responseWriter.write("game_id", "MINECRAFT"); + responseWriter.write("version", event.getResponse().getGameVersion()); + responseWriter.writePlugins(event.getResponse().getProxyVersion(), + event.getResponse().getPlugins()); + + responseWriter.write("map", event.getResponse().getMap()); + responseWriter.write("numplayers", event.getResponse().getCurrentPlayers()); + responseWriter.write("maxplayers", event.getResponse().getMaxPlayers()); + responseWriter.write("hostport", event.getResponse().getProxyPort()); + responseWriter.write("hostip", event.getResponse().getProxyHost()); + + if (!responseWriter.isBasic) { + responseWriter.writePlayers(event.getResponse().getPlayers()); + } + + // Send the response + DatagramPacket responsePacket = new DatagramPacket(queryResponse, msg.sender()); + ctx.writeAndFlush(responsePacket, ctx.voidPromise()); + }, ctx.channel().eventLoop()); + break; } - } catch (Exception e) { - logger.warn("Error while trying to handle a query packet from {}", msg.sender(), e); - // NB: Only need to explicitly release upon exception, writing the response out will decrement - // the reference count. - responsePacket.release(); + default: + // Invalid query type - just don't respond } } @@ -191,20 +179,13 @@ public class GS4QueryHandler extends SimpleChannelInboundHandler } private List getRealPluginInformation() { - // Effective Java, Third Edition; Item 83: Use lazy initialization judiciously - List res = pluginInformationList; - if (res == null) { - synchronized (this) { - if (pluginInformationList == null) { - pluginInformationList = res = server.getPluginManager().getPlugins().stream() - .map(PluginContainer::getDescription) - .map(desc -> QueryResponse.PluginInformation - .of(desc.getName().orElse(desc.getId()), desc.getVersion().orElse(null))) - .collect(Collectors.toList()); - } - } + List result = new ArrayList<>(); + for (PluginContainer plugin : server.getPluginManager().getPlugins()) { + PluginDescription description = plugin.getDescription(); + result.add(QueryResponse.PluginInformation.of(description.getName() + .orElse(description.getId()), description.getVersion().orElse(null))); } - return res; + return result; } private static class ResponseWriter {