Protect JSONML from stack overflow exceptions caused by recursion

[TamasPergerDWP]

The fix follows the method used in fixing the same issue for XML in the previous PR:
Limit the XML nesting depth for CVE-2022-45688 #720

Additionally, fixing a minor issue with XMLParserConfiguration.clone() and amending some Javadoc.

Resolves #722

[stleary]

Is there a reason for not re-using XMLParserConfiguration?
Either way, please don't expose any new APIs for JSONML besides the max nesting depth.

[TamasPergerDWP]

The main reason: not all options of XMLParserConfiguration is supported with JSONML parsing, so it seemed cleaner to create a new config class.

[TamasPergerDWP]

The new APIs for JSONML:

• public static JSONObject toJSONObject(String string, XMLtoJSONMLParserConfiguration config)
• public static JSONObject toJSONObject(XMLTokener x, XMLtoJSONMLParserConfiguration config)

These are both necessary to support max nesting depth and the keepStrings option in one function.
The alternative would be to add create 4 new toJSONObject functions that include the int maxNestingDepth parameter, on top of the existing 4 such functions - the potential parameter combinations doubling with a new optional parameter.

Is this acceptable?

[TamasPergerDWP]

Just noticed that I did not add the max depth limit override option for the toJSONArray calls - await a fix soon.

Update - fixed now, a new commit was pushed.

[TamasPergerDWP]

@stleary I have a fix for the JSONArray and JSONObject as well - by adding the depth limit to the JSONTokener class. (See my internal PR: TamasPergerDWP#1)
Three questions:

1. Shall I add the fix to the current PR, or open a new one?
2. JSONArray and JSONObject are used internally in the CDL, Cookie and CookieList classes. I introduced the maxDepthLimit optional parameter into those classes, too. Is that OK? Do those classes need additional tests for the maxDepthLimit behaviour?
3. The JSONTokener class is the parent of XMLTokener - after introducing the max nesting depth feature in JSONTokener, we may simplify the solution that is in place for the XML parsing (unifying both approaches). Is this something we should look into?

[stleary]

Let's work through the JSONML changes before starting JSONObject and JSONArray.
Please rename XMLtoJSONMLParserConfiguration to something simpler, like JSONMLParserConfiguration.
The new API methods for JSONML look OK. Code also looks OK, but still needs an in-depth review, working on it.

[TamasPergerDWP]

Let's work through the JSONML changes before starting JSONObject and JSONArray.

Fair enough, I will leave the other changes for another day.

Please rename XMLtoJSONMLParserConfiguration to something simpler, like JSONMLParserConfiguration.

Done, ready for review.

[stleary]

@TamasPergerDWP The code looks good, just one comment

[TamasPergerDWP]

@TamasPergerDWP The code looks good, just one comment

@stleary Code amended as requested

[stleary]

What problem does this code solve?
JSONML should have the same StackOverflow protection as XML.

Risks
Low

Changes to the API?
Some new API methods were added for those who don't want to use the default nesting depth.

Will this require a new release?
Yes

Should the documentation be updated?
No

Does it break the unit tests?
No

Was any code refactored in this commit?
Future refactoring will unite the parserconfig classes in a class hierarchy of some sort.

Review status
APPROVED

[stleary]

Starting 3 day comment window.