Repository: netty/netty
Issue: Completely broken WebSocket08FrameEncoder (v13 broken too) (includes 1 line fix)
Labels: defect

Bug exists in 4.0.0.Alpha1 AND verified it still exists in master. Didn't check previous to those.

Symptom:
- Other end will get an undecodable garbage payload from a netty WebSocket client.

Cause:
- Netty picks two random numbers. One random number is stored in the mask field in the WebSocket frame that is sent to the other end. The other random number is used as the mask in the code that masks the payload data. There is a very low chance of course that these two random numbers will be the same and thus a low chance that the other end will get anything but garbage when attempting to decode the payload.

Solution:
- Only generate one random number. Use that same random number as the mask value sent to the other end and as the mask value that the payload is actually encoded with.
## Diff:

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
index f32d01a..86862c2 100644
--- 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
@@ -166,7 +166,7 @@ public class WebSocket08FrameEncoder extends MessageToByteEncoder<WebSocketFrame
         if (maskPayload) {
             int random = (int) (Math.random() \* Integer.MAX_VALUE);
             mask = ByteBuffer.allocate(4).putInt(random).array();
-            out.writeInt((int) (Math.random() \* Integer.MAX_VALUE));
- ```
         out.writeInt(random);
  
       int counter = 0;
       for (int i = data.readerIndex(); i < data.writerIndex(); i ++) {
  ```
  ---
- Sam Maloney samgih@codingninjas.org


Issue comments:
@trustin @veebs @fredericBregier wdyt ?

Sorry. Saw the issue but been a bit busy with work. Will make some time to look at it tomorrow. 

Sent from my iPhone

On 18/07/2012, at 5:57 PM, Norman Maurerreply@reply.github.com wrote:

> @trustin @veebs @fredericBregier wdyt ?
> 
> ---
> 
> Reply to this email directly or view it on GitHub:
> https://github.com/netty/netty/issues/456#issuecomment-7059069

I looked at some RFC, and it seems that the proposal is correct.
See http://tools.ietf.org/html/draft-ietf-hybi-thewebsocketprotocol-08 chapter 4.3.
The mask sent should be the same used in the next writes if payload is used.
So I propose to validate this fix.

NB: in branch 3, the code is as proposed (create only one RANDOM and use it)

Let's merge in.  Thanks @spullara ! :-)

By the way, the issue is referenced to 3.5.3 but it is related to 4...

@fredericBregier so only 4 is effected atm ?

Yes, in branch 3, we already have :

```
 Integer random = (int) (Math.random() * Integer.MAX_VALUE);
 mask = ByteBuffer.allocate(4).putInt(random).array();
 header.writeBytes(mask);
```

And I just proposed a pull request for master...
I changed the writeInt to writeBytes... and I justsee that I made a typo error while looking at source 3 to master... 

@fredericBregier Ok.. In the description of this issue he mention that the problem also exist in V13.. Did you check this too ?

In fact v13 is just an extend of v08 with no additional code, so fixing v08 is enough.
(Valid also in branch 3)

@garypart Maybe you could try to download the last master branch version or applied the small fix and check if it fixes your issue correctly ?

Thanks everyone.


Pull request review comments:

Patch that resolved the issue:
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
index f32d01a2d05..715123a311d 100644
--- 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
@@ -166,7 +166,7 @@ public void encode(ChannelHandlerContext ctx,
         if (maskPayload) {
             int random = (int) (Math.random() * Integer.MAX_VALUE);
             mask = ByteBuffer.allocate(4).putInt(random).array();
-            out.writeInt((int) (Math.random() * Integer.MAX_VALUE));
+            header.writeBytes(mask);
 
             int counter = 0;
             for (int i = data.readerIndex(); i < data.writerIndex(); i ++) {


Test patch: