-
Notifications
You must be signed in to change notification settings - Fork 321
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
Fixed UnpackerConfig.bufferSize to work #658
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -156,6 +156,7 @@ public class MessageUnpacker | |
private final CodingErrorAction actionOnUnmappableString; | ||
private final int stringSizeLimit; | ||
private final int stringDecoderBufferSize; | ||
private final int bufferSize; | ||
|
||
private MessageBufferInput in; | ||
|
||
|
@@ -215,6 +216,7 @@ protected MessageUnpacker(MessageBufferInput in, MessagePack.UnpackerConfig conf | |
this.actionOnUnmappableString = config.getActionOnUnmappableString(); | ||
this.stringSizeLimit = config.getStringSizeLimit(); | ||
this.stringDecoderBufferSize = config.getStringDecoderBufferSize(); | ||
this.bufferSize = config.getBufferSize(); | ||
} | ||
|
||
/** | ||
|
@@ -1324,27 +1326,38 @@ public Instant unpackTimestamp(ExtensionTypeHeader ext) throws IOException | |
* | ||
* @return the size of the array to be read | ||
* @throws MessageTypeException when value is not MessagePack Array type | ||
* @throws MessageSizeException when size of the array is larger than 2^31 - 1 | ||
* @throws MessageSizeException when size of the array is larger than 2^31 - 1 or bufferSize | ||
* @throws IOException when underlying input throws IOException | ||
*/ | ||
public int unpackArrayHeader() | ||
throws IOException | ||
{ | ||
byte b = readByte(); | ||
int len; | ||
if (Code.isFixedArray(b)) { // fixarray | ||
return b & 0x0f; | ||
len = b & 0x0f; | ||
} | ||
switch (b) { | ||
case Code.ARRAY16: { // array 16 | ||
int len = readNextLength16(); | ||
return len; | ||
} | ||
case Code.ARRAY32: { // array 32 | ||
int len = readNextLength32(); | ||
return len; | ||
else { | ||
switch (b) { | ||
case Code.ARRAY16: { // array 16 | ||
len = readNextLength16(); | ||
break; | ||
} | ||
case Code.ARRAY32: { // array 32 | ||
len = readNextLength32(); | ||
break; | ||
} | ||
default: { | ||
throw unexpected("Array", b); | ||
} | ||
} | ||
} | ||
throw unexpected("Array", b); | ||
|
||
if (len > bufferSize) { | ||
throw new MessageSizeException(String.format("cannot unpack a Array of size larger than %,d: %,d", bufferSize, len), len); | ||
} | ||
|
||
return len; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Throwing error here is unnecessary if readPayload checks the buffer size. |
||
} | ||
|
||
/** | ||
|
@@ -1357,27 +1370,38 @@ public int unpackArrayHeader() | |
* | ||
* @return the size of the map to be read | ||
* @throws MessageTypeException when value is not MessagePack Map type | ||
* @throws MessageSizeException when size of the map is larger than 2^31 - 1 | ||
* @throws MessageSizeException when size of the map is larger than 2^31 - 1 or bufferSize | ||
* @throws IOException when underlying input throws IOException | ||
*/ | ||
public int unpackMapHeader() | ||
throws IOException | ||
{ | ||
byte b = readByte(); | ||
int len; | ||
if (Code.isFixedMap(b)) { // fixmap | ||
return b & 0x0f; | ||
} | ||
switch (b) { | ||
case Code.MAP16: { // map 16 | ||
int len = readNextLength16(); | ||
return len; | ||
} | ||
case Code.MAP32: { // map 32 | ||
int len = readNextLength32(); | ||
return len; | ||
else { | ||
switch (b) { | ||
case Code.MAP16: { // map 16 | ||
len = readNextLength16(); | ||
break; | ||
} | ||
case Code.MAP32: { // map 32 | ||
len = readNextLength32(); | ||
break; | ||
} | ||
default: { | ||
throw unexpected("Map", b); | ||
} | ||
} | ||
} | ||
throw unexpected("Map", b); | ||
|
||
if (len > bufferSize / 2) { | ||
throw new MessageSizeException(String.format("cannot unpack a Map of size larger than %,d: %,d", bufferSize / 2, len), len); | ||
} | ||
|
||
return len; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unnecessary change for the same reason |
||
} | ||
|
||
public ExtensionTypeHeader unpackExtensionTypeHeader() | ||
|
@@ -1635,11 +1659,16 @@ public void readPayload(byte[] dst) | |
* | ||
* @param length number of bytes to be read | ||
* @return the new byte array | ||
* @throws MessageSizeException when length is larger than bufferSize | ||
* @throws IOException when underlying input throws IOException | ||
*/ | ||
public byte[] readPayload(int length) | ||
throws IOException | ||
{ | ||
if (length > bufferSize) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This part is a good catch. If that happens, instead of throwing an error, we need to enlarge the buffer size by creating a new buffer large enough for storing the expected data size. This implementation would be similar to ArrayList implementation of Java. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm sorry that I don't quite understand here. |
||
throw new MessageSizeException(String.format("%,d exceeds bufferSize:%,d", length, bufferSize), length); | ||
} | ||
|
||
byte[] newArray = new byte[length]; | ||
readPayload(newArray); | ||
return newArray; | ||
|
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.
Don't change the default buffer size. msgpack-java is frequently used even for the small size of data.
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.
Especially, creating a large buffer is not free as Java will use CPU resources for filling zeros to the array contents.