From 76ab5f2b109fbd03c160b27a41935ad132287c88 Mon Sep 17 00:00:00 2001 From: Mark Hansen Date: Tue, 24 Sep 2024 16:19:42 -0700 Subject: [PATCH] CodedOutputStream: avoid updating position to go beyond end of array. In writeFixed32NoTag and writeFixed64NoTag This is a partial roll-forward of cl/673588324. PiperOrigin-RevId: 678435934 --- .../com/google/protobuf/CodedOutputStream.java | 4 ++++ .../com/google/protobuf/CodedOutputStreamTest.java | 14 ++------------ 2 files changed, 6 insertions(+), 12 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 cffa4cfb2e68..532ff3ad99b8 100644 --- a/java/core/src/main/java/com/google/protobuf/CodedOutputStream.java +++ b/java/core/src/main/java/com/google/protobuf/CodedOutputStream.java @@ -1346,6 +1346,7 @@ public final void writeUInt32NoTag(int value) throws IOException { @Override public final void writeFixed32NoTag(int value) throws IOException { + int position = this.position; // Perf: hoist field to register to avoid load/stores. try { buffer[position++] = (byte) (value & 0xFF); buffer[position++] = (byte) ((value >> 8) & 0xFF); @@ -1355,6 +1356,7 @@ public final void writeFixed32NoTag(int value) throws IOException { throw new OutOfSpaceException( String.format("Pos: %d, limit: %d, len: %d", position, limit, 1), e); } + this.position = position; // Only update position if we stayed within the array bounds. } @Override @@ -1389,6 +1391,7 @@ public final void writeUInt64NoTag(long value) throws IOException { @Override public final void writeFixed64NoTag(long value) throws IOException { + int position = this.position; // Perf: hoist field to register to avoid load/stores. try { buffer[position++] = (byte) ((int) (value) & 0xFF); buffer[position++] = (byte) ((int) (value >> 8) & 0xFF); @@ -1402,6 +1405,7 @@ public final void writeFixed64NoTag(long value) throws IOException { throw new OutOfSpaceException( String.format("Pos: %d, limit: %d, len: %d", position, limit, 1), e); } + this.position = position; // Only update position if we stayed within the array bounds. } @Override 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 2e87a10a5922..7a808e9f1308 100644 --- a/java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java +++ b/java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java @@ -294,12 +294,7 @@ public void testWriteFixed32NoTag_outOfBounds_throws() throws Exception { for (int i = 0; i < 4; i++) { Coder coder = outputType.newCoder(i); assertThrows(OutOfSpaceException.class, () -> coder.stream().writeFixed32NoTag(1)); - // These OutputTypes are not behaving correctly yet. - if (outputType != OutputType.ARRAY - && outputType != OutputType.NIO_HEAP - && outputType != OutputType.NIO_HEAP_WITH_INITIAL_OFFSET) { - assertThat(coder.stream().spaceLeft()).isEqualTo(i); - } + assertThat(coder.stream().spaceLeft()).isEqualTo(i); } } @@ -311,12 +306,7 @@ public void testWriteFixed64NoTag_outOfBounds_throws() throws Exception { for (int i = 0; i < 8; i++) { Coder coder = outputType.newCoder(i); assertThrows(OutOfSpaceException.class, () -> coder.stream().writeFixed64NoTag(1)); - // These OutputTypes are not behaving correctly yet. - if (outputType != OutputType.ARRAY - && outputType != OutputType.NIO_HEAP - && outputType != OutputType.NIO_HEAP_WITH_INITIAL_OFFSET) { - assertThat(coder.stream().spaceLeft()).isEqualTo(i); - } + assertThat(coder.stream().spaceLeft()).isEqualTo(i); } }