-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DRAFT] check for nesting limits #3805
Conversation
public void testTreeWithArray() throws Exception | ||
{ | ||
final String doc = _nestedDoc(TOO_DEEP_NESTING, "[ ", "] "); | ||
JsonNode tree = unconstrainedMapper.readTree(doc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cowtowncoder I was trying to add a test that checks the new logic in TreeTraversingParser (to validate nesting depth constraint). I found that 'readTree' validates already because it uses the jackson-core ReaderbasedJsonParser. I had to change the mapper to loosen the default nesting constraint to get this line to pass. This got me to thinking. Is there much point in having TreeTraversingParser also validate the nesting depth?
A similar question could be asked for the TokenBuffer Parser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pjfanning Good question: I don't think they should (have to) validate nesting depth: only things that read actual input should. It seems like pure overhead, plus in some cases it may be unclear which constraints to use.
Same sort of goes for String length checks too I suppose.
One counter-case might be numeric length: if buffering String tokens but requesting number, number-length checks should be applied.
@pjfanning Let's just remove validation checks from I'll try merging some of changes from here to unblock some tests. |
Ok, merged unit test fixes directly. Caused some conflict, will try to resolve. EDIT: no more conflicts. |
useful bits already merged - #3810 contains the test |
StreamReadConstraints.maxNestingDepth()
to constraint max nesting depth (default: 1000) jackson-core#943 to compile.