Skip to content

StreamReadConstraints: add maxStringLen #864

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

Merged
merged 24 commits into from
Feb 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,24 @@ public class StreamReadConstraints
private static final long serialVersionUID = 1L;

/**
* Default setting for maximum length: see {@link Builder#maxNumberLength(int)} for details.
* Default setting for maximum number length: see {@link Builder#maxNumberLength(int)} for details.
*/
public static final int DEFAULT_MAX_NUM_LEN = 1000;

/**
* Default setting for maximum string length: see {@link Builder#maxStringLength(int)} for details.
*/
public static final int DEFAULT_MAX_STRING_LEN = 1_000_000;

protected final int _maxNumLen;
protected final int _maxStringLen;

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

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

/**
* Sets the maximum number length (in chars or bytes, depending on input context).
Expand All @@ -44,20 +51,47 @@ public Builder maxNumberLength(final int maxNumLen) {
return this;
}

/**
* Sets the maximum string length (in chars or bytes, depending on input context).
* The default is 1,000,000. This limit is not exact, the limit is applied when we increase
* internal buffer sizes and an exception will happen at sizes greater than this limit. Some
* text values that are a little bigger than the limit may be treated as valid but no text
* values with sizes less than or equal to this limit will be treated as invalid.
* <p>
* Setting this value to lower than the {@link #maxNumberLength(int)} is not recommended.
* </p>
*
* @param maxStringLen the maximum string length (in chars or bytes, depending on input context)
*
* @return this builder
* @throws IllegalArgumentException if the maxStringLen is set to a negative value
*
* @since 2.15
*/
public Builder maxStringLength(final int maxStringLen) {
if (maxStringLen < 0) {
throw new IllegalArgumentException("Cannot set maxStringLen to a negative value");
}
this.maxStringLen = maxStringLen;
return this;
}

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

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

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

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

Expand All @@ -67,8 +101,9 @@ public StreamReadConstraints build() {
/**********************************************************************
*/

StreamReadConstraints(int maxNumLen) {
StreamReadConstraints(final int maxNumLen, final int maxStringLen) {
_maxNumLen = maxNumLen;
_maxStringLen = maxStringLen;
}

public static Builder builder() {
Expand Down Expand Up @@ -103,6 +138,16 @@ public int getMaxNumberLength() {
return _maxNumLen;
}

/**
* Accessor for maximum length of strings to decode.
* see {@link Builder#maxStringLength(int)} for details.
*
* @return Maximum allowed string length
*/
public int getMaxStringLength() {
return _maxStringLen;
}

/*
/**********************************************************************
/* Convenience methods for validation
Expand Down Expand Up @@ -146,4 +191,23 @@ public void validateIntegerLength(int length) throws NumberFormatException
length, _maxNumLen));
}
}

/**
* Convenience method that can be used to verify that a String
* of specified length does not exceed maximum specific by this
* constraints object: if it does, an
* {@link IllegalStateException}
* is thrown.
*
* @param length Length of string in input units
*
* @throws IllegalStateException If length exceeds maximum
*/
public void validateStringLength(int length) throws IllegalStateException
{
if (length > _maxStringLen) {
throw new IllegalStateException(String.format("String length (%d) exceeds the maximum length (%d)",
length, _maxStringLen));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ protected ParserBase(IOContext ctxt, int features) {
super(features);
_ioContext = ctxt;
_streamReadConstraints = ctxt.streamReadConstraints();
_textBuffer = ctxt.constructTextBuffer();
_textBuffer = ctxt.constructReadConstrainedTextBuffer();
DupDetector dups = Feature.STRICT_DUPLICATE_DETECTION.enabledIn(features)
? DupDetector.rootDetector(this) : null;
_parsingContext = JsonReadContext.createRootContext(dups);
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/com/fasterxml/jackson/core/io/IOContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.fasterxml.jackson.core.JsonEncoding;
import com.fasterxml.jackson.core.StreamReadConstraints;
import com.fasterxml.jackson.core.util.BufferRecycler;
import com.fasterxml.jackson.core.util.ReadConstrainedTextBuffer;
import com.fasterxml.jackson.core.util.TextBuffer;

/**
Expand Down Expand Up @@ -203,6 +204,10 @@ public TextBuffer constructTextBuffer() {
return new TextBuffer(_bufferRecycler);
}

public TextBuffer constructReadConstrainedTextBuffer() {
return new ReadConstrainedTextBuffer(_streamReadConstraints, _bufferRecycler);
}

/**
* Method for recycling or allocation byte buffer of "read I/O" type.
*<p>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package com.fasterxml.jackson.core.util;

import com.fasterxml.jackson.core.StreamReadConstraints;

public final class ReadConstrainedTextBuffer extends TextBuffer {

private final StreamReadConstraints _streamReadConstraints;

public ReadConstrainedTextBuffer(StreamReadConstraints streamReadConstraints, BufferRecycler bufferRecycler) {
super(bufferRecycler);
_streamReadConstraints = streamReadConstraints;
}

/*
/**********************************************************************
/* Convenience methods for validation
/**********************************************************************
*/

/**
* {@inheritDoc}
*/
@Override
protected void validateStringLength(int length) throws IllegalStateException
{
_streamReadConstraints.validateStringLength(length);
}
}
45 changes: 40 additions & 5 deletions src/main/java/com/fasterxml/jackson/core/util/TextBuffer.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
* </li>
* </ul>
*/
public final class TextBuffer
public class TextBuffer
{
final static char[] NO_CHARS = new char[0];

Expand Down Expand Up @@ -302,6 +302,7 @@ public void resetWithString(String value)
_inputStart = -1;
_inputLen = 0;

validateStringLength(value.length());
_resultString = value;
_resultArray = null;

Expand Down Expand Up @@ -480,22 +481,30 @@ public String contentsAsString()
if (_resultString == null) {
// Has array been requested? Can make a shortcut, if so:
if (_resultArray != null) {
// _resultArray length should already be validated, no need to check again
_resultString = new String(_resultArray);
} else {
// Do we use shared array?
if (_inputStart >= 0) {
if (_inputLen < 1) {
return (_resultString = "");
}
validateStringLength(_inputLen);
_resultString = new String(_inputBuffer, _inputStart, _inputLen);
} else { // nope... need to copy
// But first, let's see if we have just one buffer
int segLen = _segmentSize;
int currLen = _currentSize;

if (segLen == 0) { // yup
_resultString = (currLen == 0) ? "" : new String(_currentSegment, 0, currLen);
if (currLen == 0) {
_resultString = "";
} else {
validateStringLength(currLen);
_resultString = new String(_currentSegment, 0, currLen);
}
} else { // no, need to combine
validateStringLength(segLen + currLen);
StringBuilder sb = new StringBuilder(segLen + currLen);
// First stored segments
if (_segments != null) {
Expand Down Expand Up @@ -558,9 +567,8 @@ public float contentsAsFloat() throws NumberFormatException {
* @throws NumberFormatException if contents are not a valid Java number
* @since 2.14
*/
public float contentsAsFloat(boolean useFastParser) throws NumberFormatException {
final String numStr = contentsAsString();
return NumberInput.parseFloat(numStr, useFastParser);
public float contentsAsFloat(final boolean useFastParser) throws NumberFormatException {
return NumberInput.parseFloat(contentsAsString(), useFastParser);
}

/**
Expand Down Expand Up @@ -853,6 +861,7 @@ public String setCurrentAndReturn(int len) {
}
// more common case: single segment
int currLen = _currentSize;
validateStringLength(currLen);
String str = (currLen == 0) ? "" : new String(_currentSegment, 0, currLen);
_resultString = str;
return str;
Expand All @@ -867,6 +876,7 @@ public char[] finishCurrentSegment() {
int oldLen = _currentSegment.length;
_segmentSize += oldLen;
_currentSize = 0;
validateStringLength(_segmentSize);

// Let's grow segments by 50%
int newLen = oldLen + (oldLen >> 1);
Expand Down Expand Up @@ -998,6 +1008,7 @@ private char[] resultArray()
if (len < 1) {
return NO_CHARS;
}
validateStringLength(len);
final int start = _inputStart;
if (start == 0) {
return Arrays.copyOf(_inputBuffer, len);
Expand All @@ -1009,6 +1020,7 @@ private char[] resultArray()
if (size < 1) {
return NO_CHARS;
}
validateStringLength(size);
int offset = 0;
final char[] result = carr(size);
if (_segments != null) {
Expand All @@ -1024,4 +1036,27 @@ private char[] resultArray()
}

private char[] carr(int len) { return new char[len]; }

/*
/**********************************************************************
/* Convenience methods for validation
/**********************************************************************
*/

/**
* Convenience method that can be used to verify that a String
* of specified length does not exceed maximum specific by this
* constraints object: if it does, an
* {@link IllegalStateException}
* is thrown.
*
* @param length Length of string in input units
*
* @throws IllegalStateException If length exceeds maximum
* @since 2.15
*/
protected void validateStringLength(int length) throws IllegalStateException
{
// no-op
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,37 @@ public void testLongUnicodeStrings() throws IOException
_testStrings(f, input, data, 1, 1);
}

public void testLongAsciiStringsSmallLimit() throws 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.

@cowtowncoder the test I added to passes when it is run by itself but fails when it is run with other tests. It seems that running other tests with the default StreamReadConstraints seems to leave behind state - maybe in the BufferRecyclers - that means the validateStringLength checks don't get called.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, due to buffer recycling it is possible that recycled buffer has reached size of up to 64k chars.

So test will probably be set as much less strict: to set maximum length to say 100k, produce 200k String and verify that gets caught.

Another thing we could consider: making MAX_SEGMENT_LEN no higher than maxStringLength.
Then again... maybe not worth bothering yet.

{
final String[] input = new String[] {
LONG_ASCII,
LONG_ASCII,
LONG_ASCII,
LONG_ASCII,
LONG_ASCII
};
JsonFactory f = JsonFactory.builder()
.streamReadConstraints(StreamReadConstraints.builder().maxStringLength(100).build())
.build();
byte[] data = _stringDoc(f, input);

try (AsyncReaderWrapper r = asyncForBytes(f, 9000, data, 0)) {
// start with "no token"
assertNull(r.currentToken());
assertToken(JsonToken.START_ARRAY, r.nextToken());
for (int i = 0; i < input.length; ++i) {
r.nextToken();
r.currentText();
}
fail("expected IllegalStateException");
} catch (IllegalStateException ise) {
assertTrue("unexpected exception message: " + ise.getMessage(),
ise.getMessage().startsWith("String length"));
assertTrue("unexpected exception message: " + ise.getMessage(),
ise.getMessage().endsWith("exceeds the maximum length (100)"));
}
}

private void _testStrings(JsonFactory f, String[] values,
byte[] data, int offset, int readSize) throws IOException
{
Expand Down