[GitHub] activemq-artemis issue #2232: ARTEMIS-1482 Enhance test to ensure len check ...

2018-08-10 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2232
  
So we have no idea of the cause, it is a consumer so we think its a 
producer where producee maybe on older version. It happend 4 times since monday 
our app team said 


---


[GitHub] activemq-artemis issue #2232: ARTEMIS-1482 Enhance test to ensure len check ...

2018-08-10 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/2232
  
It’s already on 2.6.x


---


[GitHub] activemq-artemis issue #2232: ARTEMIS-1482 Enhance test to ensure len check ...

2018-08-10 Thread mtaylor
Github user mtaylor commented on the issue:

https://github.com/apache/activemq-artemis/pull/2232
  
@michaelandrepearce How did they do this? It's really only legitimate if 
the SimpleString is encoded incorrectly?  i.e. Set the first 4 bytes as 
something like Integer.MAX.  Other checks such as large message size should 
catch this earlier in the chain.


---


[GitHub] activemq-artemis issue #2232: ARTEMIS-1482 Enhance test to ensure len check ...

2018-08-10 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2232
  
@clebertsuconic can this be ported to the 2.6.x branch?


---


[GitHub] activemq-artemis issue #2232: ARTEMIS-1482 Enhance test to ensure len check ...

2018-08-10 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2232
  
@mtaylor i giggle, its like there are forces out there that just know, 
literally came into work today, and one of our app teams just experienced this 
issue this morning... least i know whats the issue :)


---


[GitHub] activemq-artemis issue #2232: ARTEMIS-1482 Enhance test to ensure len check ...

2018-08-09 Thread mtaylor
Github user mtaylor commented on the issue:

https://github.com/apache/activemq-artemis/pull/2232
  
@michaelandrepearce grand.  Yes that is what I was concerned about.  
Merged. Thanks


---


[GitHub] activemq-artemis issue #2232: ARTEMIS-1482 Enhance test to ensure len check ...

2018-08-09 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2232
  
@mtaylor i was editing my comment sorry with the output when your fix would 
be regressed:

The other tests will run when run via maven if thats what you're worried 
about? Here is example output of the test suite run, when i comment out your 
fix (so it OOM's)

Results :

Tests in error:
SimpleStringTest.testOutOfBoundsThrownOnMalformedString:36 » OutOfMemory 
Reque...

Tests run: 112, Failures: 0, Errors: 1, Skipped: 0


I think its quite clear and maven nicely outputs


---


[GitHub] activemq-artemis issue #2232: ARTEMIS-1482 Enhance test to ensure len check ...

2018-08-09 Thread mtaylor
Github user mtaylor commented on the issue:

https://github.com/apache/activemq-artemis/pull/2232
  
OK.  I was trying to avoid that, hence why I choose 100 bytes the first 
time round.  But I think you're right, it will only crash when this issue is 
reintroduced, in that case we catch and fix it.  


---


[GitHub] activemq-artemis issue #2232: ARTEMIS-1482 Enhance test to ensure len check ...

2018-08-09 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2232
  
@mtaylor yes it will crash, it OOM's, but then at least you know the build 
isnt good, and would be picked up and avoid it being regressed, which is the 
point of test suite.


---


[GitHub] activemq-artemis issue #2232: ARTEMIS-1482 Enhance test to ensure len check ...

2018-08-09 Thread mtaylor
Github user mtaylor commented on the issue:

https://github.com/apache/activemq-artemis/pull/2232
  
@michaelandrepearce Are you sure this doesn't cause the test suite to 
crash?  


---


[GitHub] activemq-artemis issue #2232: ARTEMIS-1482 Enhance test to ensure len check ...

2018-08-09 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2232
  
@mtaylor here is updated test that will catch (by the test going OOM) if 
the byte[] is initialized, and the validity check isnt done first. I checked 
and unit test fails without your change, but passes with it.


---