Skip to content
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.

Sign up for GitHub

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] validate deep nesting #394

Closed
wants to merge 3 commits into from
Closed

[DRAFT] validate deep nesting #394

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d3cba10
validate deep nesting
pjfanning Mar 5, 2023
cb02e27
Merge branch '2.15' into deep-nesting
pjfanning Mar 7, 2023
75bab02
refactor
pjfanning Mar 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,8 @@ public Object getInputSource() {
* We need to override this method to support coercion from basic
* String value into array, in cases where schema does not
* specify actual type.
*
* @throws UncheckedIOException if the nesting depth is too large, see {@link StreamReadConstraints.Builder#maxNestingDepth(int)}
*/
@Override
public boolean isExpectedStartArrayToken() {
Expand Down Expand Up @@ -926,13 +928,15 @@ protected JsonToken _handleStartDoc() throws IOException
// but even empty sequence must still be wrapped in logical array
if (wrapAsArray) {
_parsingContext = _reader.childArrayContext(_parsingContext);
_streamReadConstraints.validateNestingDepth(_parsingContext.getNestingDepth());
return JsonToken.START_ARRAY;
}
return null;
}

if (wrapAsArray) {
_parsingContext = _reader.childArrayContext(_parsingContext);
_streamReadConstraints.validateNestingDepth(_parsingContext.getNestingDepth());
_state = STATE_RECORD_START;
return JsonToken.START_ARRAY;
}
Expand All @@ -946,10 +950,12 @@ protected JsonToken _handleRecordStart() throws IOException
if (_columnCount == 0) { // no schema; exposed as an array
_state = STATE_UNNAMED_VALUE;
_parsingContext = _reader.childArrayContext(_parsingContext);
_streamReadConstraints.validateNestingDepth(_parsingContext.getNestingDepth());
return JsonToken.START_ARRAY;
}
// otherwise, exposed as an Object
_parsingContext = _reader.childObjectContext(_parsingContext);
_streamReadConstraints.validateNestingDepth(_parsingContext.getNestingDepth());
_state = STATE_NEXT_ENTRY;
return JsonToken.START_OBJECT;
}
Expand Down Expand Up @@ -1403,11 +1409,16 @@ public ByteArrayBuilder _getByteArrayBuilder()
return _byteArrayBuilder;
}

protected void _startArray(CsvSchema.Column column)
protected void _startArray(CsvSchema.Column column) throws UncheckedIOException
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for this not to expose IOException?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed just now - I'm not sure if this PR is really useful though, I can't really see how you can nest CSV or JavaProperties - I'm going to concentrate on use cases like Smile and CBOR first - YAML is covered by SnakeYAML LoaderOPtions (so like jackson-dataformat-xml, the argument is if we need 2 ways to police the depth, SnakeYAML has a default depth limit of 50).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Yeah, good point. In theory in future we might create nested Objects etc. But even then... seems unlikely & can cross that bridge if we get there.

For Properties it's bit different since there is the path splitting so one can actually create keys to create whatever depth.

And YAML is interesting as limits are needed but... provided natively. So can/should postpone handling changes there too I think.

TOML, once again is... its own thing.

So I think Properties are one to consider, yet less important than Smile, CBOR. Can't do it for Ion I suspect.
Protobuf doesn't have constructs for infinite recursion I don't think?

{
_currToken = JsonToken.START_ARRAY;
_parsingContext = _parsingContext.createChildArrayContext(_reader.getCurrentRow(),
_reader.getCurrentColumn());
try {
_streamReadConstraints.validateNestingDepth(_parsingContext.getNestingDepth());
} catch (IOException e) {
throw new UncheckedIOException(e);
}
_state = STATE_IN_ARRAY;
_arrayValueStart = 0;
_arrayValue = _currentValue;
Expand Down
1 change: 1 addition & 0 deletions properties/src/main/java/com/fasterxml/jackson/dataformat/javaprop/io/JPropReadContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public JPropReadContext(int contextType, JPropReadContext p, JPropNode node)
_index = -1;
_parent = p;
_branchText = node.getValue();
_nestingDepth = p == null ? 0 : p._nestingDepth + 1;
}

public static JPropReadContext create(JPropNode root) {
Expand Down
2 changes: 2 additions & 0 deletions yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/YAMLParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ public JsonToken nextToken() throws IOException
MappingStartEvent map = (MappingStartEvent) evt;
_currentAnchor = map.getAnchor();
_parsingContext = _parsingContext.createChildObjectContext(m.getLine(), m.getColumn());
_streamReadConstraints.validateNestingDepth(_parsingContext.getNestingDepth());
return (_currToken = JsonToken.START_OBJECT);
}
if (evt.is(Event.ID.MappingEnd)) { // actually error; can not have map-end here
Expand All @@ -512,6 +513,7 @@ public JsonToken nextToken() throws IOException
Mark m = evt.getStartMark();
_currentAnchor = ((NodeEvent)evt).getAnchor();
_parsingContext = _parsingContext.createChildArrayContext(m.getLine(), m.getColumn());
_streamReadConstraints.validateNestingDepth(_parsingContext.getNestingDepth());
return (_currToken = JsonToken.START_ARRAY);
}
if (evt.is(Event.ID.SequenceEnd)) {
Expand Down