Add validation to the input patch during parsing

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<String> 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());
+    }
 }

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

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<Object[]> getInvalidJsonPatches() throws IOException
+    {
+        final JsonNode patches = MAPPER.readTree(
+            new InputStreamReader(
+                Objects.requireNonNull(ValidationTest.class.getResourceAsStream(
+                    "/jsonpatch/invalid-patches.json"
+                )),
+                StandardCharsets.UTF_8
+            )
+        );
+
+        final List<Object[]> 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));
+        }
+    }
+}

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}]
+]