From 38b453be5a015b7aaefcd91b4e261e53e0e211c2 Mon Sep 17 00:00:00 2001 From: jfarrell Date: Fri, 27 Sep 2013 10:11:12 -0400 Subject: [PATCH] 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 --- .../thrift/protocol/TSimpleJSONProtocol.java | 67 +++++++++++++-- .../protocol/TestTSimpleJSONProtocol.java | 82 +++++++++++++++++++ 2 files changed, 142 insertions(+), 7 deletions(-) create mode 100644 lib/java/test/org/apache/thrift/protocol/TestTSimpleJSONProtocol.java diff --git a/lib/java/src/org/apache/thrift/protocol/TSimpleJSONProtocol.java b/lib/java/src/org/apache/thrift/protocol/TSimpleJSONProtocol.java index 33cad24d..e005a4fc 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 @@ public class TSimpleJSONProtocol extends TProtocol { 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 @@ public class TSimpleJSONProtocol extends TProtocol { } } + 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(); /** @@ -118,6 +142,15 @@ public class TSimpleJSONProtocol extends TProtocol { writeContext_ = writeContextStack_.pop(); } + /** + * 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 */ @@ -159,9 +192,10 @@ public class TSimpleJSONProtocol extends TProtocol { 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 class TSimpleJSONProtocol extends TProtocol { } public void writeListBegin(TList list) throws TException { + assertContextIsNotMapKey(LIST); writeContext_.write(); trans_.write(LBRACKET); pushWriteContext(new ListContext()); @@ -183,6 +218,7 @@ public class TSimpleJSONProtocol extends TProtocol { } public void writeSetBegin(TSet set) throws TException { + assertContextIsNotMapKey(SET); writeContext_.write(); trans_.write(LBRACKET); pushWriteContext(new ListContext()); @@ -207,8 +243,12 @@ public class TSimpleJSONProtocol extends TProtocol { } 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 class TSimpleJSONProtocol extends TProtocol { } 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 @@ public class TSimpleJSONProtocol extends TProtocol { 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 00000000..3bb6a9e4 --- /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) { + // + } + } +} -- 2.17.1