From 9c832a33a96be5999341abdf3483fa9bc1fe0316 Mon Sep 17 00:00:00 2001 From: Juerg Wullschleger Date: Thu, 15 Sep 2022 02:08:41 -0700 Subject: [PATCH] Rename StrictJsonParser to JsonParser. There will only be one Json Parser, so it is better to just call it "JsonParser". Also, remove two TODOs. And remove the test case where expected is "null". This is testsed in testNumbersToDouble. PiperOrigin-RevId: 474507082 Change-Id: I848c2f864edc4f237710d007bdf122c4574af106 --- BUILD.bazel | 4 +- .../google/crypto/tink/internal/BUILD.bazel | 8 +- ...{StrictJsonParser.java => JsonParser.java} | 13 +- .../google/crypto/tink/internal/BUILD.bazel | 6 +- ...sonParserTest.java => JsonParserTest.java} | 133 +++++++----------- 5 files changed, 71 insertions(+), 93 deletions(-) rename src/main/java/com/google/crypto/tink/internal/{StrictJsonParser.java => JsonParser.java} (93%) rename src/test/java/com/google/crypto/tink/internal/{StrictJsonParserTest.java => JsonParserTest.java} (83%) diff --git a/BUILD.bazel b/BUILD.bazel index 906bb50ef..0d15d8a7d 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -138,6 +138,7 @@ gen_maven_jar_rules( "//src/main/java/com/google/crypto/tink/hybrid/subtle:rsa_kem_hybrid_decrypt", "//src/main/java/com/google/crypto/tink/hybrid/subtle:rsa_kem_hybrid_encrypt", "//src/main/java/com/google/crypto/tink/internal:build_dispatched_code", + "//src/main/java/com/google/crypto/tink/internal:json_parser", "//src/main/java/com/google/crypto/tink/internal:key_parser", "//src/main/java/com/google/crypto/tink/internal:key_serializer", "//src/main/java/com/google/crypto/tink/internal:key_status_type_proto_converter", @@ -156,7 +157,6 @@ gen_maven_jar_rules( "//src/main/java/com/google/crypto/tink/internal:proto_parameters_serialization", "//src/main/java/com/google/crypto/tink/internal:serialization", "//src/main/java/com/google/crypto/tink/internal:serialization_registry", - "//src/main/java/com/google/crypto/tink/internal:strict_json_parser", "//src/main/java/com/google/crypto/tink/internal:tink_bug_exception", "//src/main/java/com/google/crypto/tink/internal:util", "//src/main/java/com/google/crypto/tink/jwt:json_util", @@ -443,6 +443,7 @@ gen_maven_jar_rules( "//src/main/java/com/google/crypto/tink/integration/android:android_keystore_kms_client", "//src/main/java/com/google/crypto/tink/integration/android:shared_pref_keyset_reader", "//src/main/java/com/google/crypto/tink/integration/android:shared_pref_keyset_writer", + "//src/main/java/com/google/crypto/tink/internal:json_parser-android", "//src/main/java/com/google/crypto/tink/internal:key_parser-android", "//src/main/java/com/google/crypto/tink/internal:key_serializer-android", "//src/main/java/com/google/crypto/tink/internal:key_status_type_proto_converter-android", @@ -461,7 +462,6 @@ gen_maven_jar_rules( "//src/main/java/com/google/crypto/tink/internal:proto_parameters_serialization-android", "//src/main/java/com/google/crypto/tink/internal:serialization-android", "//src/main/java/com/google/crypto/tink/internal:serialization_registry-android", - "//src/main/java/com/google/crypto/tink/internal:strict_json_parser-android", "//src/main/java/com/google/crypto/tink/internal:tink_bug_exception-android", "//src/main/java/com/google/crypto/tink/internal:util-android", "//src/main/java/com/google/crypto/tink/jwt:json_util-android", diff --git a/src/main/java/com/google/crypto/tink/internal/BUILD.bazel b/src/main/java/com/google/crypto/tink/internal/BUILD.bazel index a2c20d02a..d95ff6195 100644 --- a/src/main/java/com/google/crypto/tink/internal/BUILD.bazel +++ b/src/main/java/com/google/crypto/tink/internal/BUILD.bazel @@ -498,8 +498,8 @@ java_library( ) java_library( - name = "strict_json_parser", - srcs = ["StrictJsonParser.java"], + name = "json_parser", + srcs = ["JsonParser.java"], deps = [ "@maven//:com_google_code_findbugs_jsr305", "@maven//:com_google_code_gson_gson", @@ -507,8 +507,8 @@ java_library( ) android_library( - name = "strict_json_parser-android", - srcs = ["StrictJsonParser.java"], + name = "json_parser-android", + srcs = ["JsonParser.java"], deps = [ "@maven//:com_google_code_findbugs_jsr305", "@maven//:com_google_code_gson_gson", diff --git a/src/main/java/com/google/crypto/tink/internal/StrictJsonParser.java b/src/main/java/com/google/crypto/tink/internal/JsonParser.java similarity index 93% rename from src/main/java/com/google/crypto/tink/internal/StrictJsonParser.java rename to src/main/java/com/google/crypto/tink/internal/JsonParser.java index d808bbcca..ca33e42fb 100644 --- a/src/main/java/com/google/crypto/tink/internal/StrictJsonParser.java +++ b/src/main/java/com/google/crypto/tink/internal/JsonParser.java @@ -33,12 +33,15 @@ import javax.annotation.Nullable; /** - * Implementation of a Strict JSON Parser. + * A JSON Parser based on the GSON JsonReader. * - *

The parsing is almost identical to TypeAdapters.JSON_ELEMENT, but it rejects duplicated map - * keys and strings with invalid characters. + *

The parsing is almost identical to the normal parser provided by GSON with these changes: it + * never uses "lenient" mode, it rejects duplicated map keys and it rejects strings with invalid + * UTF16 characters. + * + *

The implementation is adapted from almost identical to GSON's TypeAdapters.JSON_ELEMENT. */ -public final class StrictJsonParser { +public final class JsonParser { private static boolean isValidString(String s) { int length = s.length(); @@ -200,5 +203,5 @@ public static long getParsedNumberAsLongOrThrow(JsonElement element) { return Long.parseLong(element.getAsNumber().toString()); } - private StrictJsonParser() {} + private JsonParser() {} } diff --git a/src/test/java/com/google/crypto/tink/internal/BUILD.bazel b/src/test/java/com/google/crypto/tink/internal/BUILD.bazel index 2351a2838..b8c8ec8c9 100644 --- a/src/test/java/com/google/crypto/tink/internal/BUILD.bazel +++ b/src/test/java/com/google/crypto/tink/internal/BUILD.bazel @@ -286,11 +286,11 @@ java_test( ) java_test( - name = "StrictJsonParserTest", + name = "JsonParserTest", size = "small", - srcs = ["StrictJsonParserTest.java"], + srcs = ["JsonParserTest.java"], deps = [ - "//src/main/java/com/google/crypto/tink/internal:strict_json_parser", + "//src/main/java/com/google/crypto/tink/internal:json_parser", "@maven//:com_google_code_gson_gson", "@maven//:com_google_truth_truth", "@maven//:junit_junit", diff --git a/src/test/java/com/google/crypto/tink/internal/StrictJsonParserTest.java b/src/test/java/com/google/crypto/tink/internal/JsonParserTest.java similarity index 83% rename from src/test/java/com/google/crypto/tink/internal/StrictJsonParserTest.java rename to src/test/java/com/google/crypto/tink/internal/JsonParserTest.java index 94b683e88..821a82ef6 100644 --- a/src/test/java/com/google/crypto/tink/internal/StrictJsonParserTest.java +++ b/src/test/java/com/google/crypto/tink/internal/JsonParserTest.java @@ -37,15 +37,15 @@ import org.junit.runner.RunWith; /** - * Tests that {@link StrictJsonParser}. + * Tests that {@link JsonParser}. * *

We currently test it together with {@link com.google.gson.internal.Streams#parse(JsonReader)}, * to show where they do the same and where they don't. */ @RunWith(Theories.class) -public final class StrictJsonParserTest { +public final class JsonParserTest { - // TODO(b/241828611) Remove this once we fully migrated to StrictJsonParser.parse. + // TODO(b/241828611) Remove this once we fully migrated to JsonParser.parse. // Streams.parse sometimes throws an IOException and sometimes an JsonParseException. private JsonElement normalParse(String input) throws IOException { try { @@ -58,7 +58,6 @@ private JsonElement normalParse(String input) throws IOException { } public static final class TestCase { - // TODO(juerg): Make expected not a string but a JsonElement. public final String name; public final String input; public final JsonElement expected; @@ -215,17 +214,6 @@ public static JsonObject jsonObject(String name, JsonElement value) { "-huge_number", "-999999999999999999999999999999999999999999999999999999999999999999999999999999999999999", new JsonPrimitive(-1e87)), - - // For these two numbers, we couldn't find non-parsed numbers that are equal to them. - new TestCase( - "huge_exp", - "99999999999999999999999999.99e+99999999999999999999999999", - null), - new TestCase( - "-huge_exp", - "-99999999999999999999999999.99e-99999999999999999999999999", - null), - new TestCase("string_with_tailing_comma", "\"a\",", new JsonPrimitive("a")), new TestCase("number_with_tailing_comma", "42,", new JsonPrimitive(42)), new TestCase("true_with_tailing_comma", "true,", new JsonPrimitive(true)), @@ -256,16 +244,12 @@ public static JsonObject jsonObject(String name, JsonElement value) { @Theory public void testBothParsers_success( @FromDataPoints("testCasesSuccess") TestCase testCase) throws Exception { - JsonElement strictOutput = StrictJsonParser.parse(testCase.input); + JsonElement output = JsonParser.parse(testCase.input); - // TODO(juerg): Remove this once invalid UTF16 strings are rejected. - if (testCase.expected != null) { - assertThat(strictOutput).isEqualTo(testCase.expected); - } + assertThat(output).isEqualTo(testCase.expected); // compare to normalParse - JsonElement normalOutput = normalParse(testCase.input); - assertThat(strictOutput).isEqualTo(normalOutput); + assertThat(output).isEqualTo(normalParse(testCase.input)); } @Theory @@ -277,7 +261,7 @@ public void parsedNumberGetAsLong_discardsAllBut64LowestOrderBits() throws Excep // conversion" of the Java Language Specification section 5.1.3, which means that all but the 32 // lowest order bits are discarded. - JsonElement numElement = StrictJsonParser.parse("9223372036854775809"); // 2^63 + 1 + JsonElement numElement = JsonParser.parse("9223372036854775809"); // 2^63 + 1 assertThat(numElement.getAsLong()).isEqualTo(-9223372036854775807L); } @@ -290,7 +274,7 @@ public void parsedNumberGetAsInt_discardsAllBut32LowestOrderBits() throws Except // conversion" of the Java Language Specification section 5.1.3, which means that all but the 32 // lowest order bits are discarded. - JsonElement numElement = StrictJsonParser.parse("2147483649"); // 2^31 + 1 + JsonElement numElement = JsonParser.parse("2147483649"); // 2^31 + 1 assertThat(numElement.getAsInt()).isEqualTo(-2147483647); } @@ -320,52 +304,52 @@ public void parsedNumberGetAsInt_discardsAllBut32LowestOrderBits() throws Except @Theory public void parsedNumberGetAsLong_validLong_sameAsParseLong( @FromDataPoints("longs") String numString) throws Exception { - JsonElement parsed = StrictJsonParser.parse(numString); + JsonElement parsed = JsonParser.parse(numString); assertThat(parsed.getAsLong()).isEqualTo(Long.parseLong(numString)); } @Theory public void parsedNumberGetAsLong_biggerThanLong_sameAsBigIntegerLongValue( @FromDataPoints("biggerThanLongs") String numString) throws Exception { - JsonElement parsed = StrictJsonParser.parse(numString); + JsonElement parsed = JsonParser.parse(numString); assertThat(parsed.getAsLong()).isEqualTo(new BigInteger(numString).longValue()); } @Theory public void parsedNumberGetAsInt_validLong_sameAsBigIntegerIntValue( @FromDataPoints("longs") String numString) throws Exception { - JsonElement parsed = StrictJsonParser.parse(numString); + JsonElement parsed = JsonParser.parse(numString); assertThat(parsed.getAsInt()).isEqualTo(new BigInteger(numString).intValue()); } @Theory public void parsedNumberGetAsInt_biggerThanLong_sameAsBigIntegerIntValue( @FromDataPoints("biggerThanLongs") String numString) throws Exception { - JsonElement parsed = StrictJsonParser.parse(numString); + JsonElement parsed = JsonParser.parse(numString); assertThat(parsed.getAsInt()).isEqualTo(new BigInteger(numString).intValue()); } @Theory public void getParsedNumberAsLongOrThrow_validLong_sameAsParseLong( @FromDataPoints("longs") String numString) throws Exception { - JsonElement parsed = StrictJsonParser.parse(numString); - assertThat(StrictJsonParser.getParsedNumberAsLongOrThrow(parsed)) + JsonElement parsed = JsonParser.parse(numString); + assertThat(JsonParser.getParsedNumberAsLongOrThrow(parsed)) .isEqualTo(Long.parseLong(numString)); } @Theory public void getParsedNumberAsLongOrThrow_biggerThanLong_throws( @FromDataPoints("biggerThanLongs") String numString) throws Exception { - JsonElement parsed = StrictJsonParser.parse(numString); + JsonElement parsed = JsonParser.parse(numString); assertThrows( - NumberFormatException.class, () -> StrictJsonParser.getParsedNumberAsLongOrThrow(parsed)); + NumberFormatException.class, () -> JsonParser.getParsedNumberAsLongOrThrow(parsed)); } @Theory public void getParsedNumberAsLongOrThrow_nestedValue_success() throws Exception { - JsonElement parsed = StrictJsonParser.parse("{\"a\":{\"b\":9223372036854775807}}"); + JsonElement parsed = JsonParser.parse("{\"a\":{\"b\":9223372036854775807}}"); JsonElement parsedNumber = parsed.getAsJsonObject().get("a").getAsJsonObject().get("b"); - long output = StrictJsonParser.getParsedNumberAsLongOrThrow(parsedNumber); + long output = JsonParser.getParsedNumberAsLongOrThrow(parsedNumber); assertThat(output).isEqualTo(9223372036854775807L); } @@ -374,89 +358,80 @@ public void getParsedNumberAsLongOrThrow_notParsed_throws() throws Exception { JsonElement notParsedJsonElementWithNumber = new JsonPrimitive(42); assertThrows( IllegalArgumentException.class, - () -> StrictJsonParser.getParsedNumberAsLongOrThrow(notParsedJsonElementWithNumber)); + () -> JsonParser.getParsedNumberAsLongOrThrow(notParsedJsonElementWithNumber)); } @Theory public void floatNumbersGetAsLong_getsTruncated() throws Exception { - assertThat(StrictJsonParser.parse("42.0").getAsLong()).isEqualTo(42); - assertThat(StrictJsonParser.parse("2.1e1").getAsLong()).isEqualTo(21); + assertThat(JsonParser.parse("42.0").getAsLong()).isEqualTo(42); + assertThat(JsonParser.parse("2.1e1").getAsLong()).isEqualTo(21); - assertThat(StrictJsonParser.parse("42.1").getAsLong()).isEqualTo(42); - assertThat(StrictJsonParser.parse("42.9999").getAsLong()).isEqualTo(42); + assertThat(JsonParser.parse("42.1").getAsLong()).isEqualTo(42); + assertThat(JsonParser.parse("42.9999").getAsLong()).isEqualTo(42); // 2^63 - 1 as float - assertThat(StrictJsonParser.parse("9.223372036854775807e18").getAsLong() - ).isEqualTo(9223372036854775807L); + assertThat(JsonParser.parse("9.223372036854775807e18").getAsLong()) + .isEqualTo(9223372036854775807L); // - 2^63 as float - assertThat(StrictJsonParser.parse("-9.223372036854775808e18").getAsLong() - ).isEqualTo(-9223372036854775808L); + assertThat(JsonParser.parse("-9.223372036854775808e18").getAsLong()) + .isEqualTo(-9223372036854775808L); } @Theory public void floatNumbersGetAsInt_getsTruncated() throws Exception { - assertThat(StrictJsonParser.parse("42.0").getAsInt()).isEqualTo(42); - assertThat(StrictJsonParser.parse("2.1e1").getAsInt()).isEqualTo(21); + assertThat(JsonParser.parse("42.0").getAsInt()).isEqualTo(42); + assertThat(JsonParser.parse("2.1e1").getAsInt()).isEqualTo(21); - assertThat(StrictJsonParser.parse("42.1").getAsInt()).isEqualTo(42); - assertThat(StrictJsonParser.parse("42.9999").getAsInt()).isEqualTo(42); + assertThat(JsonParser.parse("42.1").getAsInt()).isEqualTo(42); + assertThat(JsonParser.parse("42.9999").getAsInt()).isEqualTo(42); // 2^31 - 1 as float - assertThat(StrictJsonParser.parse("2.147483647e9").getAsInt()) - .isEqualTo(2147483647); + assertThat(JsonParser.parse("2.147483647e9").getAsInt()).isEqualTo(2147483647); // - 2^31 as float - assertThat(StrictJsonParser.parse("-2.147483648e9").getAsInt()) - .isEqualTo(-2147483648); + assertThat(JsonParser.parse("-2.147483648e9").getAsInt()).isEqualTo(-2147483648); } @Theory public void testNumbersToDouble() throws Exception { - assertThat(StrictJsonParser.parse("60911552482230981.0").getAsDouble() - ).isEqualTo(6.0911552482230984e16); - assertThat(StrictJsonParser.parse("4e+42").getAsDouble() - ).isEqualTo(4e42); - assertThat(StrictJsonParser.parse("4e42").getAsDouble() - ).isEqualTo(4e42); - assertThat(StrictJsonParser.parse("4E42").getAsDouble() - ).isEqualTo(4e42); - assertThat(StrictJsonParser.parse("-4e-42").getAsDouble() - ).isEqualTo(-4e-42); + assertThat(JsonParser.parse("60911552482230981.0").getAsDouble()) + .isEqualTo(6.0911552482230984e16); + assertThat(JsonParser.parse("4e+42").getAsDouble()).isEqualTo(4e42); + assertThat(JsonParser.parse("4e42").getAsDouble()).isEqualTo(4e42); + assertThat(JsonParser.parse("4E42").getAsDouble()).isEqualTo(4e42); + assertThat(JsonParser.parse("-4e-42").getAsDouble()).isEqualTo(-4e-42); assertThat( - StrictJsonParser.parse( + JsonParser.parse( "9999999999999999999999999999999999999999999999999999999999999999999999999999" + "99999999999") .getAsDouble()) .isEqualTo(1.0e87); assertThat( - StrictJsonParser.parse( + JsonParser.parse( "-999999999999999999999999999999999999999999999999999999999999999999999999999" + "999999999999") .getAsDouble()) .isEqualTo(-1.0e87); assertThat( - StrictJsonParser.parse("99999999999999999999999999.99e+99999999999999999999999999") + JsonParser.parse("99999999999999999999999999.99e+99999999999999999999999999") .getAsDouble()) .isPositiveInfinity(); assertThat( - StrictJsonParser.parse("-99999999999999999999999999.99e+99999999999999999999999999") + JsonParser.parse("-99999999999999999999999999.99e+99999999999999999999999999") .getAsDouble()) .isNegativeInfinity(); assertThat( - StrictJsonParser.parse("99999999999999999999999999.99e-99999999999999999999999999") + JsonParser.parse("99999999999999999999999999.99e-99999999999999999999999999") .getAsDouble()) .isEqualTo(0.0); - assertThat(StrictJsonParser.parse("0.000000000000000000000000000000001").getAsDouble() - ).isEqualTo(0.000000000000000000000000000000001); - assertThat(StrictJsonParser.parse("-0.000000000000000000000000000000001").getAsDouble() - ).isEqualTo(-0.000000000000000000000000000000001); - assertThat(StrictJsonParser.parse("42").getAsInt() - ).isEqualTo(42); - assertThat(StrictJsonParser.parse("42\n").getAsInt() - ).isEqualTo(42); - assertThat(StrictJsonParser.parse("42\f").getAsInt() - ).isEqualTo(42); + assertThat(JsonParser.parse("0.000000000000000000000000000000001").getAsDouble()) + .isEqualTo(0.000000000000000000000000000000001); + assertThat(JsonParser.parse("-0.000000000000000000000000000000001").getAsDouble()) + .isEqualTo(-0.000000000000000000000000000000001); + assertThat(JsonParser.parse("42").getAsInt()).isEqualTo(42); + assertThat(JsonParser.parse("42\n").getAsInt()).isEqualTo(42); + assertThat(JsonParser.parse("42\f").getAsInt()).isEqualTo(42); } @DataPoints("testCasesFail") @@ -576,7 +551,7 @@ public void testNumbersToDouble() throws Exception { @Theory public void testBothParsersFail( @FromDataPoints("testCasesFail") TestCase testCase) throws Exception { - assertThrows(IOException.class, () -> StrictJsonParser.parse(testCase.input)); + assertThrows(IOException.class, () -> JsonParser.parse(testCase.input)); assertThrows(IOException.class, () -> normalParse(testCase.input)); } @@ -609,8 +584,8 @@ public void testBothParsersFail( @Theory public void testStrictFailsButNormalDoesNotFail( @FromDataPoints("stricterTestCases") TestCase testCase) throws Exception { - // StrictJsonParser.parse fails. - assertThrows(IOException.class, () -> StrictJsonParser.parse(testCase.input)); + // JsonParser.parse fails. + assertThrows(IOException.class, () -> JsonParser.parse(testCase.input)); // normalParse parses successfully. JsonElement unused = normalParse(testCase.input);