diff options
author | HÃ¥kon Hallingstad <hakon@verizonmedia.com> | 2020-04-15 17:03:26 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-04-15 17:03:26 +0200 |
commit | 1eecdac6160ea9f7ed6de5e3fc478bc211561dc2 (patch) | |
tree | 5e9fc0a7b6735521c3d47d4ce9b4ee1a113b23e7 /vespajlib | |
parent | 7c1004ffb82e0512278826d24edf9dbdc91186e1 (diff) | |
parent | 4422d42987bb9aefa92abd4cb4135798479edb6a (diff) |
Merge pull request #12882 from vespa-engine/hakonhall/validate-deserialization-of-flag-data
Validate deserialization of flag data
Diffstat (limited to 'vespajlib')
8 files changed, 177 insertions, 8 deletions
diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json index 5cddc82d05a..d9467a41f78 100644 --- a/vespajlib/abi-spec.json +++ b/vespajlib/abi-spec.json @@ -2863,7 +2863,8 @@ ], "methods": [ "public static java.lang.String encode(java.util.Map)", - "public static java.lang.String escape(java.lang.String)" + "public static java.lang.String escape(java.lang.String)", + "public static boolean equals(java.lang.String, java.lang.String)" ], "fields": [] }, diff --git a/vespajlib/src/main/java/com/yahoo/slime/JsonDecoder.java b/vespajlib/src/main/java/com/yahoo/slime/JsonDecoder.java index f199fefd185..f677ae23a45 100644 --- a/vespajlib/src/main/java/com/yahoo/slime/JsonDecoder.java +++ b/vespajlib/src/main/java/com/yahoo/slime/JsonDecoder.java @@ -3,10 +3,8 @@ package com.yahoo.slime; import com.yahoo.text.Text; import com.yahoo.text.Utf8; -import org.w3c.dom.CharacterData; import java.io.ByteArrayOutputStream; -import java.nio.charset.StandardCharsets; /** * A port of the C++ json decoder intended to be fast. @@ -47,6 +45,17 @@ public class JsonDecoder { return slime; } + /** Decode bytes as a UTF-8 JSON into Slime, or throw {@link JsonParseException} on invalid JSON. */ + public Slime decodeOrThrow(Slime slime, byte[] bytes) { + in = new BufferedInput(bytes); + next(); + decodeValue(slimeInserter.adjust(slime)); + if (in.failed()) { + throw new JsonParseException(in); + } + return slime; + } + private void decodeValue(Inserter inserter) { skipWhiteSpace(); switch (c) { diff --git a/vespajlib/src/main/java/com/yahoo/slime/JsonParseException.java b/vespajlib/src/main/java/com/yahoo/slime/JsonParseException.java new file mode 100644 index 00000000000..6c42f7d38c1 --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/slime/JsonParseException.java @@ -0,0 +1,23 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.slime; + +/** + * @author hakonhall + */ +public class JsonParseException extends RuntimeException { + + private static final long serialVersionUID = 1586949558L; + + private final BufferedInput input; + + JsonParseException(BufferedInput input) { + super(input.getErrorMessage()); + this.input = input; + } + + public byte[] getOffendingBytes() { + // potentially expensive array copy + return input.getOffending(); + } + +} diff --git a/vespajlib/src/main/java/com/yahoo/slime/Slime.java b/vespajlib/src/main/java/com/yahoo/slime/Slime.java index 8357e3035c0..83934e0c206 100644 --- a/vespajlib/src/main/java/com/yahoo/slime/Slime.java +++ b/vespajlib/src/main/java/com/yahoo/slime/Slime.java @@ -159,11 +159,10 @@ public final class Slime { } /** - * Tests whether this is equal to Inspector. + * Tests whether the two Inspectors are equal. * - * Since equality of two Inspectors is subtle, {@link Object#equals(Object)} is not used. + * <p>Since equality of two Inspectors is subtle, {@link Object#equals(Object)} is not used.</p> * - * @param that inspector. * @return true if they are equal. */ public boolean equalTo(Slime that) { diff --git a/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java b/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java index 4d0b3a8ff2e..2ed7331a60c 100644 --- a/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java +++ b/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java @@ -104,6 +104,17 @@ public class SlimeUtils { return jsonToSlime(json.getBytes(StandardCharsets.UTF_8)); } + /** Throws {@link JsonParseException} on invalid JSON. */ + public static Slime jsonToSlimeOrThrow(String json) { + return jsonToSlimeOrThrow(json.getBytes(StandardCharsets.UTF_8)); + } + + public static Slime jsonToSlimeOrThrow(byte[] json) { + Slime slime = new Slime(); + new JsonDecoder().decodeOrThrow(slime, json); + return slime; + } + public static Optional<String> optionalString(Inspector inspector) { return Optional.of(inspector.asString()).filter(s -> !s.isEmpty()); } diff --git a/vespajlib/src/main/java/com/yahoo/text/JSON.java b/vespajlib/src/main/java/com/yahoo/text/JSON.java index cfff16c9aba..11e5be9328e 100644 --- a/vespajlib/src/main/java/com/yahoo/text/JSON.java +++ b/vespajlib/src/main/java/com/yahoo/text/JSON.java @@ -1,10 +1,13 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.text; +import com.yahoo.slime.Slime; +import com.yahoo.slime.SlimeUtils; + import java.util.Map; /** - * Static methods for working with the map textual format which is parsed by {@link MapParser} + * Static methods for working with JSON. * * @author bratseth */ @@ -56,4 +59,19 @@ public final class JSON { return b != null ? b.toString() : s; } + /** + * Test whether two JSON strings are equal, e.g. the order of fields in an object is irrelevant. + * + * <p>When comparing two numbers of the two JSON strings, the result is only guaranteed to be + * correct if (a) both are integers (without fraction and exponent) and each fits in a long, or + * (b) both are non-integers, are syntactically identical, and fits in a double.</p> + * + * @throws RuntimeException on invalid JSON + */ + public static boolean equals(String left, String right) { + Slime leftSlime = SlimeUtils.jsonToSlimeOrThrow(left); + Slime rightSlime = SlimeUtils.jsonToSlimeOrThrow(right); + return leftSlime.equalTo(rightSlime); + } + } diff --git a/vespajlib/src/test/java/com/yahoo/slime/SlimeUtilsTest.java b/vespajlib/src/test/java/com/yahoo/slime/SlimeUtilsTest.java index 8b13ee74aed..237b1575bfb 100644 --- a/vespajlib/src/test/java/com/yahoo/slime/SlimeUtilsTest.java +++ b/vespajlib/src/test/java/com/yahoo/slime/SlimeUtilsTest.java @@ -7,8 +7,10 @@ import org.junit.Test; import java.io.IOException; import static org.hamcrest.core.Is.is; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * @author Ulf Lilleengen @@ -78,4 +80,21 @@ public class SlimeUtilsTest { assertTrue(slime.get().field("bar").valid()); } + @Test + public void test_json_to_slime_or_throw() { + Slime slime = SlimeUtils.jsonToSlimeOrThrow("{\"foo\":\"foobie\",\"bar\":{}}"); + assertThat(slime.get().field("foo").asString(), is("foobie")); + assertTrue(slime.get().field("bar").valid()); + } + + @Test + public void test_invalid_json() { + try { + SlimeUtils.jsonToSlimeOrThrow("foo"); + fail(); + } catch (RuntimeException e) { + assertEquals("Unexpected character 'o'", e.getMessage()); + } + } + } diff --git a/vespajlib/src/test/java/com/yahoo/text/JSONTest.java b/vespajlib/src/test/java/com/yahoo/text/JSONTest.java index 22174761571..fbd0f9d0403 100644 --- a/vespajlib/src/test/java/com/yahoo/text/JSONTest.java +++ b/vespajlib/src/test/java/com/yahoo/text/JSONTest.java @@ -2,11 +2,16 @@ package com.yahoo.text; import org.junit.Test; -import static org.junit.Assert.assertEquals; import java.util.LinkedHashMap; import java.util.Map; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + /** * @author bratseth */ @@ -22,4 +27,88 @@ public class JSONTest { assertEquals("{\"a \\\"key\\\"\":3,\"key2\":\"value\",\"key3\":3.3}", JSON.encode(map)); } + @Test + public void testEquals() { + assertTrue(JSON.equals("{}", "{}")); + + // Whitespace is irrelevant + assertTrue(JSON.equals("{}", "\n{ }")); + + // Order of fields in object is irrelevant + assertTrue(JSON.equals("{\"a\":0, \"c\":1}", "{\"c\":1, \"a\":0}")); + + // Object equality is not using subset + assertFalse(JSON.equals("{\"a\":0}", "{\"a\":0, \"b\":0}")); + assertFalse(JSON.equals("{\"a\":0, \"b\":0}", "{\"a\":0}")); + + // Order of elements of array is significant + assertFalse(JSON.equals("[\"a\",\"b\"]", "[\"b\",\"a\"]")); + + // Verify null-valued fields are not ignored + assertFalse(JSON.equals("{\"a\":null}", "{}")); + + // Current impl uses BigInteger if integer doesn't fit in a long. + assertEquals(9223372036854775807L, Long.MAX_VALUE); + assertTrue(JSON.equals("{\"a\": 9223372036854775807}", "{\"a\": 9223372036854775807}")); + + // double 1.0 and int 1 are different + assertTrue(JSON.equals("{\"a\": 1}", "{\"a\": 1}")); + assertTrue(JSON.equals("{\"a\": 1.0}", "{\"a\": 1.0}")); + assertFalse(JSON.equals("{\"a\": 1.0}", "{\"a\": 1}")); + + // Double-precision on numbers. Constant from Math.E. + assertTrue(JSON.equals("{\"e\": 2.71828182845904}", "{\"e\": 2.71828182845904}")); + + // Double.MAX_VALUE is 1.7976931348623157e+308 + assertTrue(JSON.equals("{\"e\": 1.7976931348623156e+308}", "{\"e\": 1.7976931348623156e+308}")); + + // Justification of above float values + double e1 = 2.7182818284590452354; + double e2 = 2.718281828459045; + double e3 = 2.71828182845904; + assertEquals(e1, Math.E, -1); + assertEquals(e1, e2, -1); + assertNotEquals(e1, e3, -1); + + // Invalid JSON throws RuntimeException + assertRuntimeException(() -> JSON.equals("", "{}")); + assertRuntimeException(() -> JSON.equals("{}", "")); + assertRuntimeException(() -> JSON.equals("{", "{}")); + assertRuntimeException(() -> JSON.equals("{}", "{")); + } + + @Test + public void implementationSpecificEqualsBehavior() { + // Exception thrown if outside a long + assertTrue( JSON.equals("{\"a\": 9223372036854775807}", "{\"a\": 9223372036854775807}")); + assertRuntimeException(() -> JSON.equals("{\"a\": 9223372036854775808}", "{\"a\": 9223372036854775808}")); + + // Infinity if floating point number outside of double, and hence equal + assertTrue(JSON.equals("{\"a\": 2.7976931348623158e+308}", "{\"a\": 2.7976931348623158e+308}")); + + // Ignores extraneous precision + assertTrue(JSON.equals( "{\"e\": 2.7182818284590452354}", + "{\"e\": 2.7182818284590452354}")); + assertTrue(JSON.equals( "{\"e\": 2.7182818284590452354}", + "{\"e\": 2.7182818284590452355}")); + assertFalse(JSON.equals("{\"e\": 2.7182818284590452354}", + "{\"e\": 2.71828182845904}")); + + // Comparing equal but syntactically different numbers + assertFalse(JSON.equals("{\"a\": 1.0}", "{\"a\":1}")); + assertTrue(JSON.equals("{\"a\": 1.0}", "{\"a\":1.00}")); + assertTrue(JSON.equals("{\"a\": 1.0}", "{\"a\":1.0000000000000000000000000000}")); + assertTrue(JSON.equals("{\"a\": 10.0}", "{\"a\":1e1}")); + assertTrue(JSON.equals("{\"a\": 1.2}", "{\"a\":12e-1}")); + } + + private static void assertRuntimeException(Runnable runnable) { + try { + runnable.run(); + fail("Expected RuntimeException to be thrown, but no exception was thrown"); + } catch (RuntimeException e) { + // OK + } + } + } |