diff options
author | jasonh+personal@google.com <jasonh+personal@google.com@630680e5-0e50-0410-840e-4b1c322b438d> | 2009-12-01 18:05:21 +0000 |
---|---|---|
committer | jasonh+personal@google.com <jasonh+personal@google.com@630680e5-0e50-0410-840e-4b1c322b438d> | 2009-12-01 18:05:21 +0000 |
commit | 9951233e9a5c23ad285b0286e7e896bfd46f8397 (patch) | |
tree | 08ea80c9fb0fe98c5011143f5123cab37e9cd364 | |
parent | 6493368285ce77792f6076954d4311a9a457a826 (diff) | |
download | protobuf-9951233e9a5c23ad285b0286e7e896bfd46f8397.tar.gz protobuf-9951233e9a5c23ad285b0286e7e896bfd46f8397.tar.bz2 protobuf-9951233e9a5c23ad285b0286e7e896bfd46f8397.zip |
Fix Issue 136: the memoized serialized size for packed fields may not
be properly set. writeTo() may be invoked without a call to
getSerializedSize(), so the generated serialization methods would
write a length of 0 for non-empty packed fields. Just call
getSerializedSize() at the beginning of writeTo(): although this
means that we may compute the byte size needlessly when there
are no packed fields, in practice, getSerializedSize() will
already have been called - all of the writeTo() wrappers in
AbstractMessageLite invoke it.
Tested: new unittest case in WireFormatTest.java now passes
3 files changed, 31 insertions, 1 deletions
diff --git a/java/src/test/java/com/google/protobuf/WireFormatTest.java b/java/src/test/java/com/google/protobuf/WireFormatTest.java index bd1c6db1..6a5bd5de 100644 --- a/java/src/test/java/com/google/protobuf/WireFormatTest.java +++ b/java/src/test/java/com/google/protobuf/WireFormatTest.java @@ -102,6 +102,28 @@ public class WireFormatTest extends TestCase { assertEquals(rawBytes, rawBytes2); } + public void testSerializationPackedWithoutGetSerializedSize() + throws Exception { + // Write directly to an OutputStream, without invoking getSerializedSize() + // This used to be a bug where the size of a packed field was incorrect, + // since getSerializedSize() was never invoked. + TestPackedTypes message = TestUtil.getPackedSet(); + + // Directly construct a CodedOutputStream around the actual OutputStream, + // in case writeTo(OutputStream output) invokes getSerializedSize(); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + CodedOutputStream codedOutput = CodedOutputStream.newInstance(outputStream); + + message.writeTo(codedOutput); + + codedOutput.flush(); + + TestPackedTypes message2 = TestPackedTypes.parseFrom( + outputStream.toByteArray()); + + TestUtil.assertPackedFieldsSet(message2); + } + public void testSerializeExtensionsLite() throws Exception { // TestAllTypes and TestAllExtensions should have compatible wire formats, // so if we serialize a TestAllExtensions then parse it as TestAllTypes diff --git a/src/google/protobuf/compiler/java/java_message.cc b/src/google/protobuf/compiler/java/java_message.cc index 332a118f..99b57c95 100644 --- a/src/google/protobuf/compiler/java/java_message.cc +++ b/src/google/protobuf/compiler/java/java_message.cc @@ -394,6 +394,14 @@ GenerateMessageSerializationMethods(io::Printer* printer) { "public void writeTo(com.google.protobuf.CodedOutputStream output)\n" " throws java.io.IOException {\n"); printer->Indent(); + // writeTo(CodedOutputStream output) might be invoked without + // getSerializedSize() ever being called, but we need the memoized + // sizes in case this message has packed fields. Rather than emit checks for + // each packed field, just call getSerializedSize() up front for all messages. + // In most cases, getSerializedSize() will have already been called anyway by + // one of the wrapper writeTo() methods, making this call cheap. + printer->Print( + "getSerializedSize();\n"); if (descriptor_->extension_range_count() > 0) { if (descriptor_->options().message_set_wire_format()) { diff --git a/src/google/protobuf/compiler/java/java_primitive_field.cc b/src/google/protobuf/compiler/java/java_primitive_field.cc index 327c2053..1fbca5a2 100644 --- a/src/google/protobuf/compiler/java/java_primitive_field.cc +++ b/src/google/protobuf/compiler/java/java_primitive_field.cc @@ -298,7 +298,7 @@ GenerateMembers(io::Printer* printer) const { if (descriptor_->options().packed() && HasGeneratedMethods(descriptor_->containing_type())) { printer->Print(variables_, - "private int $name$MemoizedSerializedSize;\n"); + "private int $name$MemoizedSerializedSize = -1;\n"); } } |