Fix VerifyError for SpEL ternary compilation

The ternary expression node was failing to generate the
necessary unboxing bytecode when the condition part
of the expression returned a boxed Boolean rather than
a primitive boolean.

Also fixed here is an IllegalAccessError that was
seen in the same expression due to generating a
CHECKCAST bytecode for a private type.

Issue: SPR-12271

Author: aclement

spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java

@@ -446,8 +446,10 @@ public static void insertCheckCast(MethodVisitor mv, String descriptor) {
 				}
 			}
 			else {
-				// This is chopping off the 'L' to leave us with "java/lang/String"
-				mv.visitTypeInsn(CHECKCAST, descriptor.substring(1));
+				if (!descriptor.equals("Ljava/lang/Object")) {
+					// This is chopping off the 'L' to leave us with "java/lang/String"
+					mv.visitTypeInsn(CHECKCAST, descriptor.substring(1));
+				}
 			}
 		}
 	}

spring-expression/src/main/java/org/springframework/expression/spel/ast/Ternary.java

@@ -109,6 +109,9 @@ public void generateCode(MethodVisitor mv, CodeFlow codeflow) {
 		computeExitTypeDescriptor();
 		codeflow.enterCompilationScope();
 		this.children[0].generateCode(mv, codeflow);
+		if (!CodeFlow.isPrimitive(codeflow.lastDescriptor())) {
+			CodeFlow.insertUnboxInsns(mv, 'Z', codeflow.lastDescriptor());
+		}
 		codeflow.exitCompilationScope();
 		Label elseTarget = new Label();
 		Label endOfIf = new Label();

spring-expression/src/main/java/org/springframework/expression/spel/ast/VariableReference.java

@@ -16,6 +16,8 @@
 
 package org.springframework.expression.spel.ast;
 
+import java.lang.reflect.Modifier;
+
 import org.springframework.asm.MethodVisitor;
 import org.springframework.expression.EvaluationContext;
 import org.springframework.expression.TypedValue;
@@ -71,7 +73,17 @@ public TypedValue getValueInternal(ExpressionState state) throws SpelEvaluationE
 			return result;
 		}
 		TypedValue result = state.lookupVariable(this.name);
-		this.exitTypeDescriptor = CodeFlow.toDescriptorFromObject(result.getValue());
+		Object value = result.getValue();
+		if (value == null || !Modifier.isPublic(value.getClass().getModifiers())) {
+			// If the type is not public then when generateCode produces a checkcast to it
+			// then an IllegalAccessError will occur.
+			// If resorting to Object isn't sufficient, the hierarchy could be traversed for 
+			// the first public type.
+			this.exitTypeDescriptor ="Ljava/lang/Object";
+		}
+		else {
+			this.exitTypeDescriptor = CodeFlow.toDescriptorFromObject(value);
+		}
 		// a null value will mean either the value was null or the variable was not found
 		return result;
 	}

spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java

@@ -25,7 +25,6 @@
 import java.util.StringTokenizer;
 
 import org.junit.Test;
-
 import org.springframework.asm.MethodVisitor;
 import org.springframework.expression.AccessException;
 import org.springframework.expression.EvaluationContext;
@@ -522,6 +521,19 @@ public void ternary() throws Exception {
 		assertEquals(1,expression.getValue(root));		
 	}
 
+	@Test
+	public void ternaryWithBooleanReturn() { // SPR-12271
+		expression = parser.parseExpression("T(Boolean).TRUE?'abc':'def'");
+		assertEquals("abc",expression.getValue());
+		assertCanCompile(expression);
+		assertEquals("abc",expression.getValue());
+
+		expression = parser.parseExpression("T(Boolean).FALSE?'abc':'def'");
+		assertEquals("def",expression.getValue());
+		assertCanCompile(expression);
+		assertEquals("def",expression.getValue());
+	}
+	
 	@Test
 	public void elvis() throws Exception {
 		Expression expression = parser.parseExpression("'a'?:'b'");
@@ -1830,7 +1842,6 @@ public void methodReference_simpleInstanceMethodOneArgReturnPrimitive2() throws
 		assertEquals('c',resultC);
 	}
 
-
 	@Test
 	public void compoundExpression() throws Exception {
 		Payload payload = new Payload();
@@ -1914,7 +1925,18 @@ public void propertyReference() throws Exception {
 		assertCanCompile(expression);
 		assertEquals("value4",expression.getValue(tc));
 	}
-	
+
+	@Test
+	public void propertyReferenceVisibility() { // SPR-12771
+		StandardEvaluationContext ctx = new StandardEvaluationContext();
+		ctx.setVariable("httpServletRequest", HttpServlet3RequestFactory.getOne());
+		// Without a fix compilation was inserting a checkcast to a private type
+		expression = parser.parseExpression("#httpServletRequest.servletPath");
+		assertEquals("wibble",expression.getValue(ctx));
+		assertCanCompile(expression);
+		assertEquals("wibble",expression.getValue(ctx));
+	}
+		
 	@SuppressWarnings("unchecked")
 	@Test
 	public void indexer() throws Exception {
@@ -2954,4 +2976,28 @@ private static class TestClass9 {
 		public TestClass9(int i) {}
 	}
 
+	// These test classes simulate a pattern of public/private classes seen in Spring Security
+	
+	// final class HttpServlet3RequestFactory implements HttpServletRequestFactory 
+	static class HttpServlet3RequestFactory {
+	
+	  static Servlet3SecurityContextHolderAwareRequestWrapper getOne() {
+		  HttpServlet3RequestFactory outer = new HttpServlet3RequestFactory();
+		  return outer.new Servlet3SecurityContextHolderAwareRequestWrapper();
+	  }
+	  // private class Servlet3SecurityContextHolderAwareRequestWrapper extends SecurityContextHolderAwareRequestWrapper
+	  private class Servlet3SecurityContextHolderAwareRequestWrapper extends SecurityContextHolderAwareRequestWrapper {
+	  }
+	}
+	
+	// public class SecurityContextHolderAwareRequestWrapper extends HttpServletRequestWrapper 
+	static class SecurityContextHolderAwareRequestWrapper extends HttpServletRequestWrapper {
+	}
+	
+	public static class HttpServletRequestWrapper {
+		public String getServletPath() {
+			return "wibble";
+		}
+	}
+		
 }