From 03fc9e89a00697b36d34bf77f6f2119d90eccba1 Mon Sep 17 00:00:00 2001
From: Andy Clement <aclement@gopivotal.com>
Date: Mon, 27 Oct 2014 11:35:51 -0700
Subject: [PATCH] Fixed isWritable for badly formed SpEL expressions

Before this change isWritable() could return true
for a badly formed expression. That is because the
decision about whether something is writable was made
based on the node type rather than whether the node
represented something that could actually be resolved
to be a real thing. This change ensures a resolution
check is done and isWritable() should only return
true if a subsequent setValue() will succeed.

Issue: SPR-10610
---
 .../spel/ast/PropertyOrFieldReference.java    |  2 +-
 .../expression/spel/SetValueTests.java        | 48 +++++++++++++++++++
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/PropertyOrFieldReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/PropertyOrFieldReference.java
index bfb3ccf2d52..35e0fcd5222 100644
--- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/PropertyOrFieldReference.java
+++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/PropertyOrFieldReference.java
@@ -384,7 +384,7 @@ public class PropertyOrFieldReference extends SpelNodeImpl {
 
 		@Override
 		public boolean isWritable() {
-			return true;
+			return this.ref.isWritableProperty(this.ref.name, this.contextObject, this.evalContext);
 		}
 	}
 
diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SetValueTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SetValueTests.java
index 8ae798be642..5d26439e875 100644
--- a/spring-expression/src/test/java/org/springframework/expression/spel/SetValueTests.java
+++ b/spring-expression/src/test/java/org/springframework/expression/spel/SetValueTests.java
@@ -75,6 +75,54 @@ public class SetValueTests extends AbstractExpressionTests {
 		setValue("arrayContainer.bytes[1]", (byte) 3);
 		setValue("arrayContainer.chars[1]", (char) 3);
 	}
+	
+	@Test
+	public void testIsWritableForInvalidExpressions_SPR10610() {
+		Expression e = null;
+		StandardEvaluationContext lContext = TestScenarioCreator.getTestEvaluationContext();
+		
+		// PROPERTYORFIELDREFERENCE
+		// Non existent field (or property):
+		e = parser.parseExpression("arrayContainer.wibble");
+		assertFalse("Should not be writable!",e.isWritable(lContext));
+
+		e = parser.parseExpression("arrayContainer.wibble.foo");
+		try {
+			assertFalse("Should not be writable!",e.isWritable(lContext));
+			fail("Should have had an error because wibble does not really exist");
+		} catch (SpelEvaluationException see) {
+//			org.springframework.expression.spel.SpelEvaluationException: EL1008E:(pos 15): Property or field 'wibble' cannot be found on object of type 'org.springframework.expression.spel.testresources.ArrayContainer' - maybe not public?
+//					at org.springframework.expression.spel.ast.PropertyOrFieldReference.readProperty(PropertyOrFieldReference.java:225)
+			// success!
+		}
+		
+		// VARIABLE
+		// the variable does not exist (but that is OK, we should be writable)
+		e = parser.parseExpression("#madeup1");
+		assertTrue("Should be writable!",e.isWritable(lContext));
+
+		e = parser.parseExpression("#madeup2.bar"); // compound expression
+		assertFalse("Should not be writable!",e.isWritable(lContext));
+		
+		// INDEXER
+		// non existent indexer (wibble made up)
+		e = parser.parseExpression("arrayContainer.wibble[99]");
+		try {
+			assertFalse("Should not be writable!",e.isWritable(lContext));
+			fail("Should have had an error because wibble does not really exist");
+		} catch (SpelEvaluationException see) {
+			// success!
+		}
+		
+		// non existent indexer (index via a string)
+		e = parser.parseExpression("arrayContainer.ints['abc']");
+		try {
+			assertFalse("Should not be writable!",e.isWritable(lContext));
+			fail("Should have had an error because wibble does not really exist");
+		} catch (SpelEvaluationException see) {
+			// success!
+		}
+	}
 
 	@Test
 	public void testSetArrayElementValueAllPrimitiveTypesErrors() {