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
pull/3335/head
jasonh+personal@google.com 15 years ago
parent 6493368285
commit 9951233e9a
  1. 22
      java/src/test/java/com/google/protobuf/WireFormatTest.java
  2. 8
      src/google/protobuf/compiler/java/java_message.cc
  3. 2
      src/google/protobuf/compiler/java/java_primitive_field.cc

@ -102,6 +102,28 @@ public class WireFormatTest extends TestCase {
assertEquals(rawBytes, rawBytes2); 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 { public void testSerializeExtensionsLite() throws Exception {
// TestAllTypes and TestAllExtensions should have compatible wire formats, // TestAllTypes and TestAllExtensions should have compatible wire formats,
// so if we serialize a TestAllExtensions then parse it as TestAllTypes // so if we serialize a TestAllExtensions then parse it as TestAllTypes

@ -394,6 +394,14 @@ GenerateMessageSerializationMethods(io::Printer* printer) {
"public void writeTo(com.google.protobuf.CodedOutputStream output)\n" "public void writeTo(com.google.protobuf.CodedOutputStream output)\n"
" throws java.io.IOException {\n"); " throws java.io.IOException {\n");
printer->Indent(); 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_->extension_range_count() > 0) {
if (descriptor_->options().message_set_wire_format()) { if (descriptor_->options().message_set_wire_format()) {

@ -298,7 +298,7 @@ GenerateMembers(io::Printer* printer) const {
if (descriptor_->options().packed() && if (descriptor_->options().packed() &&
HasGeneratedMethods(descriptor_->containing_type())) { HasGeneratedMethods(descriptor_->containing_type())) {
printer->Print(variables_, printer->Print(variables_,
"private int $name$MemoizedSerializedSize;\n"); "private int $name$MemoizedSerializedSize = -1;\n");
} }
} }

Loading…
Cancel
Save