Skip to content

Commit 01b8835

Browse files
cowtowncodercowtowncoder
committed
Fix #265 
1 parent e2e81da commit 01b8835

File tree

4 files changed

+165
-15
lines changed

4 files changed

+165
-15
lines changed

‎release-notes/VERSION-2.x

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ Modules:
2424
(reported by Fabian M)
2525
#263 (smile) Handle invalid chunked-binary-format length gracefully
2626
(reported by Fabian M)
27+
#265 (smile) Allocate byte[] lazily for longer Smile binary data payloads
28+
(7-bit encoded)
2729

2830
2.12.2 (03-Mar-2021)
2931

‎smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParser.java

Lines changed: 122 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -271,13 +271,41 @@ protected final boolean _loadMore() throws IOException
271271

272272
/**
273273
* Helper method that will try to load at least specified number bytes in
274-
* input buffer, possible moving existing data around if necessary
274+
* input buffer, possible moving existing data around if necessary.
275+
* Exception throws if not enough content can be read.
276+
*
277+
* @param minAvailable Minimum number of bytes we absolutely need
278+
*
279+
* @throws IOException if read failed, either due to I/O issue or because not
280+
* enough content could be read before end-of-input.
275281
*/
276282
protected final void _loadToHaveAtLeast(int minAvailable) throws IOException
277283
{
278284
// No input stream, no leading (either we are closed, or have non-stream input source)
279285
if (_inputStream == null) {
280-
throw _constructError("Needed to read "+minAvailable+" bytes, reached end-of-input");
286+
throw _constructError(String.format(
287+
"Needed to read %d bytes, reached end-of-input", minAvailable));
288+
}
289+
int missing = _tryToLoadToHaveAtLeast(minAvailable);
290+
if (missing > 0) {
291+
throw _constructError(String.format(
292+
"Needed to read %d bytes, only got %d before end-of-input", minAvailable, minAvailable - missing));
293+
}
294+
}
295+
296+
/**
297+
* Helper method that will try to load at least specified number bytes in
298+
* input buffer, possible moving existing data around if necessary.
299+
*
300+
* @return Number of bytes that were missing, if any; {@code 0} for successful
301+
* read
302+
*
303+
* @since 2.12.3
304+
*/
305+
protected final int _tryToLoadToHaveAtLeast(int minAvailable) throws IOException
306+
{
307+
if (_inputStream == null) {
308+
return minAvailable;
281309
}
282310
// Need to move remaining data in front?
283311
int amount = _inputEnd - _inputPtr;
@@ -291,18 +319,20 @@ protected final void _loadToHaveAtLeast(int minAvailable) throws IOException
291319
}
292320
_inputPtr = 0;
293321
while (_inputEnd < minAvailable) {
294-
int count = _inputStream.read(_inputBuffer, _inputEnd, _inputBuffer.length - _inputEnd);
322+
final int toRead = _inputBuffer.length - _inputEnd;
323+
int count = _inputStream.read(_inputBuffer, _inputEnd, toRead);
295324
if (count < 1) {
296325
// End of input
297326
_closeInput();
298327
// Should never return 0, so let's fail
299328
if (count == 0) {
300329
throw new IOException("InputStream.read() returned 0 characters when trying to read "+amount+" bytes");
301330
}
302-
throw _constructError("Needed to read "+minAvailable+" bytes, missed "+minAvailable+" before end-of-input");
331+
return minAvailable - _inputEnd;
303332
}
304333
_inputEnd += count;
305334
}
335+
return 0;
306336
}
307337

308338
@SuppressWarnings("deprecation")
@@ -2459,7 +2489,7 @@ private final byte[] _finishBinaryRaw() throws IOException
24592489

24602490
if (_inputPtr >= _inputEnd) {
24612491
if (!_loadMore()) {
2462-
_reportIncompleteBinaryRead(expLen, 0);
2492+
_reportIncompleteBinaryReadRaw(expLen, 0);
24632493
}
24642494
_loadMoreGuaranteed();
24652495
}
@@ -2474,7 +2504,7 @@ private final byte[] _finishBinaryRaw() throws IOException
24742504
return b;
24752505
}
24762506
if (!_loadMore()) {
2477-
_reportIncompleteBinaryRead(expLen, ptr);
2507+
_reportIncompleteBinaryReadRaw(expLen, ptr);
24782508
}
24792509
}
24802510
}
@@ -2492,7 +2522,7 @@ protected byte[] _finishBinaryRawLong(final int expLen) throws IOException
24922522
int avail = _inputEnd - _inputPtr;
24932523
if (avail <= 0) {
24942524
if (!_loadMore()) {
2495-
_reportIncompleteBinaryRead(expLen, expLen-left);
2525+
_reportIncompleteBinaryReadRaw(expLen, expLen-left);
24962526
}
24972527
avail = _inputEnd - _inputPtr;
24982528
}
@@ -2510,12 +2540,12 @@ protected byte[] _finishBinaryRawLong(final int expLen) throws IOException
25102540
// followed by encoded data
25112541
private final byte[] _read7BitBinaryWithLength() throws IOException
25122542
{
2513-
int byteLen = _readUnsignedVInt();
2543+
final int byteLen = _readUnsignedVInt();
25142544

25152545
// 20-Mar-2021, tatu [dataformats-binary#260]: avoid eager allocation
25162546
// for very large content
25172547
if (byteLen > LONGEST_NON_CHUNKED_BINARY) {
2518-
// return _finishBinary7BitLong(byteLen);
2548+
return _finishBinary7BitLong(byteLen);
25192549
}
25202550

25212551
final byte[] result = new byte[byteLen];
@@ -2525,7 +2555,10 @@ private final byte[] _read7BitBinaryWithLength() throws IOException
25252555
// first, read all 7-by-8 byte chunks
25262556
while (ptr <= lastOkPtr) {
25272557
if ((_inputEnd - _inputPtr) < 8) {
2528-
_loadToHaveAtLeast(8);
2558+
int missing = _tryToLoadToHaveAtLeast(8);
2559+
if (missing > 0) {
2560+
_reportIncompleteBinaryRead7Bit(byteLen, ptr);
2561+
}
25292562
}
25302563
int i1 = (_inputBuffer[_inputPtr++] << 25)
25312564
+ (_inputBuffer[_inputPtr++] << 18)
@@ -2550,7 +2583,11 @@ private final byte[] _read7BitBinaryWithLength() throws IOException
25502583
int toDecode = (result.length - ptr);
25512584
if (toDecode > 0) {
25522585
if ((_inputEnd - _inputPtr) < (toDecode+1)) {
2553-
_loadToHaveAtLeast(toDecode+1);
2586+
int missing = _tryToLoadToHaveAtLeast(toDecode+1);
2587+
if (missing > 0) {
2588+
_reportIncompleteBinaryRead7Bit(byteLen, ptr);
2589+
}
2590+
25542591
}
25552592
int value = _inputBuffer[_inputPtr++];
25562593
for (int i = 1; i < toDecode; ++i) {
@@ -2567,7 +2604,66 @@ private final byte[] _read7BitBinaryWithLength() throws IOException
25672604
// @since 2.12.3
25682605
protected byte[] _finishBinary7BitLong(final int expLen) throws IOException
25692606
{
2570-
return null;
2607+
// No need to try to use recycled instance since we have much longer content
2608+
// and there is likely less benefit of trying to recycle segments
2609+
try (final ByteArrayBuilder bb = new ByteArrayBuilder(LONGEST_NON_CHUNKED_BINARY >> 1)) {
2610+
// Decode 1k input chunk at a time
2611+
final byte[] buffer = new byte[7 * 128];
2612+
int left = expLen;
2613+
int bufPtr = 0;
2614+
2615+
// Main loop for full 7/8 units:
2616+
while (left >= 7) {
2617+
if ((_inputEnd - _inputPtr) < 8) {
2618+
int missing = _tryToLoadToHaveAtLeast(8);
2619+
if (missing > 0) {
2620+
_reportIncompleteBinaryRead7Bit(expLen, bb.size() + bufPtr);
2621+
}
2622+
}
2623+
int i1 = (_inputBuffer[_inputPtr++] << 25)
2624+
+ (_inputBuffer[_inputPtr++] << 18)
2625+
+ (_inputBuffer[_inputPtr++] << 11)
2626+
+ (_inputBuffer[_inputPtr++] << 4);
2627+
int x = _inputBuffer[_inputPtr++];
2628+
i1 += x >> 3;
2629+
int i2 = ((x & 0x7) << 21)
2630+
+ (_inputBuffer[_inputPtr++] << 14)
2631+
+ (_inputBuffer[_inputPtr++] << 7)
2632+
+ _inputBuffer[_inputPtr++];
2633+
// Ok: got our 7 bytes, just need to split, copy
2634+
buffer[bufPtr++] = (byte)(i1 >> 24);
2635+
buffer[bufPtr++] = (byte)(i1 >> 16);
2636+
buffer[bufPtr++] = (byte)(i1 >> 8);
2637+
buffer[bufPtr++] = (byte)i1;
2638+
buffer[bufPtr++] = (byte)(i2 >> 16);
2639+
buffer[bufPtr++] = (byte)(i2 >> 8);
2640+
buffer[bufPtr++] = (byte)i2;
2641+
if (bufPtr >= buffer.length) {
2642+
bb.write(buffer, 0, bufPtr);
2643+
bufPtr = 0;
2644+
}
2645+
}
2646+
2647+
// And then the last one; we know there is room in buffer so:
2648+
// and then leftovers: n+1 bytes to decode n bytes
2649+
if (left > 0) {
2650+
if ((_inputEnd - _inputPtr) < (left+1)) {
2651+
_loadToHaveAtLeast(left+1);
2652+
}
2653+
int value = _inputBuffer[_inputPtr++];
2654+
for (int i = 1; i < left; ++i) {
2655+
value = (value << 7) + _inputBuffer[_inputPtr++];
2656+
buffer[bufPtr++] = (byte) (value >> (7 - i));
2657+
}
2658+
// last byte is different, has remaining 1 - 6 bits, right-aligned
2659+
value <<= left;
2660+
buffer[bufPtr] = (byte) (value + _inputBuffer[_inputPtr++]);
2661+
}
2662+
if (bufPtr > 0) {
2663+
bb.write(buffer, 0, bufPtr);
2664+
}
2665+
return bb.toByteArray();
2666+
}
25712667
}
25722668

25732669
/*
@@ -2840,12 +2936,24 @@ protected void _reportInvalidOther(int mask, int ptr) throws JsonParseException
28402936
}
28412937

28422938
// @since 2.12.3
2843-
protected void _reportIncompleteBinaryRead(int expLen, int actLen) throws IOException
2939+
protected void _reportIncompleteBinaryReadRaw(int expLen, int actLen) throws IOException
28442940
{
2845-
_reportInvalidEOF(String.format(" for Binary value: expected %d bytes, only found %d",
2941+
_reportInvalidEOF(String.format(
2942+
" for Binary value (raw): expected %d bytes, only found %d",
28462943
expLen, actLen), currentToken());
28472944
}
28482945

2946+
// @since 2.12.3
2947+
protected void _reportIncompleteBinaryRead7Bit(int expLen, int actLen)
2948+
throws IOException
2949+
{
2950+
// Calculate number of bytes needed (1 encoded byte expresses 7 payload bits):
2951+
final long encodedLen = (7L + 8L * expLen) / 7L;
2952+
_reportInvalidEOF(String.format(
2953+
" for Binary value (7-bit): expected %d payload bytes (from %d encoded), only decoded %d",
2954+
expLen, encodedLen, actLen), currentToken());
2955+
}
2956+
28492957
/*
28502958
/**********************************************************
28512959
/* Internal methods, other

‎smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz32180RawBinaryTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ public void testInvalidRawBinary() throws Exception
3131
try {
3232
/*JsonNode root =*/ MAPPER.readTree(bytes);
3333
} catch (JsonEOFException e) {
34-
verifyException(e, "Unexpected end-of-input for Binary value: expected 2145650751 bytes, only found 66550");
34+
verifyException(e,
35+
"Unexpected end-of-input for Binary value (raw): expected 2145650751 bytes, only found 66550");
3536
} catch (OutOfMemoryError e) {
3637
// Just to make it easier to see on fail (not ideal but better than nothing)
3738
e.printStackTrace();

‎smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/FuzzXXXXX_7BitBinaryTest.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package com.fasterxml.jackson.dataformat.smile.fuzz;
2+
3+
import java.io.ByteArrayInputStream;
4+
import java.util.Arrays;
5+
6+
import com.fasterxml.jackson.core.io.JsonEOFException;
7+
import com.fasterxml.jackson.databind.ObjectMapper;
8+
import com.fasterxml.jackson.dataformat.smile.BaseTestForSmile;
9+
10+
// For [dataformats-binary#265]
11+
public class FuzzXXXXX_7BitBinaryTest extends BaseTestForSmile
12+
{
13+
private final ObjectMapper MAPPER = smileMapper();
14+
15+
// Test with maximum declared payload size -- CF-32377
16+
public void testInvalid7BitBinary() throws Exception
17+
{
18+
final byte[] input0 = new byte[] {
19+
0x3A, 0x29, 0x0A, 0x00, // smile signature
20+
(byte) 0xE8, // binary, 7-bit encoded
21+
0x0F, 0x7E, 0x20,
22+
0x20, (byte) 0xFF, // 5 byte VInt for 0x7fe4083f (close to Integer.MAX_VALUE)
23+
};
24+
25+
// Let's expand slightly to avoid too early detection, ensure that we are
26+
// not only checking completely missing payload or such
27+
final byte[] input = Arrays.copyOf(input0, 65 * 1024);
28+
29+
try (ByteArrayInputStream bytes = new ByteArrayInputStream(input)) {
30+
try {
31+
/*JsonNode root =*/ MAPPER.readTree(bytes);
32+
} catch (JsonEOFException e) {
33+
verifyException(e, "Unexpected end-of-input for Binary value (7-bit)",
34+
"expected 2145650751 bytes (from 2452172287 encoded)",
35+
"only decoded 58226");
36+
}
37+
}
38+
}
39+
}

0 commit comments

Comments
 (0)