WebSocket frame decoding: replace byte array[4] frame mask with int. (n…

…etty#12989) Motivation: Make implementation consistent with recent changes on WebSocket08FrameEncoder netty#12969. Modifications: WebSocket08FrameDecoder: replace frame mask byte array[4] with int. Result: Simpler implementation consistent with WebSocket08FrameEncoder.
trustin · Nov 18, 2022 · e8df52e · e8df52e
1 parent 6e935e0
commit e8df52e
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 26 deletions.
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocket08FrameDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocket08FrameDecoder.java
@@ -101,7 +101,7 @@ enum State {
     private int frameRsv;
     private int frameOpcode;
     private long framePayloadLength;
-    private byte[] maskingKey;
+    private int mask;
     private int framePayloadLen1;
     private boolean receivedClosingHandshake;
     private State state = State.READING_FIRST;
@@ -300,10 +300,7 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) t
                 if (in.readableBytes() < 4) {
                     return;
                 }
-                if (maskingKey == null) {
-                    maskingKey = new byte[4];
-                }
-                in.readBytes(maskingKey);
+                mask = in.readInt();
             }
             state = State.PAYLOAD;
         case PAYLOAD:
@@ -398,20 +395,11 @@ private void unmask(ByteBuf frame) {
 
         ByteOrder order = frame.order();
 
-        // Remark: & 0xFF is necessary because Java will do signed expansion from
-        // byte to int which we don't want.
-        long longMask = (long) (maskingKey[0] & 0xff) << 24
-                        | (maskingKey[1] & 0xff) << 16
-                        | (maskingKey[2] & 0xff) << 8
-                        | (maskingKey[3] & 0xff);
+        int intMask = mask;
+        // Avoid sign extension on widening primitive conversion
+        long longMask = (long) intMask & 0xFFFFFFFFL;
         longMask |= longMask << 32;
 
-        // If the byte order of our buffers it little endian we have to bring our mask
-        // into the same format, because getInt() and writeInt() will use a reversed byte order
-        if (order == ByteOrder.LITTLE_ENDIAN) {
-            longMask = Long.reverseBytes(longMask);
-        }
-
         for (int lim = end - 7; i < lim; i += 8) {
             frame.setLong(i, frame.getLong(i) ^ longMask);
         }
@@ -421,9 +409,13 @@ private void unmask(ByteBuf frame) {
             i += 4;
         }
 
+        if (order == ByteOrder.LITTLE_ENDIAN) {
+            intMask = Integer.reverseBytes(intMask);
+        }
+
         int maskOffset = 0;
         for (; i < end; i++) {
-            frame.setByte(i, frame.getByte(i) ^ maskingKey[maskOffset++ & 3]);
+            frame.setByte(i, frame.getByte(i) ^ WebSocketUtil.byteAtIndex(intMask, maskOffset++ & 3));
         }
     }
 

diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocket08FrameEncoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocket08FrameEncoder.java
@@ -61,7 +61,6 @@
 import io.netty.util.internal.logging.InternalLogger;
 import io.netty.util.internal.logging.InternalLoggerFactory;
 
-import java.nio.ByteBuffer;
 import java.nio.ByteOrder;
 import java.util.List;
 
@@ -186,8 +185,10 @@ protected void encode(ChannelHandlerContext ctx, WebSocketFrame msg, List<Object
                     int end = data.writerIndex();
 
                     if (srcOrder == dstOrder) {
-                        // Use the optimized path only when byte orders match
-                        long longMask = mask | (long) mask << 32;
+                        // Use the optimized path only when byte orders match.
+                        // Avoid sign extension on widening primitive conversion
+                        long longMask = (long) mask & 0xFFFFFFFFL;
+                        longMask |= longMask << 32;
 
                         // If the byte order of our buffers it little endian we have to bring our mask
                         // into the same format, because getInt() and writeInt() will use a reversed byte order
@@ -207,7 +208,7 @@ protected void encode(ChannelHandlerContext ctx, WebSocketFrame msg, List<Object
                     int maskOffset = 0;
                     for (; i < end; i++) {
                         byte byteData = data.getByte(i);
-                        buf.writeByte(byteData ^ byteAtIndex(mask, maskOffset++ & 3));
+                        buf.writeByte(byteData ^ WebSocketUtil.byteAtIndex(mask, maskOffset++ & 3));
                     }
                 }
                 out.add(buf);
@@ -228,8 +229,4 @@ protected void encode(ChannelHandlerContext ctx, WebSocketFrame msg, List<Object
             }
         }
     }
-
-    private static int byteAtIndex(int mask, int index) {
-        return (mask >> 8 * (3 - index)) & 0xFF;
-    }
 }
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketUtil.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketUtil.java
@@ -160,6 +160,10 @@ static int randomNumber(int minimum, int maximum) {
         return (int) (minimum + fraction * (maximum - minimum));
     }
 
+    static int byteAtIndex(int mask, int index) {
+        return (mask >> 8 * (3 - index)) & 0xFF;
+    }
+
     /**
      * A private constructor to ensure that instances of this class cannot be made
      */