aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeff Davidson <jpd@google.com>2016-03-07 17:23:49 -0800
committerJeff Davidson <jpd@google.com>2016-03-08 12:57:12 -0800
commitae409fcc56e34811fbfe394d86108ea8cca908e9 (patch)
treee55109407c4f36b2d0992c6a00ca6e1ec4d1dda9
parent6180b684fac1cdd3d8fbb69d755be6ca540d0826 (diff)
downloadprotobuf-ae409fcc56e34811fbfe394d86108ea8cca908e9.tar.gz
Parse unknown enum values like full proto2.
Store unknown values in the unknown field set. For repeated fields, store unknown values while leaving known values in place. This can lead to some strange behaviors, such as new values not being serialized (because the unknown field set copy comes second on the wire), or the order of repeated fields changing (if the value is serialized when the field is unknown, and deserialized later when it becomes known). Having strange behavior be consistent with the standard implementation is nonetheless better than diverging. Bug: 26337187 Change-Id: I9fee24d05d387a46e15b07c7a8a97704b76b5f27
-rw-r--r--java/pom.xml6
-rw-r--r--java/src/test/java/com/google/protobuf/NanoTest.java63
-rw-r--r--src/google/protobuf/compiler/javanano/javanano_enum_field.cc84
-rw-r--r--src/google/protobuf/unittest_unknown_enum_values_nano.proto58
4 files changed, 197 insertions, 14 deletions
diff --git a/java/pom.xml b/java/pom.xml
index ec890c398..59083c229 100644
--- a/java/pom.xml
+++ b/java/pom.xml
@@ -190,6 +190,12 @@
<arg value="../src/google/protobuf/unittest_extension_packed_nano.proto" />
</exec>
<exec executable="../src/protoc">
+ <arg value="--javanano_out=store_unknown_fields=true:target/generated-test-sources" />
+ <arg value="--proto_path=../src" />
+ <arg value="--proto_path=src/test/java" />
+ <arg value="../src/google/protobuf/unittest_unknown_enum_values_nano.proto" />
+ </exec>
+ <exec executable="../src/protoc">
<arg value="--javanano_out=java_nano_generate_has=true,generate_equals=true:target/generated-test-sources" />
<arg value="--proto_path=../src" />
<arg value="--proto_path=src/test/java" />
diff --git a/java/src/test/java/com/google/protobuf/NanoTest.java b/java/src/test/java/com/google/protobuf/NanoTest.java
index 9f0b3c180..42b179313 100644
--- a/java/src/test/java/com/google/protobuf/NanoTest.java
+++ b/java/src/test/java/com/google/protobuf/NanoTest.java
@@ -61,6 +61,8 @@ import com.google.protobuf.nano.UnittestMultipleNano;
import com.google.protobuf.nano.UnittestRecursiveNano.RecursiveMessageNano;
import com.google.protobuf.nano.UnittestSimpleNano.SimpleMessageNano;
import com.google.protobuf.nano.UnittestSingleNano.SingleMessageNano;
+import com.google.protobuf.nano.UnknownEnumValuesNanoOuterClass.StandardVersion;
+import com.google.protobuf.nano.UnknownEnumValuesNanoOuterClass.VersionWithoutVal3;
import com.google.protobuf.nano.testext.Extensions;
import com.google.protobuf.nano.testext.Extensions.AnotherMessage;
import com.google.protobuf.nano.testext.Extensions.MessageWithGroup;
@@ -3875,6 +3877,67 @@ public class NanoTest extends TestCase {
assertTrue("bar was not deserialized correctly", Arrays.equals(new byte[] { 7 }, bar));
}
+ public void testUnknownOptionalEnumValue() throws Exception {
+ StandardVersion standardMsg = new StandardVersion();
+ standardMsg.optionalField = StandardVersion.VAL_3;
+ byte[] standardMsgBytes = MessageNano.toByteArray(standardMsg);
+
+ // When parsing and reading, the unknown value appears as UNKNOWN.
+ VersionWithoutVal3 withoutVal3Msg = new VersionWithoutVal3();
+ MessageNano.mergeFrom(withoutVal3Msg, standardMsgBytes);
+ assertEquals(VersionWithoutVal3.UNKNOWN, withoutVal3Msg.optionalField);
+
+ // When serializing and reparsing as the standard version, the old, unknown value wins out (even
+ // if a new one is set).
+ withoutVal3Msg.optionalField = VersionWithoutVal3.VAL_2;
+ byte[] withoutVal3MsgBytes = MessageNano.toByteArray(withoutVal3Msg);
+ StandardVersion standardMsgFromWithoutVal3Msg = new StandardVersion();
+ MessageNano.mergeFrom(standardMsgFromWithoutVal3Msg, withoutVal3MsgBytes);
+ assertEquals(StandardVersion.VAL_3, standardMsgFromWithoutVal3Msg.optionalField);
+ }
+
+ public void testUnknownRepeatedEnumValue() throws Exception {
+ StandardVersion standardMsg = new StandardVersion();
+ standardMsg.repeatedField = new int[] { StandardVersion.VAL_3, StandardVersion.VAL_2 };
+ byte[] standardMsgBytes = MessageNano.toByteArray(standardMsg);
+
+ // When parsing and reading, the unknown value is stripped out.
+ VersionWithoutVal3 withoutVal3Msg = new VersionWithoutVal3();
+ MessageNano.mergeFrom(withoutVal3Msg, standardMsgBytes);
+ assertTrue(Arrays.equals(new int[] { VersionWithoutVal3.VAL_2 }, withoutVal3Msg.repeatedField));
+
+ // When serializing and reparsing as the standard version, the old, unknown value reappears, but
+ // at the end of all entries (including any newly added ones).
+ withoutVal3Msg.repeatedField = new int[] { VersionWithoutVal3.VAL_2, VersionWithoutVal3.VAL_1 };
+ byte[] withoutVal3MsgBytes = MessageNano.toByteArray(withoutVal3Msg);
+ StandardVersion standardMsgFromWithoutVal3Msg = new StandardVersion();
+ MessageNano.mergeFrom(standardMsgFromWithoutVal3Msg, withoutVal3MsgBytes);
+ assertTrue(Arrays.equals(
+ new int[] { StandardVersion.VAL_2, StandardVersion.VAL_1, StandardVersion.VAL_3 },
+ standardMsgFromWithoutVal3Msg.repeatedField));
+ }
+
+ public void testUnknownRepeatedPackedEnumValue() throws Exception {
+ StandardVersion standardMsg = new StandardVersion();
+ standardMsg.packedField = new int[] { StandardVersion.VAL_3, StandardVersion.VAL_2 };
+ byte[] standardMsgBytes = MessageNano.toByteArray(standardMsg);
+
+ // When parsing and reading, the unknown value is stripped out.
+ VersionWithoutVal3 withoutVal3Msg = new VersionWithoutVal3();
+ MessageNano.mergeFrom(withoutVal3Msg, standardMsgBytes);
+ assertTrue(Arrays.equals(new int[] { VersionWithoutVal3.VAL_2 }, withoutVal3Msg.packedField));
+
+ // When serializing and reparsing as the standard version, the old, unknown value reappears, but
+ // at the end of all entries (including any newly added ones).
+ withoutVal3Msg.packedField = new int[] { VersionWithoutVal3.VAL_2, VersionWithoutVal3.VAL_1 };
+ byte[] withoutVal3MsgBytes = MessageNano.toByteArray(withoutVal3Msg);
+ StandardVersion standardMsgFromWithoutVal3Msg = new StandardVersion();
+ MessageNano.mergeFrom(standardMsgFromWithoutVal3Msg, withoutVal3MsgBytes);
+ assertTrue(Arrays.equals(
+ new int[] { StandardVersion.VAL_2, StandardVersion.VAL_1, StandardVersion.VAL_3 },
+ standardMsgFromWithoutVal3Msg.packedField));
+ }
+
private void assertHasWireData(MessageNano message, boolean expected) {
byte[] bytes = MessageNano.toByteArray(message);
int wireLength = bytes.length;
diff --git a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc
index 7666db38a..39a4eebcb 100644
--- a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc
@@ -144,6 +144,9 @@ GenerateClearCode(io::Printer* printer) const {
void EnumFieldGenerator::
GenerateMergingCode(io::Printer* printer) const {
+ if (params_.store_unknown_fields()) {
+ printer->Print("int initialPos = input.getPosition();\n");
+ }
printer->Print(variables_,
"int value = input.readInt32();\n"
"switch (value) {\n");
@@ -155,11 +158,20 @@ GenerateMergingCode(io::Printer* printer) const {
" has$capitalized_name$ = true;\n");
}
printer->Print(
- " break;\n"
- "}\n");
- // No default case: in case of invalid value from the wire, preserve old
- // field value. Also we are not storing the invalid value into the unknown
- // fields, because there is no way to get the value out.
+ " break;\n");
+ if (params_.store_unknown_fields()) {
+ // If storing unknown fields, store invalid values there.
+ // This is consistent with full protobuf, but note that if a client writes
+ // a new value to this field, both will be serialized on the wire, and
+ // other clients which are aware of unknown fields will see the previous
+ // value, not the new one.
+ printer->Print(
+ " default:\n"
+ " input.rewindToPosition(initialPos);\n"
+ " storeUnknownField(input, tag);\n"
+ " break;\n");
+ }
+ printer->Print("}\n");
}
void EnumFieldGenerator::
@@ -300,6 +312,9 @@ GenerateClearCode(io::Printer* printer) const {
void AccessorEnumFieldGenerator::
GenerateMergingCode(io::Printer* printer) const {
+ if (params_.store_unknown_fields()) {
+ printer->Print("int initialPos = input.getPosition();\n");
+ }
printer->Print(variables_,
"int value = input.readInt32();\n"
"switch (value) {\n");
@@ -307,11 +322,20 @@ GenerateMergingCode(io::Printer* printer) const {
printer->Print(variables_,
" $name$_ = value;\n"
" $set_has$;\n"
- " break;\n"
- "}\n");
- // No default case: in case of invalid value from the wire, preserve old
- // field value. Also we are not storing the invalid value into the unknown
- // fields, because there is no way to get the value out.
+ " break;\n");
+ if (params_.store_unknown_fields()) {
+ // If storing unknown fields, store invalid values there.
+ // This is consistent with full protobuf, but note that if a client writes
+ // a new value to this field, both will be serialized on the wire, and
+ // other clients which are aware of unknown fields will see the previous
+ // value, not the new one.
+ printer->Print(
+ " default:\n"
+ " input.rewindToPosition(initialPos);\n"
+ " storeUnknownField(input, tag);\n"
+ " break;\n");
+ }
+ printer->Print("}\n");
}
void AccessorEnumFieldGenerator::
@@ -381,7 +405,11 @@ GenerateMergingCode(io::Printer* printer) const {
"for (int i = 0; i < length; i++) {\n"
" if (i != 0) { // tag for first value already consumed.\n"
" input.readTag();\n"
- " }\n"
+ " }\n");
+ if (params_.store_unknown_fields()) {
+ printer->Print(" int initialPos = input.getPosition();\n");
+ }
+ printer->Print(
" int value = input.readInt32();\n"
" switch (value) {\n");
printer->Indent();
@@ -389,7 +417,19 @@ GenerateMergingCode(io::Printer* printer) const {
printer->Outdent();
printer->Print(variables_,
" validValues[validCount++] = value;\n"
- " break;\n"
+ " break;\n");
+ if (params_.store_unknown_fields()) {
+ // If storing unknown fields, store invalid values there.
+ // This is consistent with full protobuf. Note that this can lead to very
+ // strange behaviors if a value is serialized and reread, e.g. changes in
+ // value ordering.
+ printer->Print(
+ " default:\n"
+ " input.rewindToPosition(initialPos);\n"
+ " storeUnknownField(input, tag);\n"
+ " break;\n");
+ }
+ printer->Print(variables_,
" }\n"
"}\n"
"if (validCount != 0) {\n"
@@ -432,7 +472,11 @@ GenerateMergingCodeFromPacked(io::Printer* printer) const {
" if (i != 0) {\n"
" java.lang.System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
" }\n"
- " while (input.getBytesUntilLimit() > 0) {\n"
+ " while (input.getBytesUntilLimit() > 0) {\n");
+ if (params_.store_unknown_fields()) {
+ printer->Print(" int initialPos = input.getPosition();\n");
+ }
+ printer->Print(variables_,
" int value = input.readInt32();\n"
" switch (value) {\n");
printer->Indent();
@@ -442,7 +486,19 @@ GenerateMergingCodeFromPacked(io::Printer* printer) const {
printer->Outdent();
printer->Print(variables_,
" newArray[i++] = value;\n"
- " break;\n"
+ " break;\n");
+ if (params_.store_unknown_fields()) {
+ // If storing unknown fields, store invalid values there.
+ // This is consistent with full protobuf. Note that this can lead to very
+ // strange behaviors if a value is serialized and reread, e.g. changes in
+ // value ordering.
+ printer->Print(variables_,
+ " default:\n"
+ " input.rewindToPosition(initialPos);\n"
+ " storeUnknownField(input, $non_packed_tag$);\n"
+ " break;\n");
+ }
+ printer->Print(variables_,
" }\n"
" }\n"
" this.$name$ = newArray;\n"
diff --git a/src/google/protobuf/unittest_unknown_enum_values_nano.proto b/src/google/protobuf/unittest_unknown_enum_values_nano.proto
new file mode 100644
index 000000000..59d91c489
--- /dev/null
+++ b/src/google/protobuf/unittest_unknown_enum_values_nano.proto
@@ -0,0 +1,58 @@
+// Protocol Buffers - Google's data interchange format
+// Copyright 2016 Google Inc. All rights reserved.
+// http://code.google.com/p/protobuf/
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+package protobuf_unittest;
+
+option java_package = "com.google.protobuf.nano";
+option java_outer_classname = "UnknownEnumValuesNanoOuterClass";
+
+message StandardVersion {
+ enum Enum {
+ UNKNOWN = 0;
+ VAL_1 = 1;
+ VAL_2 = 2;
+ VAL_3 = 3;
+ }
+ optional Enum optional_field = 1;
+ repeated Enum repeated_field = 2;
+ repeated Enum packed_field = 3 [packed = true];
+}
+
+message VersionWithoutVal3 {
+ enum Enum {
+ UNKNOWN = 0;
+ VAL_1 = 1;
+ VAL_2 = 2;
+ // VAL_3 is unknown in this version.
+ }
+ optional Enum optional_field = 1;
+ repeated Enum repeated_field = 2;
+ repeated Enum packed_field = 3 [packed = true];
+}