From 64158ebaf5ca7ef79d87076dcb1ea39cf8a2d889 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 17 May 2018 10:37:10 +0100 Subject: [PATCH] Reinstate use of ConversionService for String -> File conversion Prior to 3db5c70b, RelaxedDataBinder would use a ConversionService to convert a String into a File via its ObjectToObjectConverter. 3db5c70b changed the configuration of the data binder such that a FileEditor was registered. Property editors take precedence over any conversion service so the FileEditor was used instead. This caused a regression as the FileEditor uses slightly unusual logic for a String to File conversion. Specifically, when given a value of ".", it will locate a ClassPathResource for the root of the classpath and use the result of calling getFile() on that resource. This fails when the root of the classpath is in a jar file and also provides a different result when the root of the classpath is not the current directory. This commit updates RelaxedDataBinder to suppress the registration of an editor for File. This restores the behaviour prior to 3db5c70b by allowing the ConversionService to be used instead. Closes gh-12786 --- .../boot/bind/RelaxedDataBinder.java | 27 ++++++++++++++ .../PropertiesConfigurationFactoryTests.java | 35 ++++++++++++++++++- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/bind/RelaxedDataBinder.java b/spring-boot/src/main/java/org/springframework/boot/bind/RelaxedDataBinder.java index 8be35c788df..5cb4f65cac8 100644 --- a/spring-boot/src/main/java/org/springframework/boot/bind/RelaxedDataBinder.java +++ b/spring-boot/src/main/java/org/springframework/boot/bind/RelaxedDataBinder.java @@ -16,6 +16,7 @@ package org.springframework.boot.bind; +import java.beans.PropertyEditor; import java.net.InetAddress; import java.util.ArrayList; import java.util.Collection; @@ -35,6 +36,7 @@ import org.springframework.beans.InvalidPropertyException; import org.springframework.beans.MutablePropertyValues; import org.springframework.beans.NotWritablePropertyException; import org.springframework.beans.PropertyValue; +import org.springframework.beans.propertyeditors.FileEditor; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.env.StandardEnvironment; @@ -58,6 +60,13 @@ import org.springframework.validation.DataBinder; */ public class RelaxedDataBinder extends DataBinder { + private static final Set> EXCLUDED_EDITORS; + static { + Set> excluded = new HashSet>(); + excluded.add(FileEditor.class); + EXCLUDED_EDITORS = Collections.unmodifiableSet(excluded); + } + private static final Object BLANK = new Object(); private String namePrefix; @@ -453,6 +462,24 @@ public class RelaxedDataBinder extends DataBinder { return target; } + @Override + public void registerCustomEditor(Class requiredType, + PropertyEditor propertyEditor) { + if (propertyEditor == null + || !EXCLUDED_EDITORS.contains(propertyEditor.getClass())) { + super.registerCustomEditor(requiredType, propertyEditor); + } + } + + @Override + public void registerCustomEditor(Class requiredType, String field, + PropertyEditor propertyEditor) { + if (propertyEditor == null + || !EXCLUDED_EDITORS.contains(propertyEditor.getClass())) { + super.registerCustomEditor(requiredType, field, propertyEditor); + } + } + /** * Holder to allow Map targets to be bound. */ diff --git a/spring-boot/src/test/java/org/springframework/boot/bind/PropertiesConfigurationFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/bind/PropertiesConfigurationFactoryTests.java index ed7a8498f4f..faf1ac496fe 100644 --- a/spring-boot/src/test/java/org/springframework/boot/bind/PropertiesConfigurationFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/bind/PropertiesConfigurationFactoryTests.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. @@ -16,6 +16,7 @@ package org.springframework.boot.bind; +import java.io.File; import java.io.IOException; import java.util.Collections; import java.util.Properties; @@ -27,7 +28,9 @@ import org.junit.Test; import org.springframework.beans.NotWritablePropertyException; import org.springframework.boot.context.config.RandomValuePropertySource; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.support.StaticMessageSource; +import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.core.env.MutablePropertySources; import org.springframework.core.env.PropertiesPropertySource; import org.springframework.core.env.StandardEnvironment; @@ -209,6 +212,22 @@ public class PropertiesConfigurationFactoryTests { assertThat(foo.fooDLQBar).isEqualTo(("baz")); } + @Test + public void currentDirectoryCanBeBoundToFileProperty() throws Exception { + PropertiesConfigurationFactory factory = new PropertiesConfigurationFactory( + FileProperties.class); + factory.setApplicationContext(new AnnotationConfigApplicationContext()); + factory.setConversionService(new DefaultConversionService()); + Properties properties = PropertiesLoaderUtils + .loadProperties(new ByteArrayResource("someFile: .".getBytes())); + MutablePropertySources propertySources = new MutablePropertySources(); + propertySources.addFirst(new PropertiesPropertySource("test", properties)); + factory.setPropertySources(propertySources); + factory.afterPropertiesSet(); + FileProperties fileProperties = factory.getObject(); + assertThat(fileProperties.getSomeFile()).isEqualTo(new File(".")); + } + private Foo createFoo(final String values) throws Exception { setupFactory(); return bindFoo(values); @@ -298,4 +317,18 @@ public class PropertiesConfigurationFactoryTests { } + public static class FileProperties { + + private File someFile; + + public File getSomeFile() { + return this.someFile; + } + + public void setSomeFile(File someFile) { + this.someFile = someFile; + } + + } + }