THRIFT-670. java: Unions don't skip unrecognizable fields correctly
This patch adds a test and a fix for the bug.
git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@894924 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/compiler/cpp/src/generate/t_java_generator.cc b/compiler/cpp/src/generate/t_java_generator.cc
index da1e1c6..28406d9 100644
--- a/compiler/cpp/src/generate/t_java_generator.cc
+++ b/compiler/cpp/src/generate/t_java_generator.cc
@@ -850,7 +850,10 @@
indent_up();
- indent(out) << "switch (_Fields.findByThriftId(field.id)) {" << endl;
+ indent(out) << "_Fields setField = _Fields.findByThriftId(field.id);" << endl;
+ indent(out) << "if (setField != null) {" << endl;
+ indent_up();
+ indent(out) << "switch (setField) {" << endl;
indent_up();
const vector<t_field*>& members = tstruct->get_members();
@@ -873,11 +876,18 @@
indent(out) << "}" << endl;
indent_down();
}
-
- indent(out) << "default:" << endl;
- indent(out) << " TProtocolUtil.skip(iprot, field.type);" << endl;
- indent(out) << " return null;" << endl;
+ indent(out) << "default:" << endl;
+ indent(out) << " throw new IllegalStateException(\"setField wasn't null, but didn't match any of the case statements!\");" << endl;
+
+ indent_down();
+ indent(out) << "}" << endl;
+
+ indent_down();
+ indent(out) << "} else {" << endl;
+ indent_up();
+ indent(out) << "TProtocolUtil.skip(iprot, field.type);" << endl;
+ indent(out) << "return null;" << endl;
indent_down();
indent(out) << "}" << endl;
diff --git a/lib/java/test/org/apache/thrift/test/UnionTest.java b/lib/java/test/org/apache/thrift/test/UnionTest.java
index 551e407..2527f4a 100644
--- a/lib/java/test/org/apache/thrift/test/UnionTest.java
+++ b/lib/java/test/org/apache/thrift/test/UnionTest.java
@@ -17,15 +17,18 @@
*/
package org.apache.thrift.test;
+import org.apache.thrift.TDeserializer;
+import org.apache.thrift.TSerializer;
import org.apache.thrift.protocol.TBinaryProtocol;
import org.apache.thrift.protocol.TProtocol;
import org.apache.thrift.transport.TMemoryBuffer;
+import thrift.test.ComparableUnion;
import thrift.test.Empty;
+import thrift.test.SomeEnum;
import thrift.test.StructWithAUnion;
import thrift.test.TestUnion;
-import thrift.test.SomeEnum;
-import thrift.test.ComparableUnion;
+import thrift.test.TestUnionMinusStringField;
public class UnionTest {
@@ -37,6 +40,7 @@
testEquality();
testSerialization();
testCompareTo();
+ testSkip();
}
public static void testBasic() throws Exception {
@@ -189,4 +193,17 @@
throw new RuntimeException("b was supposed to be > a, but was " + cu2.compareTo(cu));
}
}
+
+ public static void testSkip() throws Exception {
+ TestUnion tu = TestUnion.string_field("string");
+ byte[] tuSerialized = new TSerializer().serialize(tu);
+ TestUnionMinusStringField tums = new TestUnionMinusStringField();
+ new TDeserializer().deserialize(tums, tuSerialized);
+ if (tums.getSetField() != null) {
+ throw new RuntimeException("Expected tums to be unset!");
+ }
+ if (tums.getFieldValue() != null) {
+ throw new RuntimeException("Expected tums to have null value!");
+ }
+ }
}
diff --git a/test/DebugProtoTest.thrift b/test/DebugProtoTest.thrift
index 6731e5e..9cdb16d 100644
--- a/test/DebugProtoTest.thrift
+++ b/test/DebugProtoTest.thrift
@@ -299,6 +299,14 @@
6: SomeEnum enum_field;
}
+union TestUnionMinusStringField {
+ 2: i32 i32_field;
+ 3: OneOfEach struct_field;
+ 4: list<RandomStuff> struct_list;
+ 5: i32 other_i32_field;
+ 6: SomeEnum enum_field;
+}
+
union ComparableUnion {
1: string string_field;
2: binary binary_field;