From f6b38552864a28e540351b1bddbcbea2998b7b02 Mon Sep 17 00:00:00 2001 From: Swaranga Sarma Date: Wed, 31 Aug 2022 17:02:31 -0700 Subject: [PATCH] Add validation to the input patch during parsing --- .../com/github/fge/jsonpatch/JsonPatch.java | 60 +++++++++++++++++++ .../github/fge/jsonpatch/messages.properties | 1 + .../github/fge/jsonpatch/ValidationTest.java | 58 ++++++++++++++++++ .../resources/jsonpatch/invalid-patches.json | 40 +++++++++++++ 4 files changed, 159 insertions(+) create mode 100644 src/test/java/com/github/fge/jsonpatch/ValidationTest.java create mode 100644 src/test/resources/jsonpatch/invalid-patches.json diff --git a/src/main/java/com/github/fge/jsonpatch/JsonPatch.java b/src/main/java/com/github/fge/jsonpatch/JsonPatch.java index b41ed6bb..1d1cf91d 100644 --- a/src/main/java/com/github/fge/jsonpatch/JsonPatch.java +++ b/src/main/java/com/github/fge/jsonpatch/JsonPatch.java @@ -31,8 +31,11 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; /** * Implementation of JSON Patch @@ -96,6 +99,10 @@ public final class JsonPatch private static final MessageBundle BUNDLE = MessageBundles.getBundle(JsonPatchMessages.class); + // https://datatracker.ietf.org/doc/html/draft-ietf-appsawg-json-patch-10 + private static final Set VALID_OPERATIONS + = new HashSet<>(Arrays.asList("test", "add", "copy", "move", "remove", "replace")); + /** * List of operations */ @@ -127,6 +134,8 @@ public static JsonPatch fromJson(final JsonNode node) throws IOException { BUNDLE.checkNotNull(node, "jsonPatch.nullInput"); + BUNDLE.checkArgumentFormat(isValidJsonPatch(node), "jsonPatch.invalidPatch", node); + return JacksonUtils.getReader().forType(JsonPatch.class) .readValue(node); } @@ -179,4 +188,55 @@ public void serializeWithType(final JsonGenerator jgen, { serialize(jgen, provider); } + + private static boolean isValidJsonPatch(final JsonNode patch) + { + return hasValidOperation(patch) + && hasValidPath(patch) + && hasValidOptionalAttributes(patch); + } + + private static boolean hasValidOptionalAttributes(JsonNode patchElement) + { + // https://datatracker.ietf.org/doc/html/draft-ietf-appsawg-json-patch-10#section-4 + switch(patchElement.get("op").asText()) { + case "test": + case "add": + case "replace": + return hasAttribute("value", patchElement); + + case "move": + case "copy": + return hasStringAttribute("from", patchElement); + + case "remove": + default: + return true; + } + } + + private static boolean hasStringAttribute(String attribute, JsonNode patchElement) + { + return hasAttribute(attribute, patchElement) + && patchElement.get(attribute).isTextual(); + } + + private static boolean hasAttribute(String attribute, JsonNode patchElement) + { + return patchElement.has(attribute) + && !patchElement.get(attribute).isMissingNode(); + } + + private static boolean hasValidPath(JsonNode patchElement) + { + return patchElement.has("path") + && patchElement.get("path").isTextual(); + } + + private static boolean hasValidOperation(JsonNode patchElement) + { + return patchElement.has("op") + && patchElement.get("op").isTextual() + && VALID_OPERATIONS.contains(patchElement.get("op").asText()); + } } diff --git a/src/main/resources/com/github/fge/jsonpatch/messages.properties b/src/main/resources/com/github/fge/jsonpatch/messages.properties index 668240ca..d7ee5e69 100644 --- a/src/main/resources/com/github/fge/jsonpatch/messages.properties +++ b/src/main/resources/com/github/fge/jsonpatch/messages.properties @@ -20,6 +20,7 @@ common.nullArgument=argument cannot be null jsonPatch.deserFailed=unable to deserialize JSON input jsonPatch.nullInput=input cannot be null +jsonPatch.invalidPatch=invalid Json patch {0} jsonPatch.nullValue=value cannot be null jsonPatch.noSuchParent=parent of node to add does not exist jsonPatch.notAnIndex=reference token is not an array index diff --git a/src/test/java/com/github/fge/jsonpatch/ValidationTest.java b/src/test/java/com/github/fge/jsonpatch/ValidationTest.java new file mode 100644 index 00000000..9717c3f5 --- /dev/null +++ b/src/test/java/com/github/fge/jsonpatch/ValidationTest.java @@ -0,0 +1,58 @@ +package com.github.fge.jsonpatch; + +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.Lists; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +import java.io.IOException; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; +import java.util.Iterator; +import java.util.List; +import java.util.Objects; + +import static com.github.fge.jsonpatch.JsonPatchOperation.BUNDLE; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.fail; + +public class ValidationTest { + // the test cases have comments in them to explain the specific test case + // hence use a custom Object mapper that allows Json comments + private static final ObjectMapper MAPPER = new ObjectMapper() + .enable(JsonParser.Feature.ALLOW_COMMENTS); + + @DataProvider + public final Iterator getInvalidJsonPatches() throws IOException + { + final JsonNode patches = MAPPER.readTree( + new InputStreamReader( + Objects.requireNonNull(ValidationTest.class.getResourceAsStream( + "/jsonpatch/invalid-patches.json" + )), + StandardCharsets.UTF_8 + ) + ); + + final List list = Lists.newArrayList(); + + for (final JsonNode patch: patches) { + list.add(new Object[] {patch}); + } + + return list.iterator(); + } + + @Test(dataProvider = "getInvalidJsonPatches") + public final void invalidPatchesAreDetected(final JsonNode patch) throws IOException + { + try { + JsonPatch.fromJson(patch); + fail("No exception thrown!!"); + } catch (IllegalArgumentException ie) { + assertEquals(ie.getMessage(), BUNDLE.format("jsonPatch.invalidPatch", patch)); + } + } +} diff --git a/src/test/resources/jsonpatch/invalid-patches.json b/src/test/resources/jsonpatch/invalid-patches.json new file mode 100644 index 00000000..ead7434b --- /dev/null +++ b/src/test/resources/jsonpatch/invalid-patches.json @@ -0,0 +1,40 @@ +[ + // predicate with no operation + [{"path": "/field1", "value": {"id":123}}], + + // predicate with no path + [{"op": "test", "value": {"id":123}}], + + // predicate with no value + [{"op": "test", "path": "/field1"}], + + // predicate with non-text path attribute + [{"op": "test", "path": 1, "value": {"id":123}}], + + // patch with no operation + [{"path": "/field1/a", "value": "b"}], + + // patch with no path + [{"op": "add", "value": "b"}], + + // patch with non-text path + [{"op": "add", "path": 123, "value": "b"}], + + // ADD patch with no value + [{"op": "add", "path": "/field1/a"}], + + // REPLACE patch with no value + [{"op": "replace", "path": "/field1/a"}], + + // MOVE patch with no from attribute + [{"op": "move", "path": "/field1/a"}], + + // COPY patch with no from attribute + [{"op": "move", "path": "/field1/a"}], + + // MOVE patch with no non-text from attribute + [{"op": "move", "path": "/field1/a", "from": 123}], + + // COPY patch with no non-text from attribute + [{"op": "copy", "path": "/field1/a", "from": 123}] +]