THRIFT-2210: lib/java TSimpleJSONProtocol can emit invalid JSON
Client: java
Patch: Alex Levenson
TSimpleJSONProtocol can emit invalid JSON with maps whose keys are not string
diff --git a/lib/java/src/org/apache/thrift/protocol/TSimpleJSONProtocol.java b/lib/java/src/org/apache/thrift/protocol/TSimpleJSONProtocol.java
index 33cad24..e005a4f 100644
--- a/lib/java/src/org/apache/thrift/protocol/TSimpleJSONProtocol.java
+++ b/lib/java/src/org/apache/thrift/protocol/TSimpleJSONProtocol.java
@@ -59,9 +59,17 @@
private static final TSet EMPTY_SET = new TSet();
private static final TList EMPTY_LIST = new TList();
private static final TMap EMPTY_MAP = new TMap();
+ private static final String LIST = "list";
+ private static final String SET = "set";
+ private static final String MAP = "map";
protected class Context {
protected void write() throws TException {}
+
+ /**
+ * Returns whether the current value is a key in a map
+ */
+ protected boolean isMapKey() { return false; }
}
protected class ListContext extends Context {
@@ -91,6 +99,22 @@
}
}
+ protected class MapContext extends StructContext {
+ protected boolean isKey = true;
+
+ @Override
+ protected void write() throws TException {
+ super.write();
+ isKey = !isKey;
+ }
+
+ protected boolean isMapKey() {
+ // we want to coerce map keys to json strings regardless
+ // of their type
+ return isKey;
+ }
+ }
+
protected final Context BASE_CONTEXT = new Context();
/**
@@ -119,6 +143,15 @@
}
/**
+ * Used to make sure that we are not encountering a map whose keys are containers
+ */
+ protected void assertContextIsNotMapKey(String invalidKeyType) throws CollectionMapKeyException {
+ if (writeContext_.isMapKey()) {
+ throw new CollectionMapKeyException("Cannot serialize a map with keys that are of type " + invalidKeyType);
+ }
+ }
+
+ /**
* Constructor
*/
public TSimpleJSONProtocol(TTransport trans) {
@@ -159,9 +192,10 @@
public void writeFieldStop() {}
public void writeMapBegin(TMap map) throws TException {
+ assertContextIsNotMapKey(MAP);
writeContext_.write();
trans_.write(LBRACE);
- pushWriteContext(new StructContext());
+ pushWriteContext(new MapContext());
// No metadata!
}
@@ -171,6 +205,7 @@
}
public void writeListBegin(TList list) throws TException {
+ assertContextIsNotMapKey(LIST);
writeContext_.write();
trans_.write(LBRACKET);
pushWriteContext(new ListContext());
@@ -183,6 +218,7 @@
}
public void writeSetBegin(TSet set) throws TException {
+ assertContextIsNotMapKey(SET);
writeContext_.write();
trans_.write(LBRACKET);
pushWriteContext(new ListContext());
@@ -207,8 +243,12 @@
}
public void writeI32(int i32) throws TException {
- writeContext_.write();
- _writeStringData(Integer.toString(i32));
+ if(writeContext_.isMapKey()) {
+ writeString(Integer.toString(i32));
+ } else {
+ writeContext_.write();
+ _writeStringData(Integer.toString(i32));
+ }
}
public void _writeStringData(String s) throws TException {
@@ -221,13 +261,21 @@
}
public void writeI64(long i64) throws TException {
- writeContext_.write();
- _writeStringData(Long.toString(i64));
+ if(writeContext_.isMapKey()) {
+ writeString(Long.toString(i64));
+ } else {
+ writeContext_.write();
+ _writeStringData(Long.toString(i64));
+ }
}
public void writeDouble(double dub) throws TException {
- writeContext_.write();
- _writeStringData(Double.toString(dub));
+ if(writeContext_.isMapKey()) {
+ writeString(Double.toString(dub));
+ } else {
+ writeContext_.write();
+ _writeStringData(Double.toString(dub));
+ }
}
public void writeString(String str) throws TException {
@@ -382,4 +430,9 @@
return ByteBuffer.wrap(new byte[0]);
}
+ public static class CollectionMapKeyException extends TException {
+ public CollectionMapKeyException(String message) {
+ super(message);
+ }
+ }
}
diff --git a/lib/java/test/org/apache/thrift/protocol/TestTSimpleJSONProtocol.java b/lib/java/test/org/apache/thrift/protocol/TestTSimpleJSONProtocol.java
new file mode 100644
index 0000000..3bb6a9e
--- /dev/null
+++ b/lib/java/test/org/apache/thrift/protocol/TestTSimpleJSONProtocol.java
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.thrift.protocol;
+
+import java.io.UnsupportedEncodingException;
+
+import junit.framework.TestCase;
+
+import org.apache.thrift.Fixtures;
+import org.apache.thrift.TException;
+import org.apache.thrift.transport.TMemoryBuffer;
+
+import thrift.test.CompactProtoTestStruct;
+
+public class TestTSimpleJSONProtocol extends TestCase {
+ private TMemoryBuffer buf;
+ private TSimpleJSONProtocol proto;
+
+ @Override
+ protected void setUp() throws Exception {
+ buf = new TMemoryBuffer(1000);
+ proto = new TSimpleJSONProtocol(buf);
+ }
+
+ private String bufToString() {
+ try {
+ return buf.toString("UTF-8");
+ } catch (UnsupportedEncodingException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ public void testHolyMoley() throws TException {
+ Fixtures.holyMoley.write(proto);
+ assertEquals("{\"big\":[{\"im_true\":1,\"im_false\":0,\"a_bite\":35,\"integer16\":27000,\"integer32\":16777216,\"integer64\":6000000000,\"double_precision\":3.141592653589793,\"some_characters\":\"JSON THIS! \\\"\\u0001\",\"zomg_unicode\":\"ӀⅮΝ Нοⅿоɡгаρℎ Αttαⅽκ�‼\",\"what_who\":0,\"base64\":\"base64\",\"byte_list\":[1,2,3],\"i16_list\":[1,2,3],\"i64_list\":[1,2,3]},{\"im_true\":1,\"im_false\":0,\"a_bite\":-42,\"integer16\":27000,\"integer32\":16777216,\"integer64\":6000000000,\"double_precision\":3.141592653589793,\"some_characters\":\"JSON THIS! \\\"\\u0001\",\"zomg_unicode\":\"ӀⅮΝ Нοⅿоɡгаρℎ Αttαⅽκ�‼\",\"what_who\":0,\"base64\":\"base64\",\"byte_list\":[1,2,3],\"i16_list\":[1,2,3],\"i64_list\":[1,2,3]}],\"contain\":[[],[\"then a one, two\",\"three!\",\"FOUR!!\"],[\"and a one\",\"and a two\"]],\"bonks\":{\"two\":[{\"type\":1,\"message\":\"Wait.\"},{\"type\":2,\"message\":\"What?\"}],\"three\":[],\"zero\":[]}}", bufToString());
+ }
+
+ public void testNesting() throws TException {
+ Fixtures.nesting.write(proto);
+ assertEquals("{\"my_bonk\":{\"type\":31337,\"message\":\"I am a bonk... xor!\"},\"my_ooe\":{\"im_true\":1,\"im_false\":0,\"a_bite\":-42,\"integer16\":27000,\"integer32\":16777216,\"integer64\":6000000000,\"double_precision\":3.141592653589793,\"some_characters\":\"JSON THIS! \\\"\\u0001\",\"zomg_unicode\":\"ӀⅮΝ Нοⅿоɡгаρℎ Αttαⅽκ�‼\",\"what_who\":0,\"base64\":\"base64\",\"byte_list\":[1,2,3],\"i16_list\":[1,2,3],\"i64_list\":[1,2,3]}}", bufToString());
+ }
+
+ public void testOneOfEach() throws TException {
+ Fixtures.oneOfEach.write(proto);
+ assertEquals("{\"im_true\":1,\"im_false\":0,\"a_bite\":-42,\"integer16\":27000,\"integer32\":16777216,\"integer64\":6000000000,\"double_precision\":3.141592653589793,\"some_characters\":\"JSON THIS! \\\"\\u0001\",\"zomg_unicode\":\"ӀⅮΝ Нοⅿоɡгаρℎ Αttαⅽκ�‼\",\"what_who\":0,\"base64\":\"base64\",\"byte_list\":[1,2,3],\"i16_list\":[1,2,3],\"i64_list\":[1,2,3]}", bufToString());
+ }
+
+ public void testSanePartsOfCompactProtoTestStruct() throws TException {
+ // unset all the maps with container keys
+ CompactProtoTestStruct struct = Fixtures.compactProtoTestStruct.deepCopy();
+ struct.unsetList_byte_map();
+ struct.unsetSet_byte_map();
+ struct.unsetMap_byte_map();
+ struct.write(proto);
+ assertEquals("{\"a_byte\":127,\"a_i16\":32000,\"a_i32\":1000000000,\"a_i64\":1099511627775,\"a_double\":5.6789,\"a_string\":\"my string\",\"a_binary\":\"\\u0000\\u0001\\u0002\\u0003\\u0004\\u0005\\u0006\\u0007\\b\",\"true_field\":1,\"false_field\":0,\"empty_struct_field\":{},\"byte_list\":[-127,-1,0,1,127],\"i16_list\":[-1,0,1,32767],\"i32_list\":[-1,0,255,65535,16777215,2147483647],\"i64_list\":[-1,0,255,65535,16777215,4294967295,1099511627775,281474976710655,72057594037927935,9223372036854775807],\"double_list\":[0.1,0.2,0.3],\"string_list\":[\"first\",\"second\",\"third\"],\"boolean_list\":[1,1,1,0,0,0],\"struct_list\":[{},{}],\"byte_set\":[0,1,-127,127,-1],\"i16_set\":[0,1,32767,-1],\"i32_set\":[1,2,3],\"i64_set\":[9223372036854775807,72057594037927935,65535,0,-1,255,1099511627775,281474976710655,4294967295,16777215],\"double_set\":[0.1,0.2,0.3],\"string_set\":[\"second\",\"third\",\"first\"],\"boolean_set\":[0,1],\"struct_set\":[{}],\"byte_byte_map\":{\"1\":2},\"i16_byte_map\":{\"1\":1,\"32767\":1,\"-1\":1},\"i32_byte_map\":{\"1\":1,\"2147483647\":1,\"-1\":1},\"i64_byte_map\":{\"9223372036854775807\":1,\"-1\":1,\"0\":1,\"1\":1},\"double_byte_map\":{\"1.1\":1,\"-1.1\":1},\"string_byte_map\":{\"\":0,\"second\":2,\"third\":3,\"first\":1},\"boolean_byte_map\":{\"0\":0,\"1\":1},\"byte_i16_map\":{\"1\":1,\"2\":-1,\"3\":32767},\"byte_i32_map\":{\"1\":1,\"2\":-1,\"3\":2147483647},\"byte_i64_map\":{\"1\":1,\"2\":-1,\"3\":9223372036854775807},\"byte_double_map\":{\"1\":0.1,\"2\":-0.1,\"3\":1000000.0},\"byte_string_map\":{\"1\":\"\",\"2\":\"blah\",\"3\":\"loooooooooooooong string\"},\"byte_boolean_map\":{\"1\":1,\"2\":0},\"byte_map_map\":{\"0\":{},\"1\":{\"1\":1},\"2\":{\"1\":1,\"2\":2}},\"byte_set_map\":{\"0\":[],\"1\":[1],\"2\":[1,2]},\"byte_list_map\":{\"0\":[],\"1\":[1],\"2\":[1,2]}}", bufToString());
+ }
+
+ public void testThrowsOnCollectionKeys() throws TException {
+ try {
+ Fixtures.compactProtoTestStruct.write(proto);
+ fail("this should throw a CollectionMapKeyException");
+ } catch (TSimpleJSONProtocol.CollectionMapKeyException e) {
+ //
+ }
+ }
+}