Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PathEditor should interpret empty string as relative empty path #25029

Closed
jannis-baratheon opened this issue Apr 29, 2020 · 3 comments
Closed
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: declined A suggestion or change that we don't feel we should currently apply

Comments

@jannis-baratheon
Copy link

jannis-baratheon commented Apr 29, 2020

Version information

Spring-boot version: 2.2.6.RELEASE

Issue reproduction and actual result

I have some simple constructor-bound ConfigurationProperties:

	package com.example.demo;
	
	import java.nio.file.Path;
	import org.springframework.boot.context.properties.ConfigurationProperties;
	import org.springframework.boot.context.properties.ConstructorBinding;
	import org.springframework.boot.context.properties.bind.DefaultValue;

	@ConfigurationProperties
	@ConstructorBinding
	public class ConfigurationPropertiesWithPathProperty {
		private final Path somePathProperty;

		public ConfigurationPropertiesWithPathProperty(Path somePathProperty) {
			this.somePathProperty = somePathProperty;
		}

		public Path getSomePathProperty() {
			return somePathProperty;
		}
	}

This test, in which I try to bind an empty string, fails:

	package com.example.demo;

	import static org.assertj.core.api.Assertions.assertThat;

	import java.nio.file.Paths;
	import java.util.Collections;
	import java.util.HashMap;
	import java.util.Map;
	import org.junit.jupiter.api.Test;
	import org.springframework.boot.context.properties.bind.BindResult;
	import org.springframework.boot.context.properties.bind.Bindable;
	import org.springframework.boot.context.properties.bind.Binder;
	import org.springframework.boot.context.properties.source.MapConfigurationPropertySource;

	public class PropertiesTest {

		@Test
		void bindsEmptyToPathProperty() {
			Map<String, String> properties = new HashMap<>();
			properties.put("some_path_property", "");
			Binder binder = new Binder(new MapConfigurationPropertySource(properties));

			BindResult<ConfigurationPropertiesWithPathProperty> bindResult =
					binder.bind("", Bindable.of(ConfigurationPropertiesWithPathProperty.class));

			assertThat(bindResult.isBound()).isTrue();
			assertThat(bindResult.get().getSomePathProperty()).isEqualTo(Paths.get(""));
		}
	}

Additional observations

When I add a @DefaultValue to the constructor property:

	public ConfigurationPropertiesWithPathProperty(@DefaultValue("some_default") Path somePathProperty) {
		this.somePathProperty = somePathProperty;
	}

The same test fails with an exception. The exception is the same as in spring-projects/spring-boot#21264, but it means that binding to the default value is attempted.

Note that the same issue occurs not only with constructor-bound properties, but java bean binding as well.

Expected result

An empty string can be bound to a property of java.nio.file.Path type resulting in a relative empty path (equal to the result of Paths.get("")).

@jannis-baratheon jannis-baratheon changed the title @ConfigurationProperties with @ConstructorBinding unable to bind default value supplied with @DefaultValue @ConfigurationProperties with @ConstructorBinding unable to bind default value to a Path property Apr 29, 2020
@philwebb

This comment has been minimized.

@philwebb philwebb self-assigned this May 2, 2020
@philwebb philwebb changed the title @ConfigurationProperties with @ConstructorBinding unable to bind default value to a Path property @ConfigurationProperties with @ConstructorBinding unable to bind empty string to a Path property May 2, 2020
@philwebb
Copy link
Member

philwebb commented May 2, 2020

From @mbhave in the original issue:

For this, we rely on Spring Framework's PathEditor for converting from String to Path. The PathEditor returns a null value if the source value is "". This might be something that needs to be fixed in Spring Framework. If the rest of the team agrees, this issue will need to be transferred to Spring Framework's issue tracker.

@philwebb philwebb transferred this issue from spring-projects/spring-boot May 7, 2020
@philwebb philwebb removed their assignment May 7, 2020
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 7, 2020
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label May 7, 2020
@sbrannen sbrannen changed the title @ConfigurationProperties with @ConstructorBinding unable to bind empty string to a Path property PathEditor should interpret empty string as relative empty path May 7, 2020
@snicoll
Copy link
Member

snicoll commented Sep 21, 2023

Unfortunately, that's how the editors work. They treat an empty value as the absence of a value. I understand that the empty string has a special meaning for Path but this could come as a surprise for users that rely on the current behavior.

@snicoll snicoll closed this as not planned Won't fix, can't repro, duplicate, stale Sep 21, 2023
@snicoll snicoll added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

5 participants