diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/AddOperation.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/AddOperation.java index 36fa09a14..a303125b3 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/AddOperation.java +++ b/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).> map(it -> it.getType()).orElse(entityType)); } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PatchOperation.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PatchOperation.java index dcf404e0e..811734faa 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PatchOperation.java +++ b/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 Object evaluateValueFromTarget(Object targetObject, Class entityType) { - return value instanceof LateObjectEvaluator - ? ((LateObjectEvaluator) value).evaluate(spelExpression.getValueType(targetObject)) : value; + verifyPath(entityType); + + return evaluate(spelExpression.getValueType(targetObject)); + } + + protected final Object evaluate(Class 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 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); + } } /** diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/JsonPatchTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/JsonPatchTests.java index 3b178e69a..55d9e2af7 100755 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/JsonPatchTests.java +++ b/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()); diff --git a/spring-data-rest-webmvc/src/test/resources/org/springframework/data/rest/webmvc/json/patch/patch-invalid-path.json b/spring-data-rest-webmvc/src/test/resources/org/springframework/data/rest/webmvc/json/patch/patch-invalid-path.json new file mode 100644 index 000000000..98c32f6e2 --- /dev/null +++ b/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" } +]