From d330455e534b743bc8f19fd110500a42f625d8ce Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 25 Sep 2017 18:57:15 +0200 Subject: [PATCH] DATAREST-1137 - All patch operations now verify path expressions. We now make sure that all patch operations now get the path they're supposed to be applied to verified before execution. --- .../rest/webmvc/json/patch/AddOperation.java | 6 +-- .../rest/webmvc/json/patch/CopyOperation.java | 6 +-- .../rest/webmvc/json/patch/MoveOperation.java | 6 +-- .../webmvc/json/patch/PatchOperation.java | 22 ++++++-- .../webmvc/json/patch/RemoveOperation.java | 8 +-- .../webmvc/json/patch/ReplaceOperation.java | 9 ++-- .../rest/webmvc/json/patch/TestOperation.java | 9 ++-- .../json/patch/PatchOperationUnitTests.java | 53 +++++++++++++++++++ 8 files changed, 94 insertions(+), 25 deletions(-) create mode 100644 spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/PatchOperationUnitTests.java 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 4238782b0..0dfdb968b 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 @@ -1,5 +1,5 @@ /* - * Copyright 2014-2016 the original author or authors. + * Copyright 2014-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -38,10 +38,10 @@ class AddOperation extends PatchOperation { /* * (non-Javadoc) - * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class) + * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#doPerform(java.lang.Object, java.lang.Class) */ @Override - void perform(Object targetObject, Class type) { + void doPerform(Object targetObject, Class type) { addValue(targetObject, evaluateValueFromTarget(targetObject, type)); } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/CopyOperation.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/CopyOperation.java index ee8f84817..f432e59ae 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/CopyOperation.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/CopyOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2016 the original author or authors. + * Copyright 2014-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -53,10 +53,10 @@ class CopyOperation extends FromOperation { /* * (non-Javadoc) - * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class) + * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#doPerform(java.lang.Object, java.lang.Class) */ @Override - void perform(Object target, Class type) { + void doPerform(Object target, Class type) { addValue(target, pathToExpression(getFrom()).getValue(target)); } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/MoveOperation.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/MoveOperation.java index ef1869c4a..3359fd3a4 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/MoveOperation.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/MoveOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2016 the original author or authors. + * Copyright 2014-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -44,10 +44,10 @@ class MoveOperation extends FromOperation { /* * (non-Javadoc) - * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class) + * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#doPerform(java.lang.Object, java.lang.Class) */ @Override - void perform(Object target, Class type) { + void doPerform(Object target, Class type) { addValue(target, popValueAtPath(target, getFrom())); } } 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 3d06c772d..8836860b6 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 @@ -209,11 +209,25 @@ public abstract class PatchOperation { } /** - * Perform the operation. + * Perform the operation in the given target object. * - * @param target the target of the operation. + * @param target the target of the operation, must not be {@literal null}. + * @param type must not be {@literal null}. */ - abstract void perform(Object target, Class type); + final void perform(Object target, Class type) { + + verifyPath(type); + + doPerform(target, type); + } + + /** + * Implements the actually application of the operation. + * + * @param target must not be {@literal null}. + * @param type must not be {@literal null}. + */ + abstract void doPerform(Object target, Class type); private Integer targetListIndex(String path) { @@ -241,7 +255,7 @@ public abstract class PatchOperation { List segments = new ArrayList(); for (String segment : path.split("/")) { - if (!(segment.matches("\\d+") || segment.equals("-") || segment.isEmpty())) { + if (!(segment.matches("\\d+") || segment.equals("-") || segment.equals("~") || segment.isEmpty())) { segments.add(segment); } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/RemoveOperation.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/RemoveOperation.java index 51f638ad3..90a3bf98a 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/RemoveOperation.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/RemoveOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2016 the original author or authors. + * Copyright 2014-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,7 +22,7 @@ package org.springframework.data.rest.webmvc.json.patch; * @author Craig Walls * @author Oliver Gierke */ -public class RemoveOperation extends PatchOperation { +class RemoveOperation extends PatchOperation { /** * Constructs the remove operation @@ -35,10 +35,10 @@ public class RemoveOperation extends PatchOperation { /* * (non-Javadoc) - * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class) + * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#doPerform(java.lang.Object, java.lang.Class) */ @Override - void perform(Object target, Class type) { + void doPerform(Object target, Class type) { popValueAtPath(target, path); } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/ReplaceOperation.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/ReplaceOperation.java index 6629f4c1b..fcfbfac03 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/ReplaceOperation.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/ReplaceOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2014 the original author or authors. + * Copyright 2014-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,8 +19,9 @@ package org.springframework.data.rest.webmvc.json.patch; * Operation that replaces the value at the given path with a new value. * * @author Craig Walls + * @author Oliver Gierke */ -public class ReplaceOperation extends PatchOperation { +class ReplaceOperation extends PatchOperation { /** * Constructs the replace operation @@ -34,10 +35,10 @@ public class ReplaceOperation extends PatchOperation { /* * (non-Javadoc) - * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class) + * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#doPerform(java.lang.Object, java.lang.Class) */ @Override - void perform(Object target, Class type) { + void doPerform(Object target, Class type) { setValueOnTarget(target, evaluateValueFromTarget(target, type)); } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/TestOperation.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/TestOperation.java index 9c2681225..1d2ad3acc 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/TestOperation.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/TestOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2016 the original author or authors. + * Copyright 2014-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -30,6 +30,7 @@ import org.springframework.util.ObjectUtils; *

* * @author Craig Walls + * @author Oliver Gierke */ class TestOperation extends PatchOperation { @@ -45,10 +46,10 @@ class TestOperation extends PatchOperation { /* * (non-Javadoc) - * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class) + * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#doPerform(java.lang.Object, java.lang.Class) */ @Override - void perform(Object target, Class type) { + void doPerform(Object target, Class type) { Object expected = normalizeIfNumber(evaluateValueFromTarget(target, type)); Object actual = normalizeIfNumber(getValueFromTarget(target)); @@ -58,7 +59,7 @@ class TestOperation extends PatchOperation { } } - private Object normalizeIfNumber(Object expected) { + private static Object normalizeIfNumber(Object expected) { if (expected instanceof Double || expected instanceof Float) { expected = BigDecimal.valueOf(((Number) expected).doubleValue()); diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/PatchOperationUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/PatchOperationUnitTests.java new file mode 100644 index 000000000..ed89dc6ce --- /dev/null +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/PatchOperationUnitTests.java @@ -0,0 +1,53 @@ +/* + * Copyright 2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.rest.webmvc.json.patch; + +import org.junit.Assert; +import org.junit.Test; + +/** + * General unit tests for {@link PatchOperation} implementations. + * + * @author Oliver Gierke + */ +public class PatchOperationUnitTests { + + @Test // DATAREST-1137 + public void invalidPathGetsRejected() { + + String invalidPath = "/nonExistant"; + + verifyIllegalPath(new AddOperation(invalidPath, null)); + verifyIllegalPath(new CopyOperation(invalidPath, null)); + verifyIllegalPath(new MoveOperation(invalidPath, null)); + verifyIllegalPath(new RemoveOperation(invalidPath)); + verifyIllegalPath(new ReplaceOperation(invalidPath, null)); + verifyIllegalPath(new TestOperation(invalidPath, null)); + } + + private static void verifyIllegalPath(PatchOperation operation) { + + try { + + Todo todo = new Todo(1L, "A", false); + operation.perform(todo, Todo.class); + + Assert.fail("Expected PatchException!"); + + } catch (PatchException o_O) {} + + } +}