From 613cf08f7255056a2a2d185b6e6c0f2c50534ed3 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 26 Feb 2018 21:30:54 +0100 Subject: [PATCH] DATACMNS-1264 - MapDataBinder now rejects improper property expressions. We now make sure that type expressions cannot be used in SpEL expressions handled by MapDataBinder. They're gonna be rejected by considering the property not writable. SpEL expression evaluation errors are now forwarded as NotWritablePropertyException to make sure the failure gets handled as if the property referred to doesn't exist. --- .../data/web/MapDataBinder.java | 11 +++++++- .../data/web/MapDataBinderUnitTests.java | 27 ++++++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/springframework/data/web/MapDataBinder.java b/src/main/java/org/springframework/data/web/MapDataBinder.java index ad479f29a..fa719d770 100644 --- a/src/main/java/org/springframework/data/web/MapDataBinder.java +++ b/src/main/java/org/springframework/data/web/MapDataBinder.java @@ -43,6 +43,8 @@ import org.springframework.expression.AccessException; import org.springframework.expression.EvaluationContext; import org.springframework.expression.Expression; import org.springframework.expression.TypedValue; +import org.springframework.expression.spel.SpelEvaluationException; +import org.springframework.expression.spel.SpelMessage; import org.springframework.expression.spel.SpelParserConfiguration; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.StandardEvaluationContext; @@ -177,6 +179,9 @@ class MapDataBinder extends WebDataBinder { StandardEvaluationContext context = new StandardEvaluationContext(); context.addPropertyAccessor(new PropertyTraversingMapAccessor(type, conversionService)); context.setTypeConverter(new StandardTypeConverter(conversionService)); + context.setTypeLocator(typeName -> { + throw new SpelEvaluationException(SpelMessage.TYPE_NOT_FOUND, typeName); + }); context.setRootObject(map); Expression expression = PARSER.parseExpression(propertyName); @@ -208,7 +213,11 @@ class MapDataBinder extends WebDataBinder { value = conversionService.convert(value, TypeDescriptor.forObject(value), typeDescriptor); } - expression.setValue(context, value); + try { + expression.setValue(context, value); + } catch (SpelEvaluationException o_O) { + throw new NotWritablePropertyException(type, propertyName, "Could not write property!", o_O); + } } private boolean conversionRequired(@Nullable Object source, Class targetType) { diff --git a/src/test/java/org/springframework/data/web/MapDataBinderUnitTests.java b/src/test/java/org/springframework/data/web/MapDataBinderUnitTests.java index 5c264b580..0c373afa4 100755 --- a/src/test/java/org/springframework/data/web/MapDataBinderUnitTests.java +++ b/src/test/java/org/springframework/data/web/MapDataBinderUnitTests.java @@ -28,8 +28,11 @@ import java.util.List; import java.util.Map; import org.junit.Test; +import org.springframework.beans.ConfigurablePropertyAccessor; import org.springframework.beans.MutablePropertyValues; +import org.springframework.beans.NotWritablePropertyException; import org.springframework.beans.PropertyValues; +import org.springframework.expression.spel.SpelEvaluationException; import org.springframework.format.annotation.DateTimeFormat; import org.springframework.format.annotation.DateTimeFormat.ISO; import org.springframework.format.support.DefaultFormattingConversionService; @@ -91,7 +94,29 @@ public class MapDataBinderUnitTests { MutablePropertyValues values = new MutablePropertyValues(); values.add("somethingWeird", "Value"); - assertThat(bind(values)).isEqualTo(Collections. emptyMap()); + assertThat(bind(values)).isEqualTo(Collections.emptyMap()); + } + + @Test // DATACMNS-1264 + public void dropsMapExpressionsForCollectionReferences() { + + ConfigurablePropertyAccessor accessor = new MapDataBinder(Bar.class, new DefaultFormattingConversionService()) + .getPropertyAccessor(); + + assertThatExceptionOfType(NotWritablePropertyException.class) // + .isThrownBy(() -> accessor.setPropertyValue("fooBar['foo']", null)) // + .withCauseInstanceOf(SpelEvaluationException.class); + } + + @Test // DATACMNS-1264 + public void rejectsExpressionContainingTypeExpression() { + + ConfigurablePropertyAccessor accessor = new MapDataBinder(Bar.class, new DefaultFormattingConversionService()) + .getPropertyAccessor(); + + assertThatExceptionOfType(NotWritablePropertyException.class) // + .isThrownBy(() -> accessor.setPropertyValue("fooBar[T(java.lang.String)]", null)) // + .withCauseInstanceOf(SpelEvaluationException.class); } private static Map bind(PropertyValues values) {