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

Jump to bottom

Add StreamReadConstraints.maxNestingDepth() to constraint max nesting depth (default: 1000) #943

Merged
merged 1 commit into from
Mar 6, 2023
Merged

Add StreamReadConstraints.maxNestingDepth() to constraint max nesting depth (default: 1000) #943

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/main/java/com/fasterxml/jackson/core/JsonStreamContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ public abstract class JsonStreamContext
*/
protected int _index;

/**
* The nesting depth is a count of objects and arrays that have not
* been closed, `{` and `[` respectively.
*/
protected int _nestingDepth;

/*
/**********************************************************
/* Life-cycle
Expand Down Expand Up @@ -118,6 +124,14 @@ protected JsonStreamContext(int type, int index) {
*/
public final boolean inObject() { return _type == TYPE_OBJECT; }

/**
* The nesting depth is a count of objects and arrays that have not
* been closed, `{` and `[` respectively.
*/
public final int getNestingDepth() {
return _nestingDepth;
}

/**
* @return Type description String
*
Expand Down
70 changes: 64 additions & 6 deletions src/main/java/com/fasterxml/jackson/core/StreamReadConstraints.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ public class StreamReadConstraints
{
private static final long serialVersionUID = 1L;

/**
* Default setting for maximum depth: see {@link Builder#maxNestingDepth(int)} for details.
*/
public static final int DEFAULT_MAX_DEPTH = 1000;

/**
* Default setting for maximum number length: see {@link Builder#maxNumberLength(int)} for details.
*/
Expand All @@ -24,16 +29,37 @@ public class StreamReadConstraints
*/
public static final int DEFAULT_MAX_STRING_LEN = 1_000_000;

protected final int _maxNestingDepth;
protected final int _maxNumLen;
protected final int _maxStringLen;

private static final StreamReadConstraints DEFAULT =
new StreamReadConstraints(DEFAULT_MAX_NUM_LEN, DEFAULT_MAX_STRING_LEN);
new StreamReadConstraints(DEFAULT_MAX_DEPTH, DEFAULT_MAX_NUM_LEN, DEFAULT_MAX_STRING_LEN);

public static final class Builder {
private int maxNestingDepth;
private int maxNumLen;
private int maxStringLen;

/**
* Sets the maximum nesting depth. The depth is a count of objects and arrays that have not
* been closed, `{` and `[` respectively.
*
* @param maxNestingDepth the maximum depth
*
* @return this builder
* @throws IllegalArgumentException if the maxNestingDepth is set to a negative value
*
* @since 2.15
*/
public Builder maxNestingDepth(final int maxNestingDepth) {
if (maxNestingDepth < 0) {
throw new IllegalArgumentException("Cannot set maxNestingDepth to a negative value");
}
this.maxNestingDepth = maxNestingDepth;
return this;
}

/**
* Sets the maximum number length (in chars or bytes, depending on input context).
* The default is 1000.
Expand Down Expand Up @@ -79,21 +105,23 @@ public Builder maxStringLength(final int maxStringLen) {
}

Builder() {
this(DEFAULT_MAX_NUM_LEN, DEFAULT_MAX_STRING_LEN);
this(DEFAULT_MAX_DEPTH, DEFAULT_MAX_NUM_LEN, DEFAULT_MAX_STRING_LEN);
}

Builder(final int maxNumLen, final int maxStringLen) {
Builder(final int maxNestingDepth, final int maxNumLen, final int maxStringLen) {
this.maxNestingDepth = maxNestingDepth;
this.maxNumLen = maxNumLen;
this.maxStringLen = maxStringLen;
}

Builder(StreamReadConstraints src) {
maxNestingDepth = src._maxNestingDepth;
maxNumLen = src._maxNumLen;
maxStringLen = src._maxStringLen;
}

public StreamReadConstraints build() {
return new StreamReadConstraints(maxNumLen, maxStringLen);
return new StreamReadConstraints(maxNestingDepth, maxNumLen, maxStringLen);
}
}

Expand All @@ -103,7 +131,8 @@ public StreamReadConstraints build() {
/**********************************************************************
*/

StreamReadConstraints(final int maxNumLen, final int maxStringLen) {
StreamReadConstraints(final int maxNestingDepth, final int maxNumLen, final int maxStringLen) {
_maxNestingDepth = maxNestingDepth;
_maxNumLen = maxNumLen;
_maxStringLen = maxStringLen;
}
Expand All @@ -130,6 +159,16 @@ public Builder rebuild() {
/**********************************************************************
*/

/**
* Accessor for maximum depth.
* see {@link Builder#maxNestingDepth(int)} for details.
*
* @return Maximum allowed depth
*/
public int getMaxNestingDepth() {
return _maxNestingDepth;
}

/**
* Accessor for maximum length of numbers to decode.
* see {@link Builder#maxNumberLength(int)} for details.
Expand Down Expand Up @@ -158,7 +197,7 @@ public int getMaxStringLength() {

/**
* Convenience method that can be used to verify that a floating-point
* number of specified length does not exceed maximum specific by this
* number of specified length does not exceed maximum specified by this
* constraints object: if it does, a
* {@link StreamConstraintsException}
* is thrown.
Expand Down Expand Up @@ -212,4 +251,23 @@ public void validateStringLength(int length) throws StreamConstraintsException
length, _maxStringLen));
}
}

/**
* Convenience method that can be used to verify that the
* nesting depth does not exceed the maximum specified by this
* constraints object: if it does, a
* {@link StreamConstraintsException}
* is thrown.
*
* @param depth count of unclosed objects and arrays
*
* @throws StreamConstraintsException If depth exceeds maximum
*/
public void validateNestingDepth(int depth) throws StreamConstraintsException
{
if (depth > _maxNestingDepth) {
throw new StreamConstraintsException(String.format("Depth (%d) exceeds the maximum allowed nesting depth (%d)",
Copy link
Member

Choose a reason for hiding this comment

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

Not related to this PR specifically, but one thing we may want to do afterwards is to add something to note that StreamReadConstraints settings control this -- just add some more verbiage in exception message.
But I think that makes sense as a separate PR once we have nesting depth checks covered.

Copy link
Member Author

Choose a reason for hiding this comment

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

StreamConstraintsException has Javadoc that describes what the exception is about. I'm not sure there is much to be gained by making the message more verbose.

Copy link
Member

Choose a reason for hiding this comment

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

My perspective is that when someone sees a stack trace in log files, it is nice to have slightly more information on context to give Ops people bit more clue on what might be happening.

But I guess minimum level certainly is to have information on exception class itself, pointing to constraints settings. I'll see if I can add something to relevant JAva class(es) in jackson-core.

depth, _maxNestingDepth));
}
}
}
10 changes: 10 additions & 0 deletions src/main/java/com/fasterxml/jackson/core/base/ParserBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1536,4 +1536,14 @@ protected void loadMoreGuaranteed() throws IOException {

// Can't declare as deprecated, for now, but shouldn't be needed
protected void _finishString() throws IOException { }

protected final void createChildArrayContext(final int lineNr, final int colNr) throws IOException {
_parsingContext = _parsingContext.createChildArrayContext(lineNr, colNr);
_streamReadConstraints.validateNestingDepth(_parsingContext.getNestingDepth());
}

protected final void createChildObjectContext(final int lineNr, final int colNr) throws IOException {
_parsingContext = _parsingContext.createChildObjectContext(lineNr, colNr);
_streamReadConstraints.validateNestingDepth(_parsingContext.getNestingDepth());
}
}
1 change: 1 addition & 0 deletions src/main/java/com/fasterxml/jackson/core/json/JsonReadContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public JsonReadContext(JsonReadContext parent, DupDetector dups, int type, int l
_lineNr = lineNr;
_columnNr = colNr;
_index = -1;
_nestingDepth = parent == null ? 0 : parent._nestingDepth + 1;
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
30 changes: 15 additions & 15 deletions src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -752,13 +752,13 @@ public final JsonToken nextToken() throws IOException
break;
case '[':
if (!inObject) {
_parsingContext = _parsingContext.createChildArrayContext(_tokenInputRow, _tokenInputCol);
createChildArrayContext(_tokenInputRow, _tokenInputCol);
}
t = JsonToken.START_ARRAY;
break;
case '{':
if (!inObject) {
_parsingContext = _parsingContext.createChildObjectContext(_tokenInputRow, _tokenInputCol);
createChildObjectContext(_tokenInputRow, _tokenInputCol);
}
t = JsonToken.START_OBJECT;
break;
Expand Down Expand Up @@ -817,7 +817,7 @@ public final JsonToken nextToken() throws IOException
return t;
}

private final JsonToken _nextAfterName()
private final JsonToken _nextAfterName() throws IOException
{
_nameCopied = false; // need to invalidate if it was copied
JsonToken t = _nextToken;
Expand All @@ -827,9 +827,9 @@ private final JsonToken _nextAfterName()

// Also: may need to start new context?
if (t == JsonToken.START_ARRAY) {
_parsingContext = _parsingContext.createChildArrayContext(_tokenInputRow, _tokenInputCol);
createChildArrayContext(_tokenInputRow, _tokenInputCol);
} else if (t == JsonToken.START_OBJECT) {
_parsingContext = _parsingContext.createChildObjectContext(_tokenInputRow, _tokenInputCol);
createChildObjectContext(_tokenInputRow, _tokenInputCol);
}
return (_currToken = t);
}
Expand Down Expand Up @@ -1165,10 +1165,10 @@ private final JsonToken _nextTokenNotInObject(int i) throws IOException
}
switch (i) {
case '[':
_parsingContext = _parsingContext.createChildArrayContext(_tokenInputRow, _tokenInputCol);
createChildArrayContext(_tokenInputRow, _tokenInputCol);
return (_currToken = JsonToken.START_ARRAY);
case '{':
_parsingContext = _parsingContext.createChildObjectContext(_tokenInputRow, _tokenInputCol);
createChildObjectContext(_tokenInputRow, _tokenInputCol);
return (_currToken = JsonToken.START_OBJECT);
case 't':
_matchToken("true", 1);
Expand Down Expand Up @@ -1235,9 +1235,9 @@ public final String nextTextValue() throws IOException
return _textBuffer.contentsAsString();
}
if (t == JsonToken.START_ARRAY) {
_parsingContext = _parsingContext.createChildArrayContext(_tokenInputRow, _tokenInputCol);
createChildArrayContext(_tokenInputRow, _tokenInputCol);
} else if (t == JsonToken.START_OBJECT) {
_parsingContext = _parsingContext.createChildObjectContext(_tokenInputRow, _tokenInputCol);
createChildObjectContext(_tokenInputRow, _tokenInputCol);
}
return null;
}
Expand All @@ -1258,9 +1258,9 @@ public final int nextIntValue(int defaultValue) throws IOException
return getIntValue();
}
if (t == JsonToken.START_ARRAY) {
_parsingContext = _parsingContext.createChildArrayContext(_tokenInputRow, _tokenInputCol);
createChildArrayContext(_tokenInputRow, _tokenInputCol);
} else if (t == JsonToken.START_OBJECT) {
_parsingContext = _parsingContext.createChildObjectContext(_tokenInputRow, _tokenInputCol);
createChildObjectContext(_tokenInputRow, _tokenInputCol);
}
return defaultValue;
}
Expand All @@ -1281,9 +1281,9 @@ public final long nextLongValue(long defaultValue) throws IOException
return getLongValue();
}
if (t == JsonToken.START_ARRAY) {
_parsingContext = _parsingContext.createChildArrayContext(_tokenInputRow, _tokenInputCol);
createChildArrayContext(_tokenInputRow, _tokenInputCol);
} else if (t == JsonToken.START_OBJECT) {
_parsingContext = _parsingContext.createChildObjectContext(_tokenInputRow, _tokenInputCol);
createChildObjectContext(_tokenInputRow, _tokenInputCol);
}
return defaultValue;
}
Expand All @@ -1307,9 +1307,9 @@ public final Boolean nextBooleanValue() throws IOException
return Boolean.FALSE;
}
if (t == JsonToken.START_ARRAY) {
_parsingContext = _parsingContext.createChildArrayContext(_tokenInputRow, _tokenInputCol);
createChildArrayContext(_tokenInputRow, _tokenInputCol);
} else if (t == JsonToken.START_OBJECT) {
_parsingContext = _parsingContext.createChildObjectContext(_tokenInputRow, _tokenInputCol);
createChildObjectContext(_tokenInputRow, _tokenInputCol);
}
return null;
}
Expand Down
26 changes: 13 additions & 13 deletions src/main/java/com/fasterxml/jackson/core/json/UTF8DataInputJsonParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -716,10 +716,10 @@ private final JsonToken _nextTokenNotInObject(int i) throws IOException
}
switch (i) {
case '[':
_parsingContext = _parsingContext.createChildArrayContext(_tokenInputRow, _tokenInputCol);
createChildArrayContext(_tokenInputRow, _tokenInputCol);
return (_currToken = JsonToken.START_ARRAY);
case '{':
_parsingContext = _parsingContext.createChildObjectContext(_tokenInputRow, _tokenInputCol);
createChildObjectContext(_tokenInputRow, _tokenInputCol);
return (_currToken = JsonToken.START_OBJECT);
case 't':
_matchToken("true", 1);
Expand Down Expand Up @@ -754,17 +754,17 @@ private final JsonToken _nextTokenNotInObject(int i) throws IOException
return (_currToken = _handleUnexpectedValue(i));
}

private final JsonToken _nextAfterName()
private final JsonToken _nextAfterName() throws IOException
{
_nameCopied = false; // need to invalidate if it was copied
JsonToken t = _nextToken;
_nextToken = null;

// Also: may need to start new context?
if (t == JsonToken.START_ARRAY) {
_parsingContext = _parsingContext.createChildArrayContext(_tokenInputRow, _tokenInputCol);
createChildArrayContext(_tokenInputRow, _tokenInputCol);
} else if (t == JsonToken.START_OBJECT) {
_parsingContext = _parsingContext.createChildObjectContext(_tokenInputRow, _tokenInputCol);
createChildObjectContext(_tokenInputRow, _tokenInputCol);
}
return (_currToken = t);
}
Expand Down Expand Up @@ -908,9 +908,9 @@ public String nextTextValue() throws IOException
return _textBuffer.contentsAsString();
}
if (t == JsonToken.START_ARRAY) {
_parsingContext = _parsingContext.createChildArrayContext(_tokenInputRow, _tokenInputCol);
createChildArrayContext(_tokenInputRow, _tokenInputCol);
} else if (t == JsonToken.START_OBJECT) {
_parsingContext = _parsingContext.createChildObjectContext(_tokenInputRow, _tokenInputCol);
createChildObjectContext(_tokenInputRow, _tokenInputCol);
}
return null;
}
Expand All @@ -930,9 +930,9 @@ public int nextIntValue(int defaultValue) throws IOException
return getIntValue();
}
if (t == JsonToken.START_ARRAY) {
_parsingContext = _parsingContext.createChildArrayContext(_tokenInputRow, _tokenInputCol);
createChildArrayContext(_tokenInputRow, _tokenInputCol);
} else if (t == JsonToken.START_OBJECT) {
_parsingContext = _parsingContext.createChildObjectContext(_tokenInputRow, _tokenInputCol);
createChildObjectContext(_tokenInputRow, _tokenInputCol);
}
return defaultValue;
}
Expand All @@ -952,9 +952,9 @@ public long nextLongValue(long defaultValue) throws IOException
return getLongValue();
}
if (t == JsonToken.START_ARRAY) {
_parsingContext = _parsingContext.createChildArrayContext(_tokenInputRow, _tokenInputCol);
createChildArrayContext(_tokenInputRow, _tokenInputCol);
} else if (t == JsonToken.START_OBJECT) {
_parsingContext = _parsingContext.createChildObjectContext(_tokenInputRow, _tokenInputCol);
createChildObjectContext(_tokenInputRow, _tokenInputCol);
}
return defaultValue;
}
Expand All @@ -977,9 +977,9 @@ public Boolean nextBooleanValue() throws IOException
return Boolean.FALSE;
}
if (t == JsonToken.START_ARRAY) {
_parsingContext = _parsingContext.createChildArrayContext(_tokenInputRow, _tokenInputCol);
createChildArrayContext(_tokenInputRow, _tokenInputCol);
} else if (t == JsonToken.START_OBJECT) {
_parsingContext = _parsingContext.createChildObjectContext(_tokenInputRow, _tokenInputCol);
createChildObjectContext(_tokenInputRow, _tokenInputCol);
}
return null;
}
Expand Down