Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2490#discussion_r245955337
--- Diff:
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/wireformat/SessionCreateConsumerMessage.java
---
@@ -104,6 +119,11 @@ public void decodeRest(final ActiveMQBuffer buffer) {
filterString = buffer.readNullableSimpleString();
browseOnly = buffer.readBoolean();
requiresResponse = buffer.readBoolean();
+ if (buffer.readableBytes() > 0) {
--- End diff --
I assume this is to allow for old clients that don't send this value. Would
a more specific version check be clearer here for later reference? Related, I'm
guessing other changes already made for 2.7.0 have updated the version info
since it doesn't look to change here?
Also, is the reverse case safe, does an older server failing to read the
additional value (seemingly always sent now) have potential to lead to any
issues on older servers, i.e how might the buffer continue to be used later if
at all? Should the client omit the value for older servers? (Or does the
presumed version change prevent the new client working with the old server
anyway? I don't know how that stuff is handled, just commenting from reading
the diff here).
---
