jbertram commented on a change in pull request #3907:
URL: https://github.com/apache/activemq-artemis/pull/3907#discussion_r792925945



##########
File path: 
artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java
##########
@@ -185,45 +243,83 @@ public void handleBuffer(RemotingConnection connection, 
ActiveMQBuffer buffer) {
    @Override
    public void addChannelHandlers(ChannelPipeline pipeline) {
       pipeline.addLast(MqttEncoder.INSTANCE);
-      pipeline.addLast(new MqttDecoder(MQTTUtil.MAX_MESSAGE_SIZE));
+      /*
+       * If we use the value from getMaximumPacketSize() here anytime a client 
sends a packet that's too large it
+       * will receive a DISCONNECT with a reason code of 0x81 instead of 0x95 
like it should according to the spec.

Review comment:
       My goal with this commit was to be as close to the spec as possible. I 
wanted to "get it right" first and then later "make it fast" if necessary. In 
any event, computing the size of the message is pretty cheap since it's just a 
small bit of simple arithmetic. See 
`org.apache.activemq.artemis.core.protocol.mqtt.MQTTUtil#calculateMessageSize`.
   
   I'm certainly open to sending a PR to Netty for this, but that it outside 
the scope of this PR.




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to