[GitHub] activemq-artemis issue #2423: ARTEMIS-2168 Fix "populate-validated-user" fea...

[michaelandrepearce]

Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2423
  
But then that would mean it would not show on JMS properties correctly . 
Which it does today


---

[GitHub] activemq-artemis issue #2423: ARTEMIS-2168 Fix "populate-validated-user" fea...

[clebertsuconic]

Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/2423
  
Any modification has to be on extra properties. 


---

[GitHub] activemq-artemis issue #2423: ARTEMIS-2168 Fix "populate-validated-user" fea...

[michaelandrepearce]

Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2423
  
Id disagree thats a very importand broker feature.


---

[GitHub] activemq-artemis issue #2423: ARTEMIS-2168 Fix "populate-validated-user" fea...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2423
  
@michaelandrepearce I agree, that expiry bit qualifies as a bug and should 
also be fixed to disallow modification


---

[GitHub] activemq-artemis issue #2423: ARTEMIS-2168 Fix "populate-validated-user" fea...

[clebertsuconic]

Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/2423
  
Amqp headers cannot be changed per spec. I am kind of confused on what’s 
going on 

I’m a bit lost on a week or vacation.  


Can I have a TL;dr


---

[GitHub] activemq-artemis issue #2423: ARTEMIS-2168 Fix "populate-validated-user" fea...

[michaelandrepearce]

Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2423
  
@tabish121 id disagree here, the message is already modified when broker 
features like enforced expiry-delay are being used.


```  public AMQPMessage setExpiration(long expiration) {
  if (properties != null) {
 if (expiration <= 0) {
properties.setAbsoluteExpiryTime(null);
 } else {
properties.setAbsoluteExpiryTime(new Date(expiration));
 }
  } else if (expiration > 0) {
 properties = new Properties();
 properties.setAbsoluteExpiryTime(new Date(expiration));
  }

  // We are overriding expiration with an Absolute expiration time so 
any
  // previous Header based TTL also needs to be removed.
  if (header != null) {
 header.setTtl(null);
  }

  this.expiration = Math.max(0, expiration);

  return this;
   }```





---

[GitHub] activemq-artemis issue #2423: ARTEMIS-2168 Fix "populate-validated-user" fea...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2423
  
I'd say none of them are really the right way to go.  I'd suggest your 
organization create a custom broker plugin that does whatever protocol 
violating things are acceptable to you and your team and not allow for the 
inevitable issues that arise from violating the specification from being 
dropped directly into the broker.  You could also (if you really care about the 
message user id no matching) reject incoming messages there are close the 
connection of the offending client.  


---

[GitHub] activemq-artemis issue #2423: ARTEMIS-2168 Fix "populate-validated-user" fea...

[michaelandrepearce]

Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2423
  
So on the options/ideas i put on the other pr which do you think is the 
best.


---

[GitHub] activemq-artemis issue #2423: ARTEMIS-2168 Fix "populate-validated-user" fea...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2423
  
-1

This is actually worse that before because now you've not only violated the 
AMQP spec in regards to changing the User ID portion of the properties section 
but also opened it up to even more spec abuse and probably lost fidelity on the 
message such that what was sent will not be faithfully reproduced to an AMQP 
consumer reading the message. 


---

[GitHub] activemq-artemis issue #2423: ARTEMIS-2168 Fix "populate-validated-user" fea...

[michaelandrepearce]

Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2423
  
im going to close this, and re-base and re-open once the fixes for group 
seq are merged, so its a cleaner PR.


---

[GitHub] activemq-artemis issue #2423: ARTEMIS-2168 Fix "populate-validated-user" fea...

[michaelandrepearce]

Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2423
  
@tabish121 one of the alternative approaches, where when this feature is 
enabled, we simply convert to CORE, this then avoids making any changes to 
AMQPMessage (keeping untouched)


---