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

Previously the SpEL expresssion 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 is now in place.

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

@@ -15,11 +15,6 @@
  */
 package org.springframework.data.rest.webmvc.json.patch;
 
-import java.util.Arrays;
-import java.util.stream.Collectors;
-
-import org.springframework.data.mapping.PropertyPath;
-
 /**
  * Operation to add a new value to the given "path". Will throw a {@link PatchException} if the path is invalid or if
  * the given value is not assignable to the given path.
@@ -59,15 +54,6 @@ class AddOperation extends PatchOperation {
 			return super.evaluateValueFromTarget(targetObject, entityType);
 		}
 
-		String pathSource = Arrays.stream(path.split("/"))//
-				.filter(it -> !it.matches("\\d")) // no digits
-				.filter(it -> !it.equals("-")) // no "last element"s
-				.filter(it -> !it.isEmpty()) //
-				.collect(Collectors.joining("."));
-
-		PropertyPath propertyPath = PropertyPath.from(pathSource, entityType);
-
-		return value instanceof LateObjectEvaluator ? ((LateObjectEvaluator) value).evaluate(propertyPath.getType())
-				: value;
+		return evaluate(verifyPath(entityType).<Class<?>> map(it -> it.getType()).orElse(entityType));
 	}
 }

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

@@ -17,11 +17,16 @@ package org.springframework.data.rest.webmvc.json.patch;
 
 import static org.springframework.data.rest.webmvc.json.patch.PathToSpEL.*;
 
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
 
 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;
@@ -35,6 +40,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;
@@ -191,8 +198,39 @@ 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> type) {
+		return value instanceof LateObjectEvaluator ? ((LateObjectEvaluator) value).evaluate(type) : value;
+	}
+
+	/**
+	 * Verifies that the current path is available on the given type.
+	 * 
+	 * @param type must not be {@literal null}.
+	 * @return the {@link PropertyPath} representing the path. Empty if the path only consists of index lookups or append
+	 *         characters.
+	 */
+	protected final Optional<PropertyPath> verifyPath(Class<?> type) {
+
+		String pathSource = Arrays.stream(path.split("/"))//
+				.filter(it -> !it.matches("\\d")) // no digits
+				.filter(it -> !it.equals("-")) // no "last element"s
+				.filter(it -> !it.isEmpty()) //
+				.collect(Collectors.joining("."));
+
+		if (pathSource.isEmpty()) {
+			return Optional.empty();
+		}
+
+		try {
+			return Optional.of(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

@@ -162,6 +162,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" }
+]