diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java b/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java index d374e483e1306afce13a997e8ce2a53509c6bda7..9d006be914b8d941628ee0b410b6657c46418892 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java @@ -446,8 +446,10 @@ public class CodeFlow implements Opcodes { } } 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)); + } } } } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Ternary.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Ternary.java index 96868e25b0b0a4b3f366f9156b46b759858f8eb9..ae0171a3255891a6bc3d109749ecfef196d6abf5 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Ternary.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Ternary.java @@ -109,6 +109,9 @@ public class Ternary extends SpelNodeImpl { 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(); diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/VariableReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/VariableReference.java index ee7f74a4d5c7ceaa5e1ee0891ab824cf712681c7..f827bc475ca2f04a3d760a25ce63fbf2ede1fcbd 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/VariableReference.java +++ b/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 class VariableReference extends SpelNodeImpl { 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; } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java index f06926b99849fac7057e2abd53030c6c5cc76ccb..e03768143c1877755a66813aaf595c04ed980cae 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java @@ -25,7 +25,6 @@ import java.util.Map; 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 class SpelCompilationCoverageTests extends AbstractExpressionTests { 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 class SpelCompilationCoverageTests extends AbstractExpressionTests { assertEquals('c',resultC); } - @Test public void compoundExpression() throws Exception { Payload payload = new Payload(); @@ -1914,7 +1925,18 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { 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 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { 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"; + } + } + }