Github user gemmellr commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2115#discussion_r191792711
  
    --- Diff: 
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java
 ---
    @@ -606,22 +606,36 @@ public String getAddress() {
     
        @Override
        public AMQPMessage setAddress(String address) {
    -      this.address = SimpleString.toSimpleString(address, 
coreMessageObjectPools == null ? null : 
coreMessageObjectPools.getAddressStringSimpleStringPool());
    +      internalSetAddress(address);
    +      setProtonAddress(address);
           return this;
        }
     
    +   private void internalSetAddress(String address) {
    +      this.address = SimpleString.toSimpleString(address, 
coreMessageObjectPools == null ? null : 
coreMessageObjectPools.getAddressStringSimpleStringPool());
    +   }
    +
        @Override
        public AMQPMessage setAddress(SimpleString address) {
           this.address = address;
    +      setProtonAddress(address.toString());
    --- End diff --
    
    I must be misinterpretting things then. The setAddress methods (again, 
there are two) appear used already in several places of note within the broker.
    
    For example, I interpret this bit as meaning every AMQP message received on 
a link with a target address will have it called on it, is that the case?
    
https://github.com/apache/activemq-artemis/blob/2.6.0/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java#L448
    
    If so, doesnt that mean that anyone reencoding the message for any reason, 
even if they have changed nothing, might find the payload changes due to the 
setAddress side effects?


---

Reply via email to