Stax maxAttributeSize limit is only vaguely respected

[istudens]

Hello,
System property org.apache.cxf.stax.maxAttributeSize only vaguely limits attribute values. If it is set to 5000 it can send up to 8295 characters in an attribute value without denying the request.

The culprit is that the limit is checked against the size of the buffer before the last buffer expansion. After 2459 characters the buffer is grown to 3687. After 5531 characters the limit is checked against 3687 instead of 5531 and not until 8296 characters is the limit checked against the previous buffer size 5531 which is larger than 5000.

I am going to file a PR with fix + testcase immediately.

[cowtowncoder]

This is by design tho. Amount of overhead for checking for each and every character makes it less than desirable to do that -- I don't want parsing to be measurable slower. So, while I'll consider this I will most likely consider this "working as expected" -- limit check is not meant to be precise.

[istudens]

There are many comparisons done for each character and this is just one extra simple if (currentValue > maxValue) where both values are taken from local variables (one is a final int, one is int).
Does this lead to any measurable overhead? If yes, feel free to close this issue and its PR.

[e-ts]

There should not be any need to check for the limit for every character. The bug is that the limit is checked against com.ctc.wstx.util.TextBuilder#mBufferLen which is not updated because com.ctc.wstx.util.TextBuilder#append is bypassed. mBufferLen is only updated by bufferFull which sets it to the buffers length before the buffer is expanded.

The limit will not be checked against the next buffer size, nor the current buffer size but to what the buffer size was prior to the last expansion.

In one trace I did on 4.2.0, the following checks were made:

On the 97th character, the limit is checked against 0.
On the 145th character, the limit is checked against 96.
On the 217th character, the limit is checked against 144.
On the 325th character, the limit is checked against 216.
On the 487th character, the limit is checked against 324.
On the 730th character, the limit is checked against 486.
On the 1094th character, the limit is checked against 729.
On the 1640th character, the limit is checked against 1093.
On the 2459th character, the limit is checked against 1639.
On the 3688th character, the limit is checked against 2458.
On the 5531th character, the limit is checked against 3687.
On the 8296th character, the limit is checked against 5530.

[cowtowncoder]

@istudens unfortunately I do not have solid jmh (etc) based benchmarks for stax handling. But I would put that around and accept evidence of non-measurable impact (with attribute heavy xml) to justify addition of check.

@e-ts Interesting. That does sound like an easy enough bug to fix.

[cowtowncoder]

@istudens Thank you for reporting this and contributing the patch! I ended up implementing it slightly differently, party to remove once per loop check, but also because testing reminded me of one implementation peculiarity: TextBuilder instance is actually used by all attribute values of given element, so outPtr can not be directly checked.

Fix will go in 6.0.3, whenever I release that.