From 66d1f5cd0bb7324548c9ce0dfc1c34aacf536bcb Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Tue, 6 Mar 2018 15:19:59 +0100 Subject: [PATCH] Fix expansion of static-locations array This commit fixes a NPE when the static-locations array of `ResourceProperties` has to be expanded as the setter is cleaning the values of the array and is affected by a non-intuitive behaviour of the binder. When the binder needs to set an element of an array and the size of the array isn't large enough, the binder proceeds as follows: * An array of the required size is created * The content of the original array is copied over * The setter of the property is invoked with the new array * The setter of the property is invoked and the returned array is mutated to set the requested value While one would expect the array to contain the requested value when the setter is invoked, this is not the case. Also, the array might contain null values if a value at index 8 should be set and the array has a size of 3. All in all, `ResourceProperties#appendSlashIfNecessary` has to account for `null` and an additional round of cleaning has to happen once binding has completed. Closes gh-12360 --- .../autoconfigure/web/ResourceProperties.java | 14 +++- .../web/ResourcePropertiesBindingTests.java | 74 +++++++++++++++++++ 2 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ResourcePropertiesBindingTests.java diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ResourceProperties.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ResourceProperties.java index 2cd70ce584..93550ec2db 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ResourceProperties.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ResourceProperties.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 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. @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import org.springframework.beans.factory.InitializingBean; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.NestedConfigurationProperty; import org.springframework.context.ResourceLoaderAware; @@ -37,7 +38,7 @@ import org.springframework.core.io.ResourceLoader; * @since 1.1.0 */ @ConfigurationProperties(prefix = "spring.resources", ignoreUnknownFields = false) -public class ResourceProperties implements ResourceLoaderAware { +public class ResourceProperties implements ResourceLoaderAware, InitializingBean { private static final String[] SERVLET_RESOURCE_LOCATIONS = { "/" }; @@ -81,6 +82,11 @@ public class ResourceProperties implements ResourceLoaderAware { this.resourceLoader = resourceLoader; } + @Override + public void afterPropertiesSet() { + this.staticLocations = appendSlashIfNecessary(this.staticLocations); + } + public String[] getStaticLocations() { return this.staticLocations; } @@ -93,7 +99,9 @@ public class ResourceProperties implements ResourceLoaderAware { String[] normalized = new String[staticLocations.length]; for (int i = 0; i < staticLocations.length; i++) { String location = staticLocations[i]; - normalized[i] = (location.endsWith("/") ? location : location + "/"); + if (location != null) { + normalized[i] = (location.endsWith("/") ? location : location + "/"); + } } return normalized; } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ResourcePropertiesBindingTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ResourcePropertiesBindingTests.java new file mode 100644 index 0000000000..3b3a5005d7 --- /dev/null +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ResourcePropertiesBindingTests.java @@ -0,0 +1,74 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.autoconfigure.web; + +import org.junit.After; +import org.junit.Test; + +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.test.util.EnvironmentTestUtils; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Configuration; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Binding tests for {@link ResourceProperties}. + * + * @author Stephane Nicoll + */ +public class ResourcePropertiesBindingTests { + + private AnnotationConfigApplicationContext context; + + @After + public void close() { + if (this.context != null) { + this.context.close(); + } + } + + @Test + public void staticLocationsExpandArray() { + ResourceProperties properties = load( + "spring.resources.static-locations[0]=classpath:/one/", + "spring.resources.static-locations[1]=classpath:/two", + "spring.resources.static-locations[2]=classpath:/three/", + "spring.resources.static-locations[3]=classpath:/four", + "spring.resources.static-locations[4]=classpath:/five/", + "spring.resources.static-locations[5]=classpath:/six"); + assertThat(properties.getStaticLocations()).contains("classpath:/one/", + "classpath:/two/", "classpath:/three/", "classpath:/four/", + "classpath:/five/", "classpath:/six/"); + } + + private ResourceProperties load(String... environment) { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + EnvironmentTestUtils.addEnvironment(ctx, environment); + ctx.register(TestConfiguration.class); + ctx.refresh(); + this.context = ctx; + return this.context.getBean(ResourceProperties.class); + } + + @Configuration + @EnableConfigurationProperties(ResourceProperties.class) + static class TestConfiguration { + + } + +}