Introduce JSON pretty print #13357
Introduce JSON pretty print #13357
Conversation
Signed-off-by: Oleksandr Krutko <alexander.krutko@gmail.com>
| return toString(o, ident, 0); | ||
| } | ||
|
|
||
| private String toString(final Object o, String ident, int nest) |
There was a problem hiding this comment.
This probably needs loop protection. I'm sure it is not legal json for a map/array to contain itself, but somebody will create one and pass it. Throwing early is better than stack overflow, or even just print a name instead of the contents in a loop.
There was a problem hiding this comment.
Do we actually need importing additional package for such simple operation?
There was a problem hiding this comment.
Do we actually need importing additional package for such simple operation?
I think this reply is for the StringUtil comment.
No new package needed, you have access to it already in jetty-util-AJAX.
There was a problem hiding this comment.
This probably needs loop protection. I'm sure it is not legal json for a map/array to contain itself, but somebody will create one and pass it. Throwing early is better than stack overflow, or even just print a name instead of the contents in a loop.
There's no loop protection in the existing String JSON.toJSON(Object obj), why would we need it here?
This PR is just about pretty printing, indents and whatnot.
There was a problem hiding this comment.
Do we actually need importing additional package for such simple operation?
I think this reply is for the StringUtil comment.
No new package needed, you have access to it already in jetty-util-AJAX.
I will check. Thank you!
There was a problem hiding this comment.
I still don't understand why this is suddenly a requirement for jetty-util-ajax.
Just so you know, infinite recursion, stackoverflows, etc are not guarded against in jackson, org.json, or gson. (probably the 3 most used json libs in the java world).
The reason is that it causes too much memory use.
The solution for infinite recursion during serialization in libs like spring, hibernate, and jackson is to annotate specific entries to solve the recursion, not to build in object instance tracking into the serialization.
https://www.baeldung.com/jackson-bidirectional-relationships-and-infinite-recursion
Anyway, a visited Set is probably the wrong approach.
After all the same object can be in the Map in different locations safely.
A Stack is better, as it's only about what's references are above the current state that you care about, but that would need to be passed around to be sane.
There was a problem hiding this comment.
It's not suddenly a requirement, just a thought whilst we are touching this class.
If the pretty printer to be used in generating real json as part of an app? Then let's not waste the memory.
There was a problem hiding this comment.
Jackson now has protection FasterXML/jackson-core#943 against this.
There was a problem hiding this comment.
Jackson now has protection FasterXML/jackson-core#943 against this.
That solution doesn't have duplicate object instance tracking, it just sets a maximum depth of nesting content.
That's not a bad idea actually, instead of duplicate object tracking via a collection, just set a maximum depth configurable instead. Same end result, no more StackOverFlowExceptions.
There was a problem hiding this comment.
Hello,
I checked JSON spec, there is no limitation of nested objects, so it is dependent on platform and available resources.
| return array.toString(); | ||
| } | ||
|
|
||
| private String putQuote(Object o) |
There was a problem hiding this comment.
StringUtil has methods that do this... check if they are usable or not
There was a problem hiding this comment.
Jetty's StringUtil doesn't have such a method.
Why not just implement this using an Appendable and reusing the existing JSON.quotedEscape(Appendable buffer, String input) for this purpose instead?
There was a problem hiding this comment.
Ah yes, I was thinking about org.eclipse.jetty.http.QuotedCSVParser#quoteIfNeeded
That's for sure! |
This is draft PR for issue #13318.