Server: Upgrade Netty to version 4.1

The final release of the Netty 3.x line was in 2016 and ever since, it has been EOL (https://netty.io/news/2016/06/29/3-10-6-Final.html). Nowadays it has multiple unpatched security issues. This patch migrates the server to the latest Netty release. 4.1.112. The 4.0 branch is skipped since it is also EOL since 2018. There are a bunch of breaking changes between the versions: - Channel attachments were removed in favor of attributes, so now ClientState and Player objects are stored as attributes - `flush` needs to be called to actually send messages to clients so all calls to `write` are replaced with `writeAndFlush`. In theory this can be optimized to buffer multiple writes but since the server is not performance critical, I opted to keep it simple and always flush writes. - All Netty imports had their package changed from org.jboss to io.netty - ChannelBuffer was renamed to ByteBuf - A bunch of handler methods were renamed (e.g. channelConnected -> channelActive, messageReceived -> channelRead, channelClosed -> channelInactive) - IdleStateAwareChannelHandler was removed and now idle events are be processed by `userEventTriggered`. `getLastActivityTimeMillis` was also removed from idle events, so I reimplemented that functionality in ClientState. - OneToOneDecoder and OneToOneEncoder were replaced with more strongly typed classes, ByteToMessageDecoder and MessageToByteEncoder. They no longer return values, instead they add to the output list passed in as a parameter. - The ServerBootstrap API was redesigned to use a fluent API and a ChannelInitializer instead of a pipeline factory. The new API is mostly compatible with the old one though.
PhilippvK · Aug 4, 2024 · 4464e49 · 4464e49
1 parent 996141c
commit 4464e49
Show file tree

Hide file tree

Showing 31 changed files with 209 additions and 171 deletions.
diff --git a/pom.xml b/pom.xml
@@ -96,8 +96,8 @@
         <dependencies>
             <dependency>
                 <groupId>io.netty</groupId>
-                <artifactId>netty</artifactId> <!-- Use 'netty-all' for 4.0 or above -->
-                <version>3.10.6.Final</version>
+                <artifactId>netty-all</artifactId>
+                <version>4.1.112.Final</version>
                 <scope>compile</scope>
             </dependency>
             <dependency>

diff --git a/server/pom.xml b/server/pom.xml
@@ -19,7 +19,7 @@
     <dependencies>
         <dependency>
             <groupId>io.netty</groupId>
-            <artifactId>netty</artifactId> <!-- Use 'netty-all' for 4.0 or above -->
+            <artifactId>netty-all</artifactId>
             <scope>compile</scope>
         </dependency>
         <dependency>

diff --git a/server/src/main/java/org/moparforia/server/Server.java b/server/src/main/java/org/moparforia/server/Server.java
@@ -1,14 +1,20 @@
 package org.moparforia.server;
 
-import org.jboss.netty.bootstrap.ServerBootstrap;
-import org.jboss.netty.channel.*;
-import org.jboss.netty.channel.group.ChannelGroup;
-import org.jboss.netty.channel.group.DefaultChannelGroup;
-import org.jboss.netty.channel.socket.nio.NioServerSocketChannelFactory;
-import org.jboss.netty.handler.codec.frame.DelimiterBasedFrameDecoder;
-import org.jboss.netty.handler.codec.frame.Delimiters;
-import org.jboss.netty.handler.timeout.IdleStateHandler;
-import org.jboss.netty.util.HashedWheelTimer;
+import io.netty.bootstrap.ServerBootstrap;
+import io.netty.channel.Channel;
+import io.netty.channel.ChannelFuture;
+import io.netty.channel.ChannelInitializer;
+import io.netty.channel.ChannelOption;
+import io.netty.channel.EventLoopGroup;
+import io.netty.channel.group.ChannelGroup;
+import io.netty.channel.group.DefaultChannelGroup;
+import io.netty.channel.nio.NioEventLoopGroup;
+import io.netty.channel.socket.SocketChannel;
+import io.netty.channel.socket.nio.NioServerSocketChannel;
+import io.netty.handler.codec.DelimiterBasedFrameDecoder;
+import io.netty.handler.codec.Delimiters;
+import io.netty.handler.timeout.IdleStateHandler;
+import io.netty.util.concurrent.GlobalEventExecutor;
 import org.moparforia.server.event.Event;
 import org.moparforia.server.game.Lobby;
 import org.moparforia.server.game.LobbyType;
@@ -32,8 +38,6 @@
 import java.util.Iterator;
 import java.util.Optional;
 import java.util.concurrent.ConcurrentLinkedQueue;
-import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
 
 
 public class Server implements Runnable {
@@ -42,15 +46,15 @@ public class Server implements Runnable {
     public static final String DEFAULT_TRACKS_DIRECTORY = "tracks";
 
     private HashMap<Integer, Player> players = new HashMap<>();
-    private ChannelGroup allChannels = new DefaultChannelGroup();
+    private ChannelGroup allChannels = new DefaultChannelGroup(GlobalEventExecutor.INSTANCE);
     private ConcurrentLinkedQueue<Event> events = new ConcurrentLinkedQueue<>();
     private HashMap<PacketType, ArrayList<PacketHandler>> packetHandlers = new HashMap<>();
 
     private String host;
     private int port;
     private Optional<String> tracksDirectory;
 
-    private Channel serverChannel;
+    private ChannelFuture serverChannelFuture;
     private boolean running;
 
     private HashMap<LobbyType, Lobby> lobbies = new HashMap<LobbyType, Lobby>();
@@ -175,23 +179,29 @@ public void start() {
         packetHandlers = PacketHandlerFactory.getPacketHandlers();
         System.out.println("Loaded " + packetHandlers.size() + " packet handler type(s)");
 
-        ChannelFactory factory = new NioServerSocketChannelFactory(
-                Executors.newCachedThreadPool(),
-                Executors.newCachedThreadPool());
-
-        ServerBootstrap bootstrap = new ServerBootstrap(factory);
-        final ClientChannelHandler clientHandler = new ClientChannelHandler(this);
-        final IdleStateHandler idleState = new IdleStateHandler(new HashedWheelTimer(1, TimeUnit.SECONDS), 2, 0, 0);
-        bootstrap.setPipelineFactory(() -> Channels.pipeline(
-                new DelimiterBasedFrameDecoder(250, Delimiters.lineDelimiter()),
-                new PacketDecoder(),
-                new PacketEncoder(),
-                idleState,
-                clientHandler));
-        bootstrap.setOption("child.tcpNoDelay", true);
-        bootstrap.setOption("child.keepAlive", true);
+        EventLoopGroup bossGroup = new NioEventLoopGroup();
+        EventLoopGroup workerGroup = new NioEventLoopGroup();
+        ServerBootstrap bootstrap = new ServerBootstrap()
+            .group(bossGroup, workerGroup)
+            .channel(NioServerSocketChannel.class)
+            .childHandler(new ChannelInitializer<SocketChannel>() {
+                @Override
+                protected void initChannel(SocketChannel ch) {
+                    ch.attr(ClientState.CLIENT_STATE_ATTRIBUTE_KEY).set(new ClientState());
+                    ch.pipeline()
+                        .addLast(
+                            new DelimiterBasedFrameDecoder(250, Delimiters.lineDelimiter()),
+                            new PacketDecoder(),
+                            new PacketEncoder(),
+                            new IdleStateHandler(2, 0, 0),
+                            new ClientChannelHandler(Server.this)
+                            );
+                        }
+                })
+            .childOption(ChannelOption.TCP_NODELAY, true)
+            .childOption(ChannelOption.SO_KEEPALIVE, true);
         try {
-            this.serverChannel = bootstrap.bind(new InetSocketAddress(host, port));
+            this.serverChannelFuture = bootstrap.bind(new InetSocketAddress(host, port)).sync();
             this.running = true;
             new Thread(this).start();
         } catch (Exception ex) {
@@ -200,7 +210,7 @@ public void start() {
     }
 
     public void stop() throws InterruptedException {
-        this.serverChannel.close().sync();
+        this.serverChannelFuture.channel().close().sync();
         this.running = false;
     }
 

diff --git a/server/src/main/java/org/moparforia/server/event/ClientConnectedEvent.java b/server/src/main/java/org/moparforia/server/event/ClientConnectedEvent.java
@@ -1,6 +1,6 @@
 package org.moparforia.server.event;
 
-import org.jboss.netty.channel.Channel;
+import io.netty.channel.Channel;
 import org.moparforia.server.Server;
 
 import java.util.Random;
@@ -17,8 +17,7 @@ public ClientConnectedEvent(Channel channel) {
     public void process(Server server) {
         System.out.println("Client connected: " + channel);
         server.addChannel(channel);
-        //channel.write("h 1\nc io " + new Random().nextInt(1000000000) + "\nc crt 25\nc ctr\n");
-        channel.write("h 1\nc io " + new Random().nextInt(1000000000) + "\nc crt 250\nc ctr\n");
+        channel.writeAndFlush("h 1\nc io " + new Random().nextInt(1000000000) + "\nc crt 250\nc ctr\n");
     }
 
 }
diff --git a/server/src/main/java/org/moparforia/server/event/ClientDisconnectedEvent.java b/server/src/main/java/org/moparforia/server/event/ClientDisconnectedEvent.java
@@ -1,6 +1,6 @@
 package org.moparforia.server.event;
 
-import org.jboss.netty.channel.Channel;
+import io.netty.channel.Channel;
 import org.moparforia.server.Server;
 import org.moparforia.server.game.Lobby;
 import org.moparforia.server.game.Player;
@@ -15,8 +15,8 @@ public ClientDisconnectedEvent(Channel channel) {
 
     @Override
     public void process(Server server) {
-        Player player;
-        if ((player = (Player)channel.getAttachment()) != null) {
+        Player player = channel.attr(Player.PLAYER_ATTRIBUTE_KEY).get();
+        if (player != null) {
             if (player.getLobby() != null) {
               player.getLobby().removePlayer(player, Lobby.PART_REASON_USERLEFT);
             }

diff --git a/server/src/main/java/org/moparforia/server/game/Game.java b/server/src/main/java/org/moparforia/server/game/Game.java
@@ -1,6 +1,6 @@
 package org.moparforia.server.game;
 
-import org.jboss.netty.channel.Channel;
+import io.netty.channel.Channel;
 import org.moparforia.server.Server;
 import org.moparforia.server.game.gametypes.golf.MultiGame;
 import org.moparforia.server.net.Packet;
@@ -102,7 +102,7 @@ protected void sendJoinMessages(Player player) {
         sendGameInfo(player);
         sendPlayerNames(player);
         writeExcluding(player, new Packet(PacketType.DATA, Tools.tabularize("game", "join", playerCount(), player.getNick(), player.getClan())));
-        player.getChannel().write(new Packet(PacketType.DATA, Tools.tabularize("game", "owninfo", numberIndex, player.getNick(), player.getClan())));
+        player.getChannel().writeAndFlush(new Packet(PacketType.DATA, Tools.tabularize("game", "owninfo", numberIndex, player.getNick(), player.getClan())));
     }
 
     protected void sendPlayerNames(Player player) {
@@ -112,7 +112,7 @@ protected void sendPlayerNames(Player player) {
             if (!p.equals(player))
                 playersData += Tools.tabularize("", getPlayerId(p), p.getNick(), p.getClan());
         }
-        c.write(new Packet(PacketType.DATA, playersData));
+        c.writeAndFlush(new Packet(PacketType.DATA, playersData));
     }
 
 

diff --git a/server/src/main/java/org/moparforia/server/game/Lobby.java b/server/src/main/java/org/moparforia/server/game/Lobby.java
@@ -45,7 +45,7 @@ public boolean removePlayer(Player player, int partReason, String... gameName) {
         writeAll(new Packet(PacketType.DATA, cmd));
 
         if (player.getChannel().isWritable() && partReason == PART_REASON_USERLEFT) {
-            player.getChannel().write(new Packet(PacketType.DATA, "status\tlobbyselect\t300"));
+            player.getChannel().writeAndFlush(new Packet(PacketType.DATA, "status\tlobbyselect\t300"));
         }
         return true;
     }
@@ -54,23 +54,23 @@ public boolean addPlayer(Player player, int joinType) {
         if (player.getLobby() != null) {
             player.getLobby().removePlayer(player, PART_REASON_USERLEFT);
         }
-        player.getChannel().write(new Packet(PacketType.DATA, Tools.tabularize("status", "lobby", type + (player.isChatHidden() ? "h" : ""))));
+        player.getChannel().writeAndFlush(new Packet(PacketType.DATA, Tools.tabularize("status", "lobby", type + (player.isChatHidden() ? "h" : ""))));
         String[] otherPlayers = new String[playerCount()];
         int pointer = 0;
 
         for (Player p : getPlayers()) {
-            p.getChannel().write(new Packet(PacketType.DATA, Tools.tabularize(
+            p.getChannel().writeAndFlush(new Packet(PacketType.DATA, Tools.tabularize(
                     "lobby", joinType == JOIN_TYPE_NORMAL ? "join" : "joinfromgame", player.toString()//todo not sure if should be getNick or getGameString
             )));
             otherPlayers[pointer++] = p.toString();
         }
         if (pointer != 0) {
-            player.getChannel().write(new Packet(PacketType.DATA, Tools.tabularize("lobby", "users", otherPlayers)));
+            player.getChannel().writeAndFlush(new Packet(PacketType.DATA, Tools.tabularize("lobby", "users", otherPlayers)));
         } else {
-            player.getChannel().write(new Packet(PacketType.DATA, Tools.tabularize("lobby", "users")));
+            player.getChannel().writeAndFlush(new Packet(PacketType.DATA, Tools.tabularize("lobby", "users")));
         }
 
-        player.getChannel().write(new Packet(PacketType.DATA, Tools.tabularize("lobby", "ownjoin", player.toString())));
+        player.getChannel().writeAndFlush(new Packet(PacketType.DATA, Tools.tabularize("lobby", "ownjoin", player.toString())));
         if (getLobbyType() == LobbyType.MULTI) {
             sendGameList(player);
         }
@@ -89,7 +89,7 @@ public void sendGameList(Player player) {
                 length++;
             }
         }
-        player.getChannel().write(new Packet(PacketType.DATA, Tools.tabularize("lobby", "gamelist", "full", length, buff.toString())));
+        player.getChannel().writeAndFlush(new Packet(PacketType.DATA, Tools.tabularize("lobby", "gamelist", "full", length, buff.toString())));
     }
 
     public Game getGame(int id) {

diff --git a/server/src/main/java/org/moparforia/server/game/Player.java b/server/src/main/java/org/moparforia/server/game/Player.java
@@ -1,10 +1,12 @@
 package org.moparforia.server.game;
 
-import org.jboss.netty.channel.Channel;
+import io.netty.channel.Channel;
+import io.netty.util.AttributeKey;
 import org.moparforia.shared.Tools;
 
 public class Player {
 
+    public static final AttributeKey<Player> PLAYER_ATTRIBUTE_KEY = AttributeKey.newInstance("PLAYER_ATTRIBUTE");
     public static final int ACCESSLEVEL_NORMAL = 0; // todo: enum this ?
     public static final int ACCESSLEVEL_SHERIFF = 1;
     public static final int ACCESSLEVEL_ADMIN = 2;

diff --git a/server/src/main/java/org/moparforia/server/game/PlayerCollection.java b/server/src/main/java/org/moparforia/server/game/PlayerCollection.java
@@ -34,28 +34,28 @@ protected boolean removePlayer(Player player) {
 
     public void writeAll(Packet packet) {
         for (Player player : getPlayers()) {
-            player.getChannel().write(packet);
+            player.getChannel().writeAndFlush(packet);
         }
     }
 
     public void writeAll(String message) {
         for (Player player : getPlayers()) {
-            player.getChannel().write(message);
+            player.getChannel().writeAndFlush(message);
         }
     }
 
     public void writeExcluding(Player exclude, Packet packet) {
         for (Player player : getPlayers()) {
             if (player != exclude) {
-                player.getChannel().write(packet);
+                player.getChannel().writeAndFlush(packet);
             }
         }
     }
 
     public void writeExcluding(Player exclude, String message) {
         for (Player player : getPlayers()) {
             if (player != exclude) {
-                player.getChannel().write(message);
+                player.getChannel().writeAndFlush(message);
             }
         }
     }

diff --git a/server/src/main/java/org/moparforia/server/game/gametypes/GolfGame.java b/server/src/main/java/org/moparforia/server/game/gametypes/GolfGame.java
@@ -1,6 +1,6 @@
 package org.moparforia.server.game.gametypes;
 
-import org.jboss.netty.channel.Channel;
+import io.netty.channel.Channel;
 import org.moparforia.server.Server;
 import org.moparforia.server.game.Game;
 import org.moparforia.server.game.Lobby;
@@ -137,8 +137,8 @@ public void rateTrack(String rating) {
 
     public void sendGameInfo(Player player) {
         Channel c = player.getChannel();
-        c.write(new Packet(PacketType.DATA, Tools.tabularize("status", "game")));
-        c.write(new Packet(PacketType.DATA, Tools.tabularize("game", "gameinfo",
+        c.writeAndFlush(new Packet(PacketType.DATA, Tools.tabularize("status", "game")));
+        c.writeAndFlush(new Packet(PacketType.DATA, Tools.tabularize("game", "gameinfo",
                 name, passworded ? "t" : "f", gameId, numPlayers, tracks.size(),
                 tracksType, maxStrokes, strokeTimeout, waterEvent, collision, trackScoring,
                 trackScoringEnd, "f")));

diff --git a/server/src/main/java/org/moparforia/server/game/gametypes/golf/DualGame.java b/server/src/main/java/org/moparforia/server/game/gametypes/golf/DualGame.java
@@ -26,7 +26,7 @@ public DualGame(Player challenger, Player challenged, int gameId, int numberOfTr
                 false, numberOfTracks, -1, tracksType,
                 maxStrokes, strokeTimeout, waterEvent, collision,
                 trackScoring, trackScoringEnd, 2);
-        challenged.getChannel().write(new Packet(PacketType.DATA,
+        challenged.getChannel().writeAndFlush(new Packet(PacketType.DATA,
                 Tools.tabularize("lobby", "challenge", challenger.getNick(), numberOfTracks,
                         tracksType, maxStrokes, strokeTimeout, waterEvent,
                         collision, trackScoring, trackScoringEnd)));