From ec2865bfabcd013823c61002bd863eea99d17ea7 Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon" Date: Mon, 27 Jul 2026 18:23:02 -0400 Subject: [PATCH 1/2] Fix AMQP Object message decompression This fixes the AmqpMessageSupport utility to correctly decompress the body of Object Message's in all cases. Previously the decompression could stop early and not decompress the entire stream of data. This was due to the loop incorrectly casting the read int from the inflater stream as a byte before comparing to -1 to look for end of stream. This is incorrect because the read() method can return a value between -1 and 255. Java uses Two's Complement to represent signed ints, so if the returned value int is 255 and is cast to a byte it becomes -1. This meant that reading 255 would return -1 leading to the code to exit thinking end of stream has been reached. --- .../amqp/message/AmqpMessageSupport.java | 10 ++++++-- .../JMSMappingOutboundTransformerTest.java | 25 ++++++++++++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/activemq-amqp/src/main/java/org/apache/activemq/transport/amqp/message/AmqpMessageSupport.java b/activemq-amqp/src/main/java/org/apache/activemq/transport/amqp/message/AmqpMessageSupport.java index 102a1701a5e..49542337b1f 100644 --- a/activemq-amqp/src/main/java/org/apache/activemq/transport/amqp/message/AmqpMessageSupport.java +++ b/activemq-amqp/src/main/java/org/apache/activemq/transport/amqp/message/AmqpMessageSupport.java @@ -256,8 +256,14 @@ public static Binary getBinaryFromMessageBody(ActiveMQObjectMessage message) thr InputStream iis = MarshallingSupport.createInflaterInputStream( message.getMaxInflatedDataSize(), is)) { - byte value; - while ((value = (byte) iis.read()) != -1) { + // Keep the value as an int and don't cast to a byte. + // The read() method can return a int between -1 and 255 and + // previously the returned value was being cast to a byte. + // If the returned int happens to be 255, then casting that to a byte + // causes the value to become -1 (java use's Two's Complement for ints) + // so the loop will think we reached end of stream too early + int value; + while ((value = iis.read()) != -1) { os.write(value); } diff --git a/activemq-amqp/src/test/java/org/apache/activemq/transport/amqp/message/JMSMappingOutboundTransformerTest.java b/activemq-amqp/src/test/java/org/apache/activemq/transport/amqp/message/JMSMappingOutboundTransformerTest.java index 368649056b9..7e5d48bf3b7 100644 --- a/activemq-amqp/src/test/java/org/apache/activemq/transport/amqp/message/JMSMappingOutboundTransformerTest.java +++ b/activemq-amqp/src/test/java/org/apache/activemq/transport/amqp/message/JMSMappingOutboundTransformerTest.java @@ -583,9 +583,32 @@ public void testConvertObjectMessageToAmqpMessageWithAmqpValueBody() throws Exce assertTrue(value instanceof UUID); } + @Test + public void testConvertCompressedObjectMessageToAmqpMessageByte255() throws Exception { + // This specific UUID tests a decompression edge case found for Objects messages and + // is used to verify AmqpMessageSupport.getBinaryFromMessageBody() is correct. + // + // Previously the decompression could stop early and not decompress the entire stream of + // data. This was due to the loop incorrectly casting the read int from the inflater + // stream as a byte before comparing to -1 to look for end of stream. + // + // This is incorrect because the read() method can return a value between -1 and 255. + // Java uses Two's Complement to represent signed ints, so if the returned value int + // is 255 and is cast to a byte it becomes -1. This meant that reading 255 would return -1 + // leading to the code to exit thinking end of stream has been reached. + // + // This particular UUID includes a byte of 255 when decompressed to test this edge case. + testConvertCompressedObjectMessageToAmqpMessageWithDataBody( + UUID.fromString("14faffdc-387d-4e2e-8b44-748d47eaaf06")); + } + @Test public void testConvertCompressedObjectMessageToAmqpMessageWithDataBody() throws Exception { - ActiveMQObjectMessage outbound = createObjectMessage(TEST_OBJECT_VALUE, true); + testConvertCompressedObjectMessageToAmqpMessageWithDataBody(TEST_OBJECT_VALUE); + } + + private void testConvertCompressedObjectMessageToAmqpMessageWithDataBody(UUID uuid) throws Exception { + ActiveMQObjectMessage outbound = createObjectMessage(uuid, true); outbound.onSend(); outbound.storeContent(); From 2226fbca31c28867db5785c5e359f4023f03b7b5 Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon" Date: Mon, 27 Jul 2026 18:37:20 -0400 Subject: [PATCH 2/2] Remove unnecessary comment - explanation is in PR and git commit --- .../activemq/transport/amqp/message/AmqpMessageSupport.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/activemq-amqp/src/main/java/org/apache/activemq/transport/amqp/message/AmqpMessageSupport.java b/activemq-amqp/src/main/java/org/apache/activemq/transport/amqp/message/AmqpMessageSupport.java index 49542337b1f..6b6e0347181 100644 --- a/activemq-amqp/src/main/java/org/apache/activemq/transport/amqp/message/AmqpMessageSupport.java +++ b/activemq-amqp/src/main/java/org/apache/activemq/transport/amqp/message/AmqpMessageSupport.java @@ -256,12 +256,6 @@ public static Binary getBinaryFromMessageBody(ActiveMQObjectMessage message) thr InputStream iis = MarshallingSupport.createInflaterInputStream( message.getMaxInflatedDataSize(), is)) { - // Keep the value as an int and don't cast to a byte. - // The read() method can return a int between -1 and 255 and - // previously the returned value was being cast to a byte. - // If the returned int happens to be 255, then casting that to a byte - // causes the value to become -1 (java use's Two's Complement for ints) - // so the loop will think we reached end of stream too early int value; while ((value = iis.read()) != -1) { os.write(value);