Skip to content

Commit fa82155

Browse files
Fix mutability bug in Java proto lite: sub-messages inside of oneofs were not
being set as immutable. This would allow code to get a builder for a sub-message and modify the original (supposedly immutable) copy. PiperOrigin-RevId: 511598810
1 parent dbd5ae8 commit fa82155

File tree

3 files changed

+24
-7
lines changed

3 files changed

+24
-7
lines changed

java/core/src/main/java/com/google/protobuf/MessageSchema.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5510,6 +5510,12 @@ public void makeImmutable(T message) {
55105510
getMessageFieldSchema(pos).makeImmutable(UNSAFE.getObject(message, offset));
55115511
}
55125512
break;
5513+
case 60: // ONEOF_MESSAGE
5514+
case 68: // ONEOF_GROUP
5515+
if (isOneofPresent(message, numberAt(pos), pos)) {
5516+
getMessageFieldSchema(pos).makeImmutable(UNSAFE.getObject(message, offset));
5517+
}
5518+
break;
55135519
case 18: // DOUBLE_LIST:
55145520
case 19: // FLOAT_LIST:
55155521
case 20: // INT64_LIST:

java/lite/src/test/java/com/google/protobuf/LiteTest.java

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,18 @@ public void testAddAll() {
190190
}
191191
}
192192

193+
@Test
194+
public void testParsedOneofSubMessageIsImmutable() throws InvalidProtocolBufferException {
195+
TestAllTypesLite message =
196+
TestAllTypesLite.parseFrom(
197+
TestAllTypesLite.newBuilder()
198+
.setOneofNestedMessage(NestedMessage.newBuilder().addDd(1234).build())
199+
.build()
200+
.toByteArray());
201+
IntArrayList subList = (IntArrayList) message.getOneofNestedMessage().getDdList();
202+
assertThat(subList.isModifiable()).isFalse();
203+
}
204+
193205
@Test
194206
public void testMemoization() throws Exception {
195207
GeneratedMessageLite<?, ?> message = TestUtilLite.getAllLiteExtensionsSet();
@@ -2365,8 +2377,7 @@ public void testEqualsAndHashCodeWithUnknownFields() throws InvalidProtocolBuffe
23652377
Foo fooWithOnlyValue = Foo.newBuilder().setValue(1).build();
23662378

23672379
Foo fooWithValueAndExtension =
2368-
fooWithOnlyValue
2369-
.toBuilder()
2380+
fooWithOnlyValue.toBuilder()
23702381
.setValue(1)
23712382
.setExtension(Bar.fooExt, Bar.newBuilder().setName("name").build())
23722383
.build();
@@ -2382,8 +2393,7 @@ public void testEqualsAndHashCodeWithExtensions() throws InvalidProtocolBufferEx
23822393
Foo fooWithOnlyValue = Foo.newBuilder().setValue(1).build();
23832394

23842395
Foo fooWithValueAndExtension =
2385-
fooWithOnlyValue
2386-
.toBuilder()
2396+
fooWithOnlyValue.toBuilder()
23872397
.setValue(1)
23882398
.setExtension(Bar.fooExt, Bar.newBuilder().setName("name").build())
23892399
.build();
@@ -2515,9 +2525,9 @@ public void testParseFromByteBufferThrows_extensions() {
25152525
assertWithMessage("expected exception").fail();
25162526
} catch (InvalidProtocolBufferException expected) {
25172527
assertThat(
2518-
TestAllExtensionsLite.newBuilder()
2519-
.setExtension(UnittestLite.optionalInt32ExtensionLite, 123)
2520-
.build())
2528+
TestAllExtensionsLite.newBuilder()
2529+
.setExtension(UnittestLite.optionalInt32ExtensionLite, 123)
2530+
.build())
25212531
.isEqualTo(expected.getUnfinishedMessage());
25222532
}
25232533
}

src/google/protobuf/unittest_lite.proto

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ message TestAllTypesLite {
4747
message NestedMessage {
4848
optional int32 bb = 1;
4949
optional int64 cc = 2;
50+
repeated int32 dd = 3 [packed = true];
5051
}
5152

5253
message NestedMessage2 {

0 commit comments

Comments
 (0)