Skip to content

Commit

Permalink
THRIFT-5474: TSimpleJsonProtocol with inner quotes does not produce v…
Browse files Browse the repository at this point in the history 
…alid JSON.

There is an issue in the generation of JSON from the TSimpleJSONProtocol, where if there are quotes inside of a string, then those quotes will not be serialized correctly. These quotes one more layer of escaping to produce valid JSON. I have included JSON parsing into the test for TSimpleJSONProtocol to ensure that this does not happen if there are changes to this class in the future.
  • Loading branch information
@thealmightygrant
thealmightygrant committed Oct 28, 2021
1 parent 007b99b commit ce40f97
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 13 deletions.
9 changes: 5 additions & 4 deletions lib/java/gradle.properties
View file
Expand Up @@ -27,9 +27,10 @@ maven-repository-id=apache.releases.https
# Dependency versions
httpclient.version=4.5.10
httpcore.version=4.4.12
slf4j.version=1.7.28
servlet.version=4.0.1
tomcat.embed.version=9.0.43
jackson.version=2.12.5
javax.annotation.version=1.3.2
junit.version=4.12
mockito.version=1.10.19
javax.annotation.version=1.3.2
servlet.version=4.0.1
slf4j.version=1.7.28
tomcat.embed.version=9.0.43
11 changes: 7 additions & 4 deletions lib/java/gradle/environment.gradle
View file
Expand Up @@ -44,12 +44,13 @@ ext.mavenRepositoryUrl = property('maven-repository-url')
// Versions used in this project
ext.httpclientVersion = property('httpclient.version')
ext.httpcoreVersion = property('httpcore.version')
ext.servletVersion = property('servlet.version')
ext.tomcatEmbedVersion = property('tomcat.embed.version')
ext.slf4jVersion = property('slf4j.version')
ext.jacksonVersion = property('jackson.version')
ext.javaxAnnotationVersion = property('javax.annotation.version')
ext.junitVersion = property('junit.version')
ext.mockitoVersion = property('mockito.version')
ext.javaxAnnotationVersion = property('javax.annotation.version')
ext.servletVersion = property('servlet.version')
ext.slf4jVersion = property('slf4j.version')
ext.tomcatEmbedVersion = property('tomcat.embed.version')

// In this section you declare where to find the dependencies of your project
repositories {
Expand All @@ -71,6 +72,8 @@ dependencies {
compile "javax.annotation:javax.annotation-api:${javaxAnnotationVersion}"

testCompile "junit:junit:${junitVersion}"
testCompile "com.fasterxml.jackson.core:jackson-core:${jacksonVersion}"
testCompile "com.fasterxml.jackson.core:jackson-databind:${jacksonVersion}"
testCompile "org.mockito:mockito-all:${mockitoVersion}"
testRuntime "org.slf4j:slf4j-log4j12:${slf4jVersion}"
}
1 change: 1 addition & 0 deletions lib/java/src/org/apache/thrift/protocol/TSimpleJSONProtocol.java
View file
Expand Up @@ -314,6 +314,7 @@ public void writeString(String str) throws TException {
char c = str.charAt(i);
switch (c) {
case '"':
escape.append('\\');
case '\\':
escape.append('\\');
escape.append(c);
Expand Down
23 changes: 18 additions & 5 deletions lib/java/test/org/apache/thrift/protocol/TestTSimpleJSONProtocol.java
View file
Expand Up @@ -18,8 +18,10 @@
*/
package org.apache.thrift.protocol;

import java.nio.charset.StandardCharsets;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.core.JsonFactory;

import java.nio.charset.StandardCharsets;
import junit.framework.TestCase;

import org.apache.thrift.Fixtures;
Expand All @@ -32,6 +34,9 @@
import thrift.test.HolyMoley;

public class TestTSimpleJSONProtocol extends TestCase {
private static final ObjectMapper JSON_MAPPER = new ObjectMapper();
private static final JsonFactory JSON_FACTORY = JSON_MAPPER.getFactory();

private TMemoryBuffer buf;
private TSimpleJSONProtocol proto;

Expand All @@ -51,17 +56,23 @@ public void testHolyMoley() throws TException {
holyMoley.unsetBonks();
holyMoley.unsetContain();
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]}]}", bufToString());
final String holyMoleyItsAString = bufToString();
JSON_FACTORY.createParser(holyMoleyItsAString);
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]}]}", holyMoleyItsAString);
}

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());
final String nestingString = bufToString();
JSON_FACTORY.createParser(nestingString);
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]}}", nestingString);
}

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());
final String oneOfEachString = bufToString();
JSON_FACTORY.createParser(oneOfEachString);
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]}", oneOfEachString);
}

public void testSanePartsOfCompactProtoTestStruct() throws TException {
Expand All @@ -82,7 +93,9 @@ public void testSanePartsOfCompactProtoTestStruct() throws TException {
struct.unsetDouble_byte_map();
struct.unsetString_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\":[{},{}],\"i32_set\":[1,2,3],\"boolean_set\":[0,1],\"struct_set\":[{}],\"byte_byte_map\":{\"1\":2},\"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.1},\"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]},\"field500\":500,\"field5000\":5000,\"field20000\":20000}", bufToString());
final String compactString = bufToString();
JSON_FACTORY.createParser(compactString);
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\":[{},{}],\"i32_set\":[1,2,3],\"boolean_set\":[0,1],\"struct_set\":[{}],\"byte_byte_map\":{\"1\":2},\"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.1},\"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]},\"field500\":500,\"field5000\":5000,\"field20000\":20000}", compactString);
}

public void testThrowsOnCollectionKeys() throws TException {
Expand Down

0 comments on commit ce40f97

Please sign in to comment.