DATAREST-1127 - Patch operations now always verify paths before operation application.

Previously, the SpEL expression created from JSON Patch path expressions were executed without double checking whether these paths actually exist on the target object in the first place. This check is now in place.

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/AddOperation.java

@@ -15,11 +15,7 @@
  */
 package org.springframework.data.rest.webmvc.json.patch;
 
-import java.util.ArrayList;
-import java.util.List;
-
 import org.springframework.data.mapping.PropertyPath;
-import org.springframework.util.StringUtils;
 
 /**
  * Operation to add a new value to the given "path". Will throw a {@link PatchException} if the path is invalid or if
@@ -60,17 +56,8 @@ class AddOperation extends PatchOperation {
 			return super.evaluateValueFromTarget(targetObject, entityType);
 		}
 
-		List<String> segments = new ArrayList<String>();
+		PropertyPath path = verifyPath(entityType);
 
-		for (String segment : path.split("/")) {
-			if (!(segment.matches("\\d+") || segment.equals("-") || segment.isEmpty())) {
-				segments.add(segment);
-			}
-		}
-
-		PropertyPath propertyPath = PropertyPath.from(StringUtils.collectionToDelimitedString(segments, "."), entityType);
-
-		return value instanceof LateObjectEvaluator ? ((LateObjectEvaluator) value).evaluate(propertyPath.getType())
-				: value;
+		return evaluate(path == null ? entityType : path.getType());
 	}
 }

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PatchOperation.java

@@ -17,14 +17,18 @@ package org.springframework.data.rest.webmvc.json.patch;
 
 import static org.springframework.data.rest.webmvc.json.patch.PathToSpEL.*;
 
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 
 import org.springframework.core.CollectionFactory;
 import org.springframework.core.convert.TypeDescriptor;
+import org.springframework.data.mapping.PropertyPath;
+import org.springframework.data.mapping.PropertyReferenceException;
 import org.springframework.expression.Expression;
 import org.springframework.expression.ExpressionException;
 import org.springframework.expression.spel.SpelEvaluationException;
+import org.springframework.util.StringUtils;
 
 /**
  * Abstract base class representing and providing support methods for patch operations.
@@ -35,6 +39,8 @@ import org.springframework.expression.spel.SpelEvaluationException;
  */
 public abstract class PatchOperation {
 
+	private static final String INVALID_PATH_REFERENCE = "Invalid path reference %s on type %s (from source %s)!";
+
 	protected final String op;
 	protected final String path;
 	protected final Object value;
@@ -193,8 +199,13 @@ public abstract class PatchOperation {
 	 */
 	protected <T> Object evaluateValueFromTarget(Object targetObject, Class<T> entityType) {
 
-		return value instanceof LateObjectEvaluator
-				? ((LateObjectEvaluator) value).evaluate(spelExpression.getValueType(targetObject)) : value;
+		verifyPath(entityType);
+
+		return evaluate(spelExpression.getValueType(targetObject));
+	}
+
+	protected final <T> Object evaluate(Class<T> targetType) {
+		return value instanceof LateObjectEvaluator ? ((LateObjectEvaluator) value).evaluate(targetType) : value;
 	}
 
 	/**
@@ -219,4 +230,32 @@ public abstract class PatchOperation {
 			return null;
 		}
 	}
+
+	/**
+	 * Verifies, that the path for the current operation is a valid property path on the given target type.
+	 * 
+	 * @param type must not be {@literal null}.
+	 */
+	protected PropertyPath verifyPath(Class<?> type) {
+
+		List<String> segments = new ArrayList<String>();
+
+		for (String segment : path.split("/")) {
+			if (!(segment.matches("\\d+") || segment.equals("-") || segment.isEmpty())) {
+				segments.add(segment);
+			}
+		}
+
+		if (segments.isEmpty()) {
+			return null;
+		}
+
+		String pathSource = StringUtils.collectionToDelimitedString(segments, ".");
+
+		try {
+			return PropertyPath.from(pathSource, type);
+		} catch (PropertyReferenceException o_O) {
+			throw new PatchException(String.format(INVALID_PATH_REFERENCE, pathSource, type, path), o_O);
+		}
+	}
 }

spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/JsonPatchTests.java

@@ -171,6 +171,11 @@ public class JsonPatchTests {
 		patch.apply(todo, Todo.class);
 	}
 
+	@Test(expected = PatchException.class) // DATAREST-1127
+	public void rejectsInvalidPaths() throws Exception {
+		readJsonPatch("patch-invalid-path.json").apply(new Todo(), Todo.class);
+	}
+
 	private Patch readJsonPatch(String jsonPatchFile) throws IOException, JsonParseException, JsonMappingException {
 
 		ClassPathResource resource = new ClassPathResource(jsonPatchFile, getClass());

spring-data-rest-webmvc/src/test/resources/org/springframework/data/rest/webmvc/json/patch/patch-invalid-path.json

@@ -0,0 +1,3 @@
+[
+	{"op":"replace", "path":"/somethingInvalid", "value": "bar" }
+]