From bc24489f93de0dc10a6aa5ed22d119bc3deac514 Mon Sep 17 00:00:00 2001 From: Mark Hansen Date: Tue, 1 Oct 2024 17:09:15 -0700 Subject: [PATCH] Throw more informative OutOfSpaceExceptions when we run out of space serializing a proto. Before, some encoders would not give any details about position/limit/length. Now a few more places do. Just found this while trying to add some tests for the exception message, and found some encoders weren't setting it. This doesn't fix all the places that OutOfSpaceException didn't have a useful message. PiperOrigin-RevId: 681218740 --- .../google/protobuf/CodedOutputStream.java | 6 ++--- .../protobuf/CodedOutputStreamTest.java | 23 +++++++++++-------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/java/core/src/main/java/com/google/protobuf/CodedOutputStream.java b/java/core/src/main/java/com/google/protobuf/CodedOutputStream.java index 342fdd923a..fefb936481 100644 --- a/java/core/src/main/java/com/google/protobuf/CodedOutputStream.java +++ b/java/core/src/main/java/com/google/protobuf/CodedOutputStream.java @@ -1666,7 +1666,7 @@ public abstract class CodedOutputStream extends ByteOutput { try { buffer.put(value); } catch (BufferOverflowException e) { - throw new OutOfSpaceException(e); + throw new OutOfSpaceException(buffer.position(), buffer.limit(), 1, e); } } @@ -1725,7 +1725,7 @@ public abstract class CodedOutputStream extends ByteOutput { try { buffer.putInt(value); } catch (BufferOverflowException e) { - throw new OutOfSpaceException(e); + throw new OutOfSpaceException(buffer.position(), buffer.limit(), FIXED32_SIZE, e); } } @@ -1751,7 +1751,7 @@ public abstract class CodedOutputStream extends ByteOutput { try { buffer.putLong(value); } catch (BufferOverflowException e) { - throw new OutOfSpaceException(e); + throw new OutOfSpaceException(buffer.position(), buffer.limit(), FIXED64_SIZE, e); } } diff --git a/java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java b/java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java index 7a808e9f13..4c6aa421d6 100644 --- a/java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java +++ b/java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java @@ -8,7 +8,6 @@ package com.google.protobuf; import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth.assertWithMessage; import static com.google.common.truth.TruthJUnit.assume; import static org.junit.Assert.assertThrows; @@ -293,7 +292,9 @@ public class CodedOutputStreamTest { for (int i = 0; i < 4; i++) { Coder coder = outputType.newCoder(i); - assertThrows(OutOfSpaceException.class, () -> coder.stream().writeFixed32NoTag(1)); + OutOfSpaceException e = + assertThrows(OutOfSpaceException.class, () -> coder.stream().writeFixed32NoTag(1)); + assertThat(e).hasMessageThat().contains("len: 4"); assertThat(coder.stream().spaceLeft()).isEqualTo(i); } } @@ -305,7 +306,9 @@ public class CodedOutputStreamTest { for (int i = 0; i < 8; i++) { Coder coder = outputType.newCoder(i); - assertThrows(OutOfSpaceException.class, () -> coder.stream().writeFixed64NoTag(1)); + OutOfSpaceException e = + assertThrows(OutOfSpaceException.class, () -> coder.stream().writeFixed64NoTag(1)); + assertThat(e).hasMessageThat().contains("len: 8"); assertThat(coder.stream().spaceLeft()).isEqualTo(i); } } @@ -577,7 +580,9 @@ public class CodedOutputStreamTest { if (outputType == OutputType.STREAM) { return; } - assertThrows(OutOfSpaceException.class, () -> coder.stream().write((byte) 1)); + OutOfSpaceException e = + assertThrows(OutOfSpaceException.class, () -> coder.stream().write((byte) 1)); + assertThat(e).hasMessageThat().contains("len: 1"); if (outputType.supportsSpaceLeft()) { assertThat(coder.stream().spaceLeft()).isEqualTo(0); } @@ -673,12 +678,10 @@ public class CodedOutputStreamTest { Coder coder = outputType.newCoder(notEnoughBytes); String invalidString = newString(Character.MIN_HIGH_SURROGATE, 'f', 'o', 'o', 'b', 'a', 'r'); - try { - coder.stream().writeStringNoTag(invalidString); - assertWithMessage("Expected OutOfSpaceException").fail(); - } catch (OutOfSpaceException e) { - assertThat(e).hasCauseThat().isInstanceOf(IndexOutOfBoundsException.class); - } + OutOfSpaceException e = + assertThrows( + OutOfSpaceException.class, () -> coder.stream().writeStringNoTag(invalidString)); + assertThat(e).hasCauseThat().isInstanceOf(IndexOutOfBoundsException.class); } /** Regression test for https://github.com/protocolbuffers/protobuf/issues/292 */