From 75d08d405d5e514bf3a21641798905cc1deb2aff Mon Sep 17 00:00:00 2001
From: Volodymyr Masliy <volodymyr.masliy@symphony-solutions.eu>
Date: Fri, 17 May 2019 11:22:14 +0300
Subject: [PATCH] Fix cross-namespace broadcast happens on single ns

Given mutable nature of Packet object and a fact that single Packet
instance is created in BroadcastOperations before sending it all
clients, when event is emitted to room where clients with different
namespaces are joined, then all of them will receive that event on
single namespace.

By using withNs method, Packet copy will be created, effectively, solving
the issue
---
 .../socketio/protocol/Packet.java             | 22 +++++++
 .../socketio/transport/NamespaceClient.java   |  4 +-
 .../socketio/protocol/PacketTest.java         | 61 +++++++++++++++++++
 3 files changed, 85 insertions(+), 2 deletions(-)
 create mode 100644 src/test/java/com/corundumstudio/socketio/protocol/PacketTest.java

diff --git a/src/main/java/com/corundumstudio/socketio/protocol/Packet.java b/src/main/java/com/corundumstudio/socketio/protocol/Packet.java
index 26f324c..6360278 100644
--- a/src/main/java/com/corundumstudio/socketio/protocol/Packet.java
+++ b/src/main/java/com/corundumstudio/socketio/protocol/Packet.java
@@ -77,6 +77,28 @@ public class Packet implements Serializable {
         return (T)data;
     }
 
+    /**
+     * Creates a copy of #{@link Packet} with new namespace set
+     * if it differs from current namespace.
+     * Otherwise, returns original object unchanged
+     */
+    public Packet withNsp(String namespace) {
+        if (this.nsp.equalsIgnoreCase(namespace)) {
+            return this;
+        } else {
+            Packet newPacket = new Packet(this.type);
+            newPacket.setAckId(this.ackId);
+            newPacket.setData(this.data);
+            newPacket.setDataSource(this.dataSource);
+            newPacket.setName(this.name);
+            newPacket.setSubType(this.subType);
+            newPacket.setNsp(namespace);
+            newPacket.attachments = this.attachments;
+            newPacket.attachmentsCount = this.attachmentsCount;
+            return newPacket;
+        }
+    }
+
     public void setNsp(String endpoint) {
         this.nsp = endpoint;
     }
diff --git a/src/main/java/com/corundumstudio/socketio/transport/NamespaceClient.java b/src/main/java/com/corundumstudio/socketio/transport/NamespaceClient.java
index 68e7474..43e97ec 100644
--- a/src/main/java/com/corundumstudio/socketio/transport/NamespaceClient.java
+++ b/src/main/java/com/corundumstudio/socketio/transport/NamespaceClient.java
@@ -104,8 +104,8 @@ public class NamespaceClient implements SocketIOClient {
         if (!isConnected()) {
             return;
         }
-        packet.setNsp(namespace.getName());
-        baseClient.send(packet);
+
+        baseClient.send(packet.withNsp(namespace.getName()));
     }
 
     public void onDisconnect() {
diff --git a/src/test/java/com/corundumstudio/socketio/protocol/PacketTest.java b/src/test/java/com/corundumstudio/socketio/protocol/PacketTest.java
new file mode 100644
index 0000000..57f5b46
--- /dev/null
+++ b/src/test/java/com/corundumstudio/socketio/protocol/PacketTest.java
@@ -0,0 +1,61 @@
+package com.corundumstudio.socketio.protocol;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+
+import io.netty.buffer.Unpooled;
+import org.junit.Test;
+
+public class PacketTest {
+
+    @Test
+    public void packetCopyIsCreatedWhenNamespaceDiffers() {
+        Packet oldPacket = createPacket();
+
+        String newNs = "new";
+        Packet newPacket = oldPacket.withNsp(newNs);
+        assertEquals(newNs, newPacket.getNsp());
+        assertPacketCopied(oldPacket, newPacket);
+    }
+
+    @Test
+    public void packetCopyIsCreatedWhenNewNamespaceDiffersAndIsNull() {
+        Packet packet = createPacket();
+        Packet newPacket = packet.withNsp(null);
+        assertNull(newPacket.getNsp());
+        assertPacketCopied(packet, newPacket);
+    }
+
+    @Test
+    public void originalPacketReturnedIfNamespaceIsTheSame() {
+        Packet packet = new Packet(PacketType.MESSAGE);
+        assertSame(packet, packet.withNsp(""));
+    }
+
+    private void assertPacketCopied(Packet oldPacket, Packet newPacket) {
+        assertNotSame(newPacket, oldPacket);
+        assertEquals(oldPacket.getName(), newPacket.getName());
+        assertEquals(oldPacket.getType(), newPacket.getType());
+        assertEquals(oldPacket.getSubType(), newPacket.getSubType());
+        assertEquals(oldPacket.getAckId(), newPacket.getAckId());
+        assertEquals(oldPacket.getAttachments().size(), newPacket.getAttachments().size());
+        assertSame(oldPacket.getAttachments(), newPacket.getAttachments());
+        assertEquals(oldPacket.getData(), newPacket.getData());
+        assertSame(oldPacket.getDataSource(), newPacket.getDataSource());
+    }
+
+    private Packet createPacket() {
+        Packet packet = new Packet(PacketType.MESSAGE);
+        packet.setSubType(PacketType.EVENT);
+        packet.setName("packetName");
+        packet.setData("data");
+        packet.setAckId(1L);
+        packet.setNsp("old");
+        packet.setDataSource(Unpooled.wrappedBuffer(new byte[]{10}));
+        packet.initAttachments(1);
+        packet.addAttachment(Unpooled.wrappedBuffer(new byte[]{20}));
+        return packet;
+    }
+}
\ No newline at end of file