From 3f0c348033ecde791622cf6102b433fd3879a6be Mon Sep 17 00:00:00 2001 From: Aurash Mahbod Date: Wed, 14 Aug 2013 14:09:52 -0700 Subject: [PATCH] Allow for ref-type arrays containing null elements. Strip the null elements out before serializing the array. This is helpful in the cases where the user wants to construct an array of an inexact size for serialization. For example: User constructs array of size 5 because they anticipate adding more than 1 element before serialization. Only 3 get added, so the array looks like [Obj, Obj, Obj, null, null]. This would curently crash without this CL. All repeated fields of ref-type elements can contain null elements: repeated strings, repeated bytes, and repeated messages/groups. Change-Id: I117391c868c9a436536d70d6151780e9cc7e8227 Conflicts: src/google/protobuf/compiler/javanano/javanano_message_field.cc --- .../java/com/google/protobuf/NanoTest.java | 33 +++++++++++- .../javanano/javanano_message_field.cc | 20 +++++--- .../javanano/javanano_primitive_field.cc | 51 ++++++++++++++----- 3 files changed, 82 insertions(+), 22 deletions(-) diff --git a/java/src/test/java/com/google/protobuf/NanoTest.java b/java/src/test/java/com/google/protobuf/NanoTest.java index b9061e9ffd..68d2c457a2 100644 --- a/java/src/test/java/com/google/protobuf/NanoTest.java +++ b/java/src/test/java/com/google/protobuf/NanoTest.java @@ -2724,8 +2724,6 @@ public class NanoTest extends TestCase { // Complete equality for messages with accessors: TestNanoAccessors f = createMessageWithAccessorsForHashCodeEqualsTest(); TestNanoAccessors fEquivalent = createMessageWithAccessorsForHashCodeEqualsTest(); - System.out.println("equals: " + f.equals(fEquivalent)); - System.out.println("hashCode: " + f.hashCode() + " vs " + fEquivalent.hashCode()); // If using accessors, explicitly setting a field to its default value // should make the message different. @@ -2965,6 +2963,37 @@ public class NanoTest extends TestCase { assertEquals(TestAllTypesNano.BAR, message.repeatedPackedNestedEnum[1]); } + public void testNullRepeatedFieldElements() throws Exception { + // Check that serialization with null array elements doesn't NPE. + String string1 = "1"; + String string2 = "2"; + byte[] bytes1 = {3, 4}; + byte[] bytes2 = {5, 6}; + TestAllTypesNano.NestedMessage msg1 = new TestAllTypesNano.NestedMessage(); + msg1.bb = 7; + TestAllTypesNano.NestedMessage msg2 = new TestAllTypesNano.NestedMessage(); + msg2.bb = 8; + + TestAllTypesNano message = new TestAllTypesNano(); + message.repeatedString = new String[] {null, string1, string2}; + message.repeatedBytes = new byte[][] {bytes1, null, bytes2}; + message.repeatedNestedMessage = new TestAllTypesNano.NestedMessage[] {msg1, msg2, null}; + message.repeatedGroup = new TestAllTypesNano.RepeatedGroup[] {null, null, null}; + + byte[] serialized = MessageNano.toByteArray(message); // should not NPE + TestAllTypesNano deserialized = MessageNano.mergeFrom(new TestAllTypesNano(), serialized); + assertEquals(2, deserialized.repeatedString.length); + assertEquals(string1, deserialized.repeatedString[0]); + assertEquals(string2, deserialized.repeatedString[1]); + assertEquals(2, deserialized.repeatedBytes.length); + assertTrue(Arrays.equals(bytes1, deserialized.repeatedBytes[0])); + assertTrue(Arrays.equals(bytes2, deserialized.repeatedBytes[1])); + assertEquals(2, deserialized.repeatedNestedMessage.length); + assertEquals(msg1.bb, deserialized.repeatedNestedMessage[0].bb); + assertEquals(msg2.bb, deserialized.repeatedNestedMessage[1].bb); + assertEquals(0, deserialized.repeatedGroup.length); + } + public void testRepeatedMerge() throws Exception { // Check that merging repeated fields cause the arrays to expand with // new data. diff --git a/src/google/protobuf/compiler/javanano/javanano_message_field.cc b/src/google/protobuf/compiler/javanano/javanano_message_field.cc index 9f8298c7ac..d9abea36e6 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message_field.cc +++ b/src/google/protobuf/compiler/javanano/javanano_message_field.cc @@ -304,9 +304,12 @@ GenerateMergingCode(io::Printer* printer) const { void RepeatedMessageFieldGenerator:: GenerateSerializationCode(io::Printer* printer) const { printer->Print(variables_, - "if (this.$name$ != null) {\n" - " for ($type$ element : this.$name$) {\n" - " output.write$group_or_message$($number$, element);\n" + "if (this.$name$ != null && this.$name$.length > 0) {\n" + " for (int i = 0; i < this.$name$.length; i++) {\n" + " $type$ element = this.$name$[i];\n" + " if (element != null) {\n" + " output.write$group_or_message$($number$, element);\n" + " }\n" " }\n" "}\n"); } @@ -314,10 +317,13 @@ GenerateSerializationCode(io::Printer* printer) const { void RepeatedMessageFieldGenerator:: GenerateSerializedSizeCode(io::Printer* printer) const { printer->Print(variables_, - "if (this.$name$ != null) {\n" - " for ($type$ element : this.$name$) {\n" - " size += com.google.protobuf.nano.CodedOutputByteBufferNano\n" - " .compute$group_or_message$Size($number$, element);\n" + "if (this.$name$ != null && this.$name$.length > 0) {\n" + " for (int i = 0; i < this.$name$.length; i++) {\n" + " $type$ element = this.$name$[i];\n" + " if (element != null) {\n" + " size += com.google.protobuf.nano.CodedOutputByteBufferNano\n" + " .compute$group_or_message$Size($number$, element);\n" + " }\n" " }\n" "}\n"); } diff --git a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc index cc5fd4553a..5887a7e515 100644 --- a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc +++ b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc @@ -685,7 +685,7 @@ GenerateClearCode(io::Printer* printer) const { void RepeatedPrimitiveFieldGenerator:: GenerateMergingCode(io::Printer* printer) const { // First, figure out the length of the array, then parse. - if (descriptor_->options().packed()) { + if (descriptor_->is_packable() && descriptor_->options().packed()) { printer->Print(variables_, "int length = input.readRawVarint32();\n" "int limit = input.pushLimit(length);\n" @@ -736,14 +736,28 @@ GenerateMergingCode(io::Printer* printer) const { void RepeatedPrimitiveFieldGenerator:: GenerateRepeatedDataSizeCode(io::Printer* printer) const { - // Creates a variable dataSize and puts the serialized size in - // there. - if (FixedSize(descriptor_->type()) == -1) { + // Creates a variable dataSize and puts the serialized size in there. + // If the element type is a Java reference type, also generates + // dataCount which stores the number of non-null elements in the field. + if (IsReferenceType(GetJavaType(descriptor_))) { + printer->Print(variables_, + "int dataCount = 0;\n" + "int dataSize = 0;\n" + "for (int i = 0; i < this.$name$.length; i++) {\n" + " $type$ element = this.$name$[i];\n" + " if (element != null) {\n" + " dataCount++;\n" + " dataSize += com.google.protobuf.nano.CodedOutputByteBufferNano\n" + " .compute$capitalized_type$SizeNoTag(element);\n" + " }\n" + "}\n"); + } else if (FixedSize(descriptor_->type()) == -1) { printer->Print(variables_, "int dataSize = 0;\n" - "for ($type$ element : this.$name$) {\n" + "for (int i = 0; i < this.$name$.length; i++) {\n" + " $type$ element = this.$name$[i];\n" " dataSize += com.google.protobuf.nano.CodedOutputByteBufferNano\n" - " .compute$capitalized_type$SizeNoTag(element);\n" + " .compute$capitalized_type$SizeNoTag(element);\n" "}\n"); } else { printer->Print(variables_, @@ -757,18 +771,26 @@ GenerateSerializationCode(io::Printer* printer) const { "if (this.$name$ != null && this.$name$.length > 0) {\n"); printer->Indent(); - if (descriptor_->options().packed()) { + if (descriptor_->is_packable() && descriptor_->options().packed()) { GenerateRepeatedDataSizeCode(printer); printer->Print(variables_, "output.writeRawVarint32($tag$);\n" "output.writeRawVarint32(dataSize);\n" - "for ($type$ element : this.$name$) {\n" - " output.write$capitalized_type$NoTag(element);\n" + "for (int i = 0; i < this.$name$.length; i++) {\n" + " output.write$capitalized_type$NoTag(this.$name$[i]);\n" + "}\n"); + } else if (IsReferenceType(GetJavaType(descriptor_))) { + printer->Print(variables_, + "for (int i = 0; i < this.$name$.length; i++) {\n" + " $type$ element = this.$name$[i];\n" + " if (element != null) {\n" + " output.write$capitalized_type$($number$, element);\n" + " }\n" "}\n"); } else { printer->Print(variables_, - "for ($type$ element : this.$name$) {\n" - " output.write$capitalized_type$($number$, element);\n" + "for (int i = 0; i < this.$name$.length; i++) {\n" + " output.write$capitalized_type$($number$, this.$name$[i]);\n" "}\n"); } @@ -786,14 +808,17 @@ GenerateSerializedSizeCode(io::Printer* printer) const { printer->Print( "size += dataSize;\n"); - if (descriptor_->options().packed()) { + if (descriptor_->is_packable() && descriptor_->options().packed()) { printer->Print(variables_, "size += $tag_size$;\n" "size += com.google.protobuf.nano.CodedOutputByteBufferNano\n" " .computeRawVarint32Size(dataSize);\n"); + } else if (IsReferenceType(GetJavaType(descriptor_))) { + printer->Print(variables_, + "size += $tag_size$ * dataCount;\n"); } else { printer->Print(variables_, - "size += $tag_size$ * this.$name$.length;\n"); + "size += $tag_size$ * this.$name$.length;\n"); } printer->Outdent();