From 3118f14dbbdb653e976a994c0afa8c06e258f678 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Mon, 28 May 2018 14:22:38 +0200 Subject: [PATCH] Fix NPE when OnExpressionCondition is invoked with a null BeanFactory Closes gh-13249 --- .../condition/OnExpressionCondition.java | 27 ++++++++++---- .../ConditionalOnExpressionTests.java | 37 +++++++++++++++++-- 2 files changed, 53 insertions(+), 11 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnExpressionCondition.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnExpressionCondition.java index 8a2117d889..896c73c6b4 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnExpressionCondition.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnExpressionCondition.java @@ -29,6 +29,7 @@ import org.springframework.core.type.AnnotatedTypeMetadata; * A Condition that evaluates a SpEL expression. * * @author Dave Syer + * @author Stephane Nicoll * @see ConditionalOnExpression */ @Order(Ordered.LOWEST_PRECEDENCE - 20) @@ -43,18 +44,28 @@ class OnExpressionCondition extends SpringBootCondition { expression = wrapIfNecessary(expression); String rawExpression = expression; expression = context.getEnvironment().resolvePlaceholders(expression); + ConditionMessage.Builder messageBuilder = ConditionMessage + .forCondition(ConditionalOnExpression.class, "(" + rawExpression + ")"); ConfigurableListableBeanFactory beanFactory = context.getBeanFactory(); - BeanExpressionResolver resolver = (beanFactory != null - ? beanFactory.getBeanExpressionResolver() : null); - BeanExpressionContext expressionContext = (beanFactory != null - ? new BeanExpressionContext(beanFactory, null) : null); + if (beanFactory != null) { + boolean result = evaluateExpression(beanFactory, expression); + return new ConditionOutcome(result, messageBuilder.resultedIn(result)); + } + else { + return ConditionOutcome + .noMatch(messageBuilder.because("no BeanFactory available.")); + } + } + + private Boolean evaluateExpression(ConfigurableListableBeanFactory beanFactory, + String expression) { + BeanExpressionResolver resolver = beanFactory.getBeanExpressionResolver(); if (resolver == null) { resolver = new StandardBeanExpressionResolver(); } - boolean result = (Boolean) resolver.evaluate(expression, expressionContext); - return new ConditionOutcome(result, ConditionMessage - .forCondition(ConditionalOnExpression.class, "(" + rawExpression + ")") - .resultedIn(result)); + BeanExpressionContext expressionContext = new BeanExpressionContext(beanFactory, + null); + return (Boolean) resolver.evaluate(expression, expressionContext); } /** diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnExpressionTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnExpressionTests.java index b85966e7d1..ad217d51ce 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnExpressionTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnExpressionTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-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. @@ -16,25 +16,34 @@ package org.springframework.boot.autoconfigure.condition; +import java.util.HashMap; +import java.util.Map; + import org.junit.Test; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.ConditionContext; import org.springframework.context.annotation.Configuration; +import org.springframework.core.type.AnnotatedTypeMetadata; +import org.springframework.mock.env.MockEnvironment; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; /** * Tests for {@link ConditionalOnExpression}. * * @author Dave Syer + * @author Stephane Nicoll */ public class ConditionalOnExpressionTests { private final AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); @Test - public void testResourceExists() { + public void expressionEvaluatesToTrueRegisterBean() { this.context.register(BasicConfiguration.class); this.context.refresh(); assertThat(this.context.containsBean("foo")).isTrue(); @@ -42,12 +51,34 @@ public class ConditionalOnExpressionTests { } @Test - public void testResourceNotExists() { + public void expressionEvaluatesToFalseDoesNotRegisterBean() { this.context.register(MissingConfiguration.class); this.context.refresh(); assertThat(this.context.containsBean("foo")).isFalse(); } + @Test + public void expressionEvaluationWithNoBeanFactoryDoesNotMatch() { + OnExpressionCondition condition = new OnExpressionCondition(); + MockEnvironment environment = new MockEnvironment(); + ConditionContext conditionContext = mock(ConditionContext.class); + given(conditionContext.getEnvironment()).willReturn(environment); + ConditionOutcome outcome = condition.getMatchOutcome(conditionContext, + mockMetaData("invalid-spel")); + assertThat(outcome.isMatch()).isFalse(); + assertThat(outcome.getMessage()).contains("invalid-spel") + .contains("no BeanFactory available"); + } + + private AnnotatedTypeMetadata mockMetaData(String value) { + AnnotatedTypeMetadata metadata = mock(AnnotatedTypeMetadata.class); + Map attributes = new HashMap(); + attributes.put("value", value); + given(metadata.getAnnotationAttributes(ConditionalOnExpression.class.getName())) + .willReturn(attributes); + return metadata; + } + @Configuration @ConditionalOnExpression("false") protected static class MissingConfiguration {