Skip to content

Commit

Permalink
Throw OutOfSpaceException instead of IllegalArgumentException.
Browse files Browse the repository at this point in the history
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: protocolbuffers#292

Change-Id: If478d68cf15bfd0662252d008e42b2bf1ff1c75e
  • Loading branch information
Charles Munger authored and brianduff committed Apr 28, 2015
1 parent a69b461 commit 6732dd7
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,12 @@ public void writeStringNoTag(final String value) throws IOException {
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();
Expand All @@ -311,7 +317,10 @@ public void writeStringNoTag(final String value) throws IOException {
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;
}
}

Expand Down
18 changes: 18 additions & 0 deletions javanano/src/test/java/com/google/protobuf/nano/NanoTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
package com.google.protobuf.nano;

import com.google.protobuf.nano.MapTestProto.TestMap;
import com.google.protobuf.nano.CodedOutputByteBufferNano;
import com.google.protobuf.nano.MapTestProto.TestMap.MessageValue;
import com.google.protobuf.nano.NanoAccessorsOuterClass.TestNanoAccessors;
import com.google.protobuf.nano.NanoHasOuterClass.TestAllTypesNanoHas;
Expand Down Expand Up @@ -2322,6 +2323,23 @@ public void testDifferentStringLengthsNano() throws Exception {
}
}

/** 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);
Expand Down

0 comments on commit 6732dd7

Please sign in to comment.