[activemq-artemis] branch master updated: ARTEMIS-2500 CoreMessage doesn't make a ful copy of its props on moveHeadersAndProperties

[cl...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

clebertsuconic pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git


The following commit(s) were added to refs/heads/master by this push:
     new a250428  ARTEMIS-2500 CoreMessage doesn't make a ful copy of its props on moveHeadersAndProperties
     new 7bde870  This closes #2846
a250428 is described below

commit a2504288c6dd90f31a8c8f6f01f546e172565282
Author: Howard Gao <ho...@gmail.com>
AuthorDate: Mon Sep 23 11:55:38 2019 +0800

    ARTEMIS-2500 CoreMessage doesn't make a ful copy of its props on moveHeadersAndProperties
    
    When CoreMessage is doing copyHeadersAndProperties() it doesn't
    make a full copy of its properties (a TypedProperties object).
    It will cause problem when multiple threads/parties are modifying the
    properties of the copied messages from the same message.
    
    This will be particular bad if the message is a large message
    where moveHeadersAndProperties is being used.
---
 .../artemis/core/message/impl/CoreMessage.java     |  2 +-
 .../unit/core/message/impl/MessageImplTest.java    | 61 ++++++++++++++++++++++
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java
index 1a2353c..79559fe 100644
--- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java
+++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java
@@ -461,7 +461,7 @@ public class CoreMessage extends RefCountMessage implements ICoreMessage {
       priority = msg.getPriority();
 
       if (msg instanceof CoreMessage) {
-         properties = ((CoreMessage) msg).getProperties();
+         properties = new TypedProperties(((CoreMessage) msg).getProperties());
       }
    }
 
diff --git a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/message/impl/MessageImplTest.java b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/message/impl/MessageImplTest.java
index 0e0ded8..047d5e5 100644
--- a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/message/impl/MessageImplTest.java
+++ b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/message/impl/MessageImplTest.java
@@ -37,6 +37,8 @@ import org.apache.activemq.artemis.core.protocol.core.impl.wireformat.SessionSen
 import org.apache.activemq.artemis.core.server.ActiveMQServer;
 import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
 import org.apache.activemq.artemis.utils.RandomUtil;
+import org.apache.activemq.artemis.utils.UUID;
+import org.apache.activemq.artemis.utils.UUIDGenerator;
 import org.apache.activemq.artemis.utils.Wait;
 import org.junit.Assert;
 import org.junit.Test;
@@ -240,6 +242,65 @@ public class MessageImplTest extends ActiveMQTestBase {
       }
    }
 
+   @Test
+   public void testMessageCopyHeadersAndProperties() {
+      CoreMessage msg1 = new CoreMessage(123, 18);
+      SimpleString address = new SimpleString("address");
+      msg1.setAddress(address);
+      UUID uid = UUIDGenerator.getInstance().generateUUID();
+      msg1.setUserID(uid);
+      byte type = 3;
+      msg1.setType(type);
+      boolean durable = true;
+      msg1.setDurable(durable);
+      long expiration = System.currentTimeMillis();
+      msg1.setExpiration(expiration);
+      long timestamp = System.currentTimeMillis();
+      msg1.setTimestamp(timestamp);
+      byte priority = 9;
+      msg1.setPriority(priority);
+
+      String routeTo = "_HQ_ROUTE_TOsomething";
+      String value = "Byte array substitute";
+      msg1.putStringProperty(routeTo, value);
+
+      CoreMessage msg2 = new CoreMessage(456, 18);
+
+      msg2.copyHeadersAndProperties(msg1);
+
+      assertEquals(msg1.getAddress(), msg2.getAddress());
+      assertEquals(msg1.getUserID(), msg2.getUserID());
+      assertEquals(msg1.getType(), msg2.getType());
+      assertEquals(msg1.isDurable(), msg2.isDurable());
+      assertEquals(msg1.getExpiration(), msg2.getExpiration());
+      assertEquals(msg1.getTimestamp(), msg2.getTimestamp());
+      assertEquals(msg1.getPriority(), msg2.getPriority());
+
+      assertEquals(value, msg2.getStringProperty(routeTo));
+
+      //now change the property on msg2 shouldn't affect msg1
+      msg2.setAddress(address.concat("new"));
+      msg2.setUserID(UUIDGenerator.getInstance().generateUUID());
+      msg2.setType(--type);
+      msg2.setDurable(!durable);
+      msg2.setExpiration(expiration + 1000);
+      msg2.setTimestamp(timestamp + 1000);
+      msg2.setPriority(--priority);
+
+      msg2.putStringProperty(routeTo, value + "new");
+
+      assertNotEquals(msg1.getAddress(), msg2.getAddress());
+      assertNotEquals(msg1.getUserID(), msg2.getUserID());
+      assertNotEquals(msg1.getType(), msg2.getType());
+      assertNotEquals(msg1.isDurable(), msg2.isDurable());
+      assertNotEquals(msg1.getExpiration(), msg2.getExpiration());
+      assertNotEquals(msg1.getTimestamp(), msg2.getTimestamp());
+      assertNotEquals(msg1.getPriority(), msg2.getPriority());
+
+      assertNotEquals(msg1.getStringProperty(routeTo), msg2.getStringProperty(routeTo));
+
+   }
+
    private void internalMessageCopy() throws Exception {
       final long RUNS = 2;
       final CoreMessage msg = new CoreMessage(123, 18);