From ebfc42610b47af51de7ce9eacc65960a80c6d490 Mon Sep 17 00:00:00 2001 From: Charles Munger Date: Tue, 21 Apr 2015 14:35:46 -0700 Subject: Throw OutOfSpaceException instead of IllegalArgumentException. When a MessageNano containing a String is serialized into a buffer that is too small to contain it, and the buffer's boundary happens to be where the string field's length delimiting varint is serialized, and the string's length and 3*length have the same length when encoded as a varint, an IllegalArgumentException is thrown rather than an OutOfSpaceException. Github issue: https://github.com/google/protobuf/issues/292 Change-Id: If478d68cf15bfd0662252d008e42b2bf1ff1c75e --- .../protobuf/nano/CodedOutputByteBufferNano.java | 11 ++++++++++- java/src/test/java/com/google/protobuf/NanoTest.java | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/java/src/main/java/com/google/protobuf/nano/CodedOutputByteBufferNano.java b/java/src/main/java/com/google/protobuf/nano/CodedOutputByteBufferNano.java index 324a63f..53060da 100644 --- a/java/src/main/java/com/google/protobuf/nano/CodedOutputByteBufferNano.java +++ b/java/src/main/java/com/google/protobuf/nano/CodedOutputByteBufferNano.java @@ -300,6 +300,12 @@ public final class CodedOutputByteBufferNano { final int maxLengthVarIntSize = computeRawVarint32Size(value.length() * MAX_UTF8_EXPANSION); if (minLengthVarIntSize == maxLengthVarIntSize) { int oldPosition = buffer.position(); + // Buffer.position, when passed a position that is past its limit, throws + // IllegalArgumentException, and this class is documented to throw + // OutOfSpaceException instead. + if (buffer.remaining() < minLengthVarIntSize) { + throw new OutOfSpaceException(oldPosition + minLengthVarIntSize, buffer.limit()); + } buffer.position(oldPosition + minLengthVarIntSize); encode(value, buffer); int newPosition = buffer.position(); @@ -311,7 +317,10 @@ public final class CodedOutputByteBufferNano { encode(value, buffer); } } catch (BufferOverflowException e) { - throw new OutOfSpaceException(buffer.position(), buffer.limit()); + final OutOfSpaceException outOfSpaceException = new OutOfSpaceException(buffer.position(), + buffer.limit()); + outOfSpaceException.initCause(e); + throw outOfSpaceException; } } diff --git a/java/src/test/java/com/google/protobuf/NanoTest.java b/java/src/test/java/com/google/protobuf/NanoTest.java index 4c3b416..ef1572a 100644 --- a/java/src/test/java/com/google/protobuf/NanoTest.java +++ b/java/src/test/java/com/google/protobuf/NanoTest.java @@ -31,6 +31,7 @@ package com.google.protobuf; import com.google.protobuf.nano.CodedInputByteBufferNano; +import com.google.protobuf.nano.CodedOutputByteBufferNano; import com.google.protobuf.nano.EnumClassNanoMultiple; import com.google.protobuf.nano.EnumClassNanos; import com.google.protobuf.nano.EnumValidity; @@ -2340,6 +2341,23 @@ public class NanoTest extends TestCase { } } + /** Regression test for https://github.com/google/protobuf/issues/292 */ + public void testCorrectExceptionThrowWhenEncodingStringsWithoutEnoughSpace() throws Exception { + String testCase = "Foooooooo"; + assertEquals(CodedOutputByteBufferNano.computeRawVarint32Size(testCase.length()), + CodedOutputByteBufferNano.computeRawVarint32Size(testCase.length() * 3)); + assertEquals(11, CodedOutputByteBufferNano.computeStringSize(1, testCase)); + // Tag is one byte, varint describing string length is 1 byte, string length is 9 bytes. + // An array of size 1 will cause a failure when trying to write the varint. + for (int i = 0; i < 11; i++) { + CodedOutputByteBufferNano bufferNano = CodedOutputByteBufferNano.newInstance(new byte[i]); + try { + bufferNano.writeString(1, testCase); + fail("Should have thrown an out of space exception"); + } catch (CodedOutputByteBufferNano.OutOfSpaceException expected) {} + } + } + private void testEncodingOfString(char c, int length) throws InvalidProtocolBufferNanoException { TestAllTypesNano testAllTypesNano = new TestAllTypesNano(); final String fullString = fullString(c, length); -- cgit v1.1