I had wondered the same thing. I'd be +1 on it. Jon
On Fri, Feb 4, 2022 at 11:17 AM Colm O hEigeartaigh <cohei...@apache.org> wrote: > Hey JB/all, > > I'm wondering if we can also take the opportunity to consider > switching from Apache Log4J 1.x to Reload4J for AMQ 5.15.x/5.16.x. > https://reload4j.qos.ch/ is a fork of Log4J 1.x with the CVE fixes > applied. Other Apache projects such as Apache Avro have switched over > to use it, similarly pax logging > ( > https://github.com/ops4j/org.ops4j.pax.logging/commit/a21b1b41fb107164522e587643e8daff2724ce86 > ). > > There will be many users of AMQ who will not want or be able to > migrate to AMQ 5.17 and hence Log4J 2. It's difficult to convey to > users that the CVEs don't apply to the default configuration, as many > users just see critical CVEs via their own scanning tools and insist > on upgrades. In addition, Log4J 1.x is not maintained since 2015 and > if a new CVE were to come out that affects the default configuration, > it would be problematic. > > If you approve I can create a PR. > > Colm. > > On Fri, Feb 4, 2022 at 7:51 AM Jean-Baptiste Onofré <j...@nanthrax.net> > wrote: > > > > Hi Clebert, > > > > Good idea ! > > > > If Chris agree with this plan, I'm fine with it. > > > > Let me work on the tests then (I will create PRs). > > > > Regards > > JB > > > > On 04/02/2022 00:55, Clebert Suconic wrote: > > > I would say.. write the tests... if they pass no need to cancel the > > > release as they will not affect runtime. only cancel the release if > > > you actually find an issue. > > > > > > On Thu, Feb 3, 2022 at 8:15 AM Jean-Baptiste Onofré <j...@nanthrax.net> > wrote: > > >> > > >> Hi Chris, > > >> > > >> OK, fair enough for me. I will cancel the vote and work with Matt to > add > > >> the tests. > > >> > > >> Thanks, > > >> Regards > > >> JB > > >> > > >> On 03/02/2022 13:57, Christopher Shannon wrote: > > >>> JB, > > >>> > > >>> I would rather just add tests first and re-run the vote as it > sounded like > > >>> it wouldn't take too long to add a few tests. I wanted to verify > some of > > >>> those fixes work and make sense and it would be a lot easier not > having to > > >>> try and figure out how to set up a manual test that will most likely > be > > >>> inconsistent from what others used depending on the config settings > and > > >>> testing scenarios. Furthermore, it's very common to think an issue > is fixed > > >>> from manual testing and then run through unit tests and then find an > issue > > >>> (missing edge case, etc) > > >>> > > >>> Matt, for AMQ-8397 you are right that it would be tricky to test the > > >>> behavior itself but I don't think that's necessary. Since the fix is > really > > >>> just about adding a new flag to disable sending to the DLQ I would > say > > >>> instead of testing the full behavior you could at least just write a > quick > > >>> test that would verify the default is what you want (off by default) > and > > >>> that changing the policy actually properly sets it on the > destination as > > >>> intended. > > >>> > > >>> On Thu, Feb 3, 2022 at 3:44 AM Jean-Baptiste Onofré <j...@nanthrax.net> > wrote: > > >>> > > >>>> Hi, > > >>>> > > >>>> As some features have been tested manually (you did a pass, I did a > > >>>> quick pass on some as well), it's good enough for the release. > > >>>> Agree with Chris that we should add test to "secure" the code > > >>>> coverage/regression, but it should be done. > > >>>> Either way is fine for me: if we consider it's blocker, I can > cancel the > > >>>> current vote, include new tests and cut a new release. > > >>>> Else, I would say, let's move forward with the current vote and > include > > >>>> tests asap (decoupled from this vote/release). > > >>>> > > >>>> Regards > > >>>> JB > > >>>> > > >>>> On 02/02/2022 22:21, Matt Pavlovich wrote: > > >>>>> Note— I did test these changes manually. > > >>>>> > > >>>>>> On Feb 2, 2022, at 2:55 PM, Matt Pavlovich <mattr...@gmail.com> > wrote: > > >>>>>> > > >>>>>> Hey Christopher- > > >>>>>> > > >>>>>> I see your point on the wording of AMQ-8443. It is an edge case > with > > >>>> ConnectionControl, not failover in general. I’ll clear up the > description. > > >>>> I’m happy to add tests for a couple of them; however, AMQ-8397 is > tricky. > > >>>>>> > > >>>>>> What do you suggest for a reliable test scenario for AMQ-8397? > > >>>>>> > > >>>>>> AMQ-8053 - This is a very minor change, I’ll add a test > > >>>>>> AMQ-8443 - Edge case, I’ll add a test > > >>>>>> AMQ-8397 - Note— this is ‘disabled by default in 5.16.x’. This is > a > > >>>> reporting change on an edge case from being “move message to DLQ” > to log > > >>>> message and increment a counter. This is really difficult to > consistently > > >>>> create a unit test to produce the scenario on varying hardware. > This is a > > >>>> heavy contention scenario and I was not able to find any tests that > produce > > >>>> the current behavior. I’d be happy to adapt verification if you can > provide > > >>>> a stable base scenario (or point to an existing unit test that I > may have > > >>>> missed). > > >>>>>> > > >>>>>> Thanks, > > >>>>>> Matt Pavlovich > > >>>>>> > > >>>>>>> On Feb 2, 2022, at 12:56 PM, Christopher Shannon < > > >>>> christopher.l.shan...@gmail.com <mailto: > christopher.l.shan...@gmail.com>> > > >>>> wrote: > > >>>>>>> > > >>>>>>> Unfortunately I am -1. I just checked the release and several > Jiras > > >>>> don't > > >>>>>>> have any unit tests for verification. > > >>>>>>> > > >>>>>>> https://issues.apache.org/jira/browse/AMQ-8053 < > > >>>> https://issues.apache.org/jira/browse/AMQ-8053> > > >>>>>>> https://issues.apache.org/jira/browse/AMQ-8372 > > >>>>>>> https://issues.apache.org/jira/browse/AMQ-8397 > > >>>>>>> https://issues.apache.org/jira/browse/AMQ-8443 > > >>>>>>> > > >>>>>>> There might be more, I quit checking after the first several. > All of > > >>>> these > > >>>>>>> fixes and improvements need tests. It's important to add some > tests to > > >>>>>>> verify the actual bug or issue and to make sure it doesn't get > broken > > >>>> in > > >>>>>>> the future. I am not comfortable releasing this without having > tests > > >>>> for > > >>>>>>> these issues. The same of course goes into anything with 5.17.0. > > >>>> Obviously > > >>>>>>> we don't need tests for trivial stuff (dependency upgrades, null > > >>>> pointer > > >>>>>>> checks, etc) but other stuff should be tested. > > >>>>>>> > > >>>>>>> The biggest issue I have is AMQ-8443 which claims this is a fix > for the > > >>>>>>> "failover transport reconnect not working" but no demonstration > of the > > >>>>>>> issue or how the fix works in a unit test. > > >>>>>>> > > >>>>>>> On Wed, Feb 2, 2022 at 3:29 AM Francois Papon < > > >>>> francois.pa...@openobject.fr> > > >>>>>>> wrote: > > >>>>>>> > > >>>>>>>> +1 (non-binding) > > >>>>>>>> > > >>>>>>>> Thanks! > > >>>>>>>> > > >>>>>>>> regards, > > >>>>>>>> > > >>>>>>>> François > > >>>>>>>> > > >>>>>>>> On 01/02/2022 21:41, Jean-Baptiste Onofre wrote: > > >>>>>>>>> Hi everyone, > > >>>>>>>>> > > >>>>>>>>> I submit Apache ActiveMQ 5.16.4 release to your vote. > > >>>>>>>>> > > >>>>>>>>> This release includes important fixes and updates on the 5.16.x > > >>>> series, > > >>>>>>>> especially: > > >>>>>>>>> - fix couple of warnings/issues on JDK16+ > > >>>>>>>>> - fix stack trace display on transports > > >>>>>>>>> - better secure on WebConsole > > >>>>>>>>> - several dependencies updates > > >>>>>>>>> - and much more! > > >>>>>>>>> > > >>>>>>>>> Please take a look on the Release Notes for details: > > >>>>>>>>> > > >>>>>>>> > > >>>> > https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12350483 > > >>>>>>>>> > > >>>>>>>>> Maven staging repository is: > > >>>>>>>>> > > >>>>>>>> > > >>>> > https://repository.apache.org/content/repositories/orgapacheactivemq-1244/ > > >>>>>>>>> > > >>>>>>>>> Dist staging repository is: > > >>>>>>>>> > https://dist.apache.org/repos/dist/dev/activemq/activemq/5.16.4/ > > >>>>>>>>> > > >>>>>>>>> Git tag: > > >>>>>>>>> activemq-5.16.4 > > >>>>>>>>> > > >>>>>>>>> Please vote to approve this release: > > >>>>>>>>> > > >>>>>>>>> [ ] +1 Approve the release > > >>>>>>>>> [ ] -1 Don't approve the release (please provide specific > comments) > > >>>>>>>>> > > >>>>>>>>> This vote will be open for at least 72 hours. > > >>>>>>>>> > > >>>>>>>>> Thanks ! > > >>>>>>>>> Regards > > >>>>>>>>> JB > > >>>>>>>> > > >>>>>> > > >>>>> > > >>>>> > > >>>> > > >>> > > > > > > > > > >
