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 124941117..4238782b0 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,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 segments = new ArrayList(); + 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()); } } 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 aa35ea499..3d06c772d 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,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 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 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 segments = new ArrayList(); + + 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); + } + } } 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 cf899a1e5..9dc15395a 100644 --- 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 @@ -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()); 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" } +]