Proposed enhancement to expression max length functionality (control changes, visibility, unit tests)

[JCgH4164838Gh792C124B5]

Proposed enhancement to Y. Zamani's change in PR#82.

• Apply methods to control changes to the expressionMaxLength.
• Appy minimal controls to ensure consistency and visibility of changes to
  expressionMaxLength.
• Provide unit tests for the behaviour of expressionMaxLength and the new
  methods.

[JCgH4164838Gh792C124B5]

Hello OGNL Team.

Although there may be some overhead related to the consistency changes in this PR, since the maximum expression length is intended for security the extra overhead is probably justified (to ensure consistent reads/writes for its state).

Since most OGNL expressions are parsed, compiled and cached it should have minimal overhead for most use cases. If it does turn out to be too expensive it could always be made non-volatile again ...

The added methods for freezing/thawing changes to the maximum length are only intended to prevent accidental changes to that value, while still allowing flexibility for various use cases (thought about making it only possible to freeze it and never thaw it again but that might be too inflexible).

The additional unit tests should be of use even if some of the ideas in this PR are rejected.

Thanks in advance for your consideration.

[lukaszlenart]

Hm... now when I'm looking at these changes I'm not really sure if they will improve security, it will just complicate things a bit for hackers. As they aren't invasive we can try to test them first with some older Struts attacks to see how will it work.

LGTM 👍 (if no objections)

[JCgH4164838Gh792C124B5]

I tried to think of a way to provide state-consistency across threads (for the maximum expression length) without resorting to synchronization and volatile, but due to the static context of the parseExpression() method there don't seem to be any practical alternatives ... unlike if it was instance-based.

Having a maximum length limit for expressions available might only be of limited impact security-wise, but it could also have value as a "style" consideration (for applications using OGNL). Once expressions start exceeding the "recommended max. length" and failing it could promote review of expression length for alternatives.

[lukaszlenart]

Going away of the static context is my plan for OGNL 4 - this will also allow you to create multiple OGNL runtimes at the same time...

[lukaszlenart]

I'm going to release 3.1.26 right away :)