You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "hai-ben (via GitHub)" <gi...@apache.org> on 2026/09/03 20:38:45 UTC
[PR] [6.x] fix(MQTTWireFormat) rejecting valid 4-byte remaining lengths [activemq]
hai-ben opened a new pull request, #2517:
URL: https://github.com/apache/activemq/pull/2517
Increment in the check loop happens before the check, causing an error. Additionally this code is [practically duplicated here](https://github.com/apache/activemq/blob/main/activemq-mqtt/src/main/java/org/apache/activemq/transport/mqtt/MQTTCodec.java#L165-L167C18). That code has it correct, but it looks tricky to abstract into a shared class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] fix: MQTTWireFormat rejecting valid 4-byte remaining lengths [activemq]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon merged PR #2517:
URL: https://github.com/apache/activemq/pull/2517
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] fix: MQTTWireFormat rejecting valid 4-byte remaining lengths [activemq]
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on PR #2517:
URL: https://github.com/apache/activemq/pull/2517#issuecomment-5532711724
This is the correct fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [6.x] fix(MQTTWireFormat) rejecting valid 4-byte remaining lengths [activemq]
Posted by "hai-ben (via GitHub)" <gi...@apache.org>.
hai-ben commented on PR #2517:
URL: https://github.com/apache/activemq/pull/2517#issuecomment-5532594833
@mattrpav I've put an issue here: https://github.com/apache/activemq/issues/2518
I was able to reproduce with paho-mqtt and MQTT.js. I don't see a reason other clients wouldn't experience this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] fix: MQTTWireFormat rejecting valid 4-byte remaining lengths [activemq]
Posted by "hai-ben (via GitHub)" <gi...@apache.org>.
hai-ben commented on PR #2517:
URL: https://github.com/apache/activemq/pull/2517#issuecomment-5705844893
Can we also backport this to 5.19?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] fix: MQTTWireFormat rejecting valid 4-byte remaining lengths [activemq]
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on code in PR #2517:
URL: https://github.com/apache/activemq/pull/2517#discussion_r3929100284
##########
activemq-mqtt/src/test/java/org/apache/activemq/transport/mqtt/MQTTCodecTest.java:
##########
@@ -364,4 +364,51 @@ public void testUnmarshalInvalidRemainingLengthField() {
}
}
+ @Test
+ public void testCodecAcceptsFourByteRemainingLength() throws Exception {
+ // 2,097,152 is the smallest value that requires a 4-byte Remaining Length.
+ // Payload size = target RL - variable header overhead (2 topic length + topic + 2 packet id)
+ final String topic = "TOPIC";
+ final int overhead = 2 + topic.length() + 2; // topic length field + topic + message id (QoS 1)
+ final int payloadSize = 2_097_152 - overhead;
+
+ PUBLISH publish = new PUBLISH();
+ publish.qos(QoS.AT_LEAST_ONCE);
+ publish.messageId((short) 1);
+ publish.topicName(new UTF8Buffer(topic));
+ publish.payload(new Buffer(new byte[payloadSize]));
+
+ DataByteArrayOutputStream output = new DataByteArrayOutputStream();
+ wireFormat.marshal(publish.encode(), output);
+ Buffer marshalled = output.toBuffer();
+
+ DataByteArrayInputStream input = new DataByteArrayInputStream(marshalled);
+ codec.parse(input, marshalled.length());
+
+ assertEquals("Expected one frame from a valid 4-byte Remaining Length", 1, frames.size());
+ PUBLISH decoded = new PUBLISH().decode(frames.get(0));
+ assertEquals(payloadSize, decoded.payload().length());
+ }
+
+ @Test
+ public void testUnmarshalAcceptsFourByteRemainingLength() throws Exception {
+ final String topic = "TOPIC";
+ final int overhead = 2 + topic.length() + 2;
+ final int payloadSize = 2_097_152 - overhead;
+
+ PUBLISH publish = new PUBLISH();
+ publish.qos(QoS.AT_LEAST_ONCE);
+ publish.messageId((short) 1);
+ publish.topicName(new UTF8Buffer(topic));
+ publish.payload(new Buffer(new byte[payloadSize]));
+
+ DataByteArrayOutputStream output = new DataByteArrayOutputStream();
Review Comment:
try-with-resources here
##########
activemq-mqtt/src/test/java/org/apache/activemq/transport/mqtt/MQTTCodecTest.java:
##########
@@ -364,4 +364,51 @@ public void testUnmarshalInvalidRemainingLengthField() {
}
}
+ @Test
+ public void testCodecAcceptsFourByteRemainingLength() throws Exception {
+ // 2,097,152 is the smallest value that requires a 4-byte Remaining Length.
+ // Payload size = target RL - variable header overhead (2 topic length + topic + 2 packet id)
+ final String topic = "TOPIC";
+ final int overhead = 2 + topic.length() + 2; // topic length field + topic + message id (QoS 1)
+ final int payloadSize = 2_097_152 - overhead;
+
+ PUBLISH publish = new PUBLISH();
+ publish.qos(QoS.AT_LEAST_ONCE);
+ publish.messageId((short) 1);
+ publish.topicName(new UTF8Buffer(topic));
+ publish.payload(new Buffer(new byte[payloadSize]));
+
+ DataByteArrayOutputStream output = new DataByteArrayOutputStream();
+ wireFormat.marshal(publish.encode(), output);
+ Buffer marshalled = output.toBuffer();
+
+ DataByteArrayInputStream input = new DataByteArrayInputStream(marshalled);
Review Comment:
try-with-resources here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] fix: MQTTWireFormat rejecting valid 4-byte remaining lengths [activemq]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on PR #2517:
URL: https://github.com/apache/activemq/pull/2517#issuecomment-5533241664
I think this is good to merge as long as CI passes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] fix: MQTTWireFormat rejecting valid 4-byte remaining lengths [activemq]
Posted by "hai-ben (via GitHub)" <gi...@apache.org>.
hai-ben commented on PR #2517:
URL: https://github.com/apache/activemq/pull/2517#issuecomment-5532816246
@mattrpav added the try blocks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [6.x] fix(MQTTWireFormat) rejecting valid 4-byte remaining lengths [activemq]
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on PR #2517:
URL: https://github.com/apache/activemq/pull/2517#issuecomment-5532474534
Do you know which MQTT client and library was used?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] fix: MQTTWireFormat rejecting valid 4-byte remaining lengths [activemq]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on PR #2517:
URL: https://github.com/apache/activemq/pull/2517#issuecomment-5705993192
> Can we also backport this to 5.19?
It already was and is targeted for 5.19.12: https://github.com/apache/activemq/pull/2522
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [6.x] fix(MQTTWireFormat) rejecting valid 4-byte remaining lengths [activemq]
Posted by "sergio-d-lemos (via GitHub)" <gi...@apache.org>.
sergio-d-lemos commented on PR #2517:
URL: https://github.com/apache/activemq/pull/2517#issuecomment-5531933849
LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact