OF-2628: Avoid polynomial regular expression used on uncontrolled data

[guusdk]

Prevent possibility of regular expression execution depending on user-provided values taking exceptionally long / many resources to complete

This commit replaces the regular expression with a solution that doesn't use regular expressions. Arguably, the complexity of this code does not change much.

[guusdk]

Is this a solution that covers all edge cases, and is roughly not much worse in performance as compared to the original?

[Fishbowler]

I wonder if we can test efficacy AND performance with a unit test pre- and post-change

[GregDThomas]

LGTM, and is better in that it allows hex char references.

I can't offer any opinion on performance, other than perhaps suggest it probably doesn't matter too much unless it's awful, which I doubt?

[Fishbowler]

Added some unit tests and ran them on main and on this branch a bunch of times via mvn -Dtest=XMLLightweightParserTest#testHasIllegalCharacterReferences* test - sometimes one is faster, sometimes the other. There's no perceivable difference.

[Fishbowler]

I won't merge unit Guus gives my unit tests a look. They're really very brief...

[GregDThomas]

Might want to replace @test with @RepeatedTest(10_000) - or some other number, but they look pretty quick to run - to get a bigger sample for timing purposes.

[guusdk]

The original should also have allowed for hex-based character references - but the fact that that wasn't clear is probably a good indication that this change isn't the worst, from a readability perspective.

As for the test:

• /me points at org.jivesoftware.openfire.nio.XmlNumericCharacterReferenceTest
• Although it's good to get some anecdotal evidence, I don't think we should use unit testing for automated load comparison, to be honest.

[Fishbowler]

Thanks Guus. I totally didn't spot those, and they're much better functional coverage. Reverted.

I agree with not using unit tests for automated load comparison in the general case. It requires constrained and understood code paths and an environment where the performance variables are somewhat understood. Understanding the results are also important (e.g. one run with a 10% isn't interesting, but a consistent 10% execution time change very much is).