Root path resolution for java.nio.Path properties does not work on Linux anymore

[plc010]

Under Spring Boot 2.4.4 having a configuration class with a java.nio.Path field does not map correctly for a root filesystem on Linux if the spring-boot-web-starter is included.

Example application.properties value:
my-config.workingPath=/my-working-path

When the above is run on linux with the web starter included, the path is set under the tomcat temp directory, or under the exploded WAR file directory if running on a different application server.

If the path is set to a value such as: c:\temp\my-working-path on a Windows machine, there is no issue with or without the web starter dependency.

Creating a ConfigurationPropertiesBinding converter seems to be picked up when running on Windows, but ignored on Linux.

Changing the properties class field from java.nio.Path to String and implementing a custom get method that returns Paths.get(s) works fine on windows and linxu both with and without the web starter included in the project.

Issue does not seem to exist prior to Spring Boot 2.4.4.

[wilkinsona]

Thanks for the report. I don't recall any changes for Path conversion in 2.4.4, but something may have changed in Spring Framework. To help us to diagnose the root cause of the problem, can you please provide a minimal sample that reproduces it? You can share it with us by pushing it to a separate repository on GitHub or zipping it up and attaching it to this issue.

[wilkinsona]

Scratch that. There's no need for a sample as the problem's easier to reproduce than I had anticipated. This will do it:

package com.example.demo;

import java.nio.file.Path;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.ConfigurableApplicationContext;

@SpringBootApplication
@EnableConfigurationProperties(TestProperties.class)
public class Gh25734Application {

	public static void main(String[] args) {
		ConfigurableApplicationContext context = SpringApplication.run(Gh25734Application.class, "--test.path=/some/root/path");
		TestProperties testProperties = context.getBean(TestProperties.class);
		System.out.println(testProperties.getPath());
		context.close();
	}

}

@ConfigurationProperties(prefix="test")
class TestProperties {
	
	private Path path;

	public Path getPath() {
		return path;
	}

	public void setPath(Path path) {
		this.path = path;
	}
	
}

With Spring Boot 2.4.3 it outputs the following:

/some/root/path

Whereas Spring Boot 2.4.4 outputs the following:

/private/var/folders/2w/mhq2nt4d0xx2w6w4c36n468h0000gn/T/tomcat-docbase.8080.3932269735022930379/some/root/path

With Spring Boot 2.4.4 and Spring Framework downgraded to 5.3.4 it outputs the following:

/some/root/path

This looks like a regression in Spring Framework 5.3.5, I suspect due to this change. We'll transfer this issue to the Framework team so that they can take a look.

[sbrannen]

Potential regression introduced in conjunction with #26575.

[jhoeller]

This looks very much like a regression caused by that change indeed. I suspect it only happens for non-existing paths because that's what #26575 was trying to fix in Windows scenarios? Any suggestions for how to refine that check to work on Windows as well as Linux?

On a side note, we need to backport this to 5.2.x as well. Fortunately 5.2.14 isn't released yet but the original change got backported to the branch.

[wilkinsona]

I suspect it only happens for non-existing paths

It seems to happen for paths that exist as well. For example, /private is resolved to /private/var/folders/2w/mhq2nt4d0xx2w6w4c36n468h0000gn/T/tomcat-docbase.8080.365659160696891053/private on macOS.

That happens because the isFile() call on resource (a ServletContextResource) returns true.

[jhoeller]

Hmm, I guess I'm still missing something...

If the path exists, wouldn't it always have gone into the resource.getFile().toPath() part to begin with? That change only really reduced the cases for our special non-existent path resolution. If resource.exists() returns true, the change shouldn't make a difference since the additional !resource.isFile() check concatenates with !resource.exists().

So if we fail to resolve against root in such a Servlet resource scenario on Linux, do we have potentially an issue with the resource.exists() and/or the resource.getFile() implementation in ServletContextResource? I'd appreciate further insight there since the change in the original PR still seems sensible (and did fix the behavior on Windows)...

[wilkinsona]

With 5.3.4, the call to exists() on the ServletContextResource returns false, even for a path that exists on the file system as it's not part of the servlet context. This is sufficient for it to use Paths.get(text).normalize() as the value:

spring-framework/spring-beans/src/main/java/org/springframework/beans/propertyeditors/PathEditor.java

Lines 100 to 102 in d9a9686

	else if (!resource.exists() && nioPathCandidate) {
	setValue(Paths.get(text).normalize());
	}

On 5.3.5 the call to exists() isn't made as isFile() has already returned true. It returns true due to this.servletContext.getRealPath(this.path) returning a non-null value. As a result, resource.getFile().toPath() is used as the value:

spring-framework/spring-beans/src/main/java/org/springframework/beans/propertyeditors/PathEditor.java

Lines 100 to 111 in 7c2a72c

	else if (!resource.isFile() && !resource.exists() && nioPathCandidate) {
	// Prefer getFile().toPath() below for non-existent file handles
	setValue(Paths.get(text).normalize());
	}
	else {
	try {
	setValue(resource.getFile().toPath());
	}
	catch (IOException ex) {
	throw new IllegalArgumentException("Failed to retrieve file for " + resource, ex);
	}
	}

I think 5.3.4 would behave differently if the servlet context happened to contain a resource that matched the path. If this ambiguity is avoided by using file:/private as the value rather than /private, a Path of /private is the end result with 5.3.4 and 5.3.5.

[jhoeller]

Ah ok that clarifies things quite a bit, thanks for digging deeper there, @wilkinsona ...

So now we need to figure out which part of the interaction is wrong there. Maybe we'll replace the isFile() check with a specific file: prefix check, to the best of my present understanding this would fix the regression on Linux and preserve the original fix on Windows. That said, it's a shame that ServletContextResource is behaving somewhat oddly there.

[fatroom]

Got struck by this issue as well on OSX, spring 5.3.5 behaves differently with IDEA run and gradleRun.
Example project is here: https://github.com/fatroom/spring-path-injection-example

[kascaks]

To be honest, I'm not quite sure what the fix was trying to correct. The way I understand file uri scheme, you cannot have double slash after file: and all the backslashes should be converted to forward as well. i.e. file://C:\file.txt is not allowed. It either has to be file:///C:/file.txt or file:/C:/file.txt. When changing the URI in testWindowsAbsoluteFilePath() as mentioned above (e.g. pathEditor.setAsText("file:/C:/no_way_this_file_is_found.doc");, there is no problem to pass the test in 5.3.4.

[jhoeller]

It turns out that the lenient resolution of Windows-style path separators after a "file:" prefix has been established by FileSystemResource and the underlying java.net.URL, so it seems sensible to accept it consistently here as well. Since java.net.URI throws a URISyntaxException in such a case, we turn nioPathCandidate to false in case of a "file:" prefix now, with no need for an isFile() check later on. In other words, the original fix is revised to only apply for "file:" specifications now, not for all file resources.

[jhoeller]

@wilkinsona @snicoll It'd be great if you could test the fix with Boot on Linux/MacOS before we release tomorrow. I can confirm that it (still) works fine on Windows; hopefully this fixes the regression for good.

[snicoll]

@jhoeller I confirm the sample above works for MacOS with the latest snapshot. I ran the app in a linux docker container and it worked as well.