From a09d6379e6fe95a99b6e3d13c5e37623c35d8eb7 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 26 Feb 2018 21:43: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 | 35 ++++++++++++++----- .../data/web/MapDataBinderUnitTests.java | 34 +++++++++++++++++- 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/springframework/data/web/MapDataBinder.java b/src/main/java/org/springframework/data/web/MapDataBinder.java index 8c6b973da..fc9d165f2 100644 --- a/src/main/java/org/springframework/data/web/MapDataBinder.java +++ b/src/main/java/org/springframework/data/web/MapDataBinder.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 the original author or authors. + * Copyright 2015-2018 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. @@ -39,8 +39,12 @@ import org.springframework.data.mapping.PropertyReferenceException; import org.springframework.data.util.TypeInformation; import org.springframework.expression.AccessException; import org.springframework.expression.EvaluationContext; +import org.springframework.expression.EvaluationException; import org.springframework.expression.Expression; +import org.springframework.expression.TypeLocator; 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; @@ -104,6 +108,13 @@ class MapDataBinder extends WebDataBinder { private static final SpelExpressionParser PARSER = new SpelExpressionParser( new SpelParserConfiguration(false, true)); + private static final TypeLocator REJECTING_LOCATOR = new TypeLocator() { + + @Override + public Class findType(String typeName) throws EvaluationException { + throw new SpelEvaluationException(SpelMessage.TYPE_NOT_FOUND, typeName); + } + }; private final @NonNull Class type; private final @NonNull Map map; @@ -164,6 +175,7 @@ class MapDataBinder extends WebDataBinder { StandardEvaluationContext context = new StandardEvaluationContext(); context.addPropertyAccessor(new PropertyTraversingMapAccessor(type, conversionService)); context.setTypeConverter(new StandardTypeConverter(conversionService)); + context.setTypeLocator(REJECTING_LOCATOR); context.setRootObject(map); Expression expression = PARSER.parseExpression(propertyName); @@ -176,20 +188,24 @@ class MapDataBinder extends WebDataBinder { if (conversionRequired(value, propertyType.getType())) { - PropertyDescriptor descriptor = BeanUtils - .getPropertyDescriptor(owningType.getType(), leafProperty.getSegment()); + PropertyDescriptor descriptor = BeanUtils.getPropertyDescriptor(owningType.getType(), + leafProperty.getSegment()); MethodParameter methodParameter = new MethodParameter(descriptor.getReadMethod(), -1); TypeDescriptor typeDescriptor = TypeDescriptor.nested(methodParameter, 0); 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(Object source, Class targetType) { - if (targetType.isInstance(source)) { + if (source == null || targetType.isInstance(source)) { return false; } @@ -274,11 +290,12 @@ class MapDataBinder extends WebDataBinder { Class actualPropertyType = path.getType(); - TypeDescriptor valueDescriptor = conversionService.canConvert(String.class, actualPropertyType) ? TypeDescriptor - .valueOf(String.class) : TypeDescriptor.valueOf(HashMap.class); + TypeDescriptor valueDescriptor = conversionService.canConvert(String.class, actualPropertyType) + ? TypeDescriptor.valueOf(String.class) + : TypeDescriptor.valueOf(HashMap.class); - return path.isCollection() ? TypeDescriptor.collection(emptyValue.getClass(), valueDescriptor) : TypeDescriptor - .map(emptyValue.getClass(), TypeDescriptor.valueOf(String.class), valueDescriptor); + return path.isCollection() ? TypeDescriptor.collection(emptyValue.getClass(), valueDescriptor) + : TypeDescriptor.map(emptyValue.getClass(), TypeDescriptor.valueOf(String.class), valueDescriptor); } } diff --git a/src/test/java/org/springframework/data/web/MapDataBinderUnitTests.java b/src/test/java/org/springframework/data/web/MapDataBinderUnitTests.java index 1f775ce26..e09a13853 100644 --- a/src/test/java/org/springframework/data/web/MapDataBinderUnitTests.java +++ b/src/test/java/org/springframework/data/web/MapDataBinderUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2017 the original author or authors. + * Copyright 2015-2018 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. @@ -28,9 +28,15 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import org.hamcrest.Matchers; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; +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; @@ -42,6 +48,8 @@ import org.springframework.format.support.DefaultFormattingConversionService; */ public class MapDataBinderUnitTests { + public @Rule ExpectedException exception = ExpectedException.none(); + @Test // DATACMNS-630 public void honorsFormattingAnnotationOnAccessor() { @@ -95,6 +103,30 @@ public class MapDataBinderUnitTests { assertThat(bind(values), is(Collections. emptyMap())); } + @Test // DATACMNS-1264 + public void dropsMapExpressionsForCollectionReferences() { + + ConfigurablePropertyAccessor accessor = new MapDataBinder(Bar.class, new DefaultFormattingConversionService()) + .getPropertyAccessor(); + + exception.expect(NotWritablePropertyException.class); + exception.expectCause(is(Matchers. instanceOf(SpelEvaluationException.class))); + + accessor.setPropertyValue("fooBar['foo']", null); + } + + @Test // DATACMNS-1264 + public void rejectsExpressionContainingTypeExpression() { + + ConfigurablePropertyAccessor accessor = new MapDataBinder(Bar.class, new DefaultFormattingConversionService()) + .getPropertyAccessor(); + + exception.expect(NotWritablePropertyException.class); + exception.expectCause(is(Matchers. instanceOf(SpelEvaluationException.class))); + + accessor.setPropertyValue("fooBar[T(java.lang.String)]", null); + } + private static Map bind(PropertyValues values) { MapDataBinder binder = new MapDataBinder(Root.class, new DefaultFormattingConversionService());