Spring-framework: MockServletContext should treat InvalidPathException like an IOException

[Spunc]

I have a simple ConfigurationProperties class that binds a String property to a Path object:

@Component
@ConfigurationProperties("app")
@Getter
@Setter
public class AppConfigurationProperties {

    private Path somePath;
}

Then I define app.somePath=C:\\temp within _application.properties_. This works when running the application, but a simple Test that loads the context throws:

com.example.demo.DemoApplicationTests > contextLoads() FAILED
    java.lang.IllegalStateException
        Caused by: org.springframework.boot.context.properties.ConfigurationPropertiesBindException
            Caused by: org.springframework.boot.context.properties.bind.BindException
                Caused by: org.springframework.core.convert.ConverterNotFoundException

The test class is most simple:

@SpringBootTest
public class DemoApplicationTests {

    @Test
    public void contextLoads() {
    }
}

I created a project with the most recent Spring-Boot version (2.1.8.RELEASE) with Initializr (Gradle build with Spring Web as only dependency) to replicate the problem in isolation. It can be found here. I run the project on Windows 10.

I found out that the test passes, if there are no backslashes in the property value. Furthermore, it passes if I don't use org.springframework.boot:spring-boot-starter-web but org.springframework.boot:spring-boot-starter as dependency. (The test project does not require web, but the actual project does.)

[mbhave]

Thanks for the sample @Spunc. I ran DemoApplicationTests without changing anything and it was green. Is there something additional that needs to be done to reproduce the failure?

[wilkinsona]

It looks like running on Windows is significant. The test fails for me there. We end up with the following:

1. PathEditor setAsText(String) is called with C:\temp
2. ResourceEditor setAsText(String) is called with C:\temp
3. GenericWebApplicationContext (the ResourceLoader) getResource(String) is called with C:\temp
4. A ServletContextResource with the path /C:/temp is created
5. The stack unwinds into PathEditor and it calls exists() on the resource
6. SpringBootMockServletContext getResource is called with /C:/temp
7. MockServletContext getResource(String) is called with /C:/temp
8. SpringBootMockServletContext getResourceLocation(String) is called with /C:/temp
9. MockServletContext getResourceLocation(String) is called with /C:/temp and it prepends the base path and returns src/main/webapp/C:/temp
10. FileSystemResourceLoader getResource(String) is called with src/main/webapp/C:/temp
11. FileSystemResourceLoader getResourceByPath(String) is called with src/main/webapp/C:/temp
12. A FileSystemResource is constructed with src/main/webapp/C:/temp. It creates a File and then calls toPath() which fails with an InvalidPathException

The Windows-specific behaviour is in step 12. The InvalidPathException is not thrown on macOS where a UnixPath is successfully created and returned.

This feels like a bug in MockServletContext getResource(String) to me (which is why the problem only occurs in the tests of a web application). The contract allows the method to throw a MalformedURLException or to return null. The implementation catches IOException and returns null. I think this catch needs to be broadened to map other failures, such as InvalidPathException, to null as well.

@Spunc You can work around the problem by using file:/C:/temp as the value of your app.somePath property.

[Spunc]

Thank you very much for your replies. At first, I was surprised that @mbhave could not reproduce the problem. I had colleagues verify the issue and indeed, those with Windows have the problem while those with Mac-OS are not affected.

Thanks, @wilkinsona for your suggestion of a workaround. We will apply that.

[sbrannen]

Thanks for the great detective work, @wilkinsona!

We'll look into it.

[xak2000]

I also run into this problem with spring 5.1.5. Now I updated to 5.1.11 and the problem "solved", but now it prints warning to the log.

@sbrannen @wilkinsona
I don't understand why org.springframework.mock.web.MockServletContext.getResourceLocation(String path) should prepend resourceBasePath to an absolute Windows path like C:\temp? So the result path is src/main/webapp/C:/temp. This would never work anyway.

If we leave /C:/temp here, then this.file.toPath() would not throw. It creates WindowsPath("C:\temp") instance.

Maybe this is incorrect to leave it like this because this is servlet context anyway (I'm not sure here what's the intention). But it is also have no sense to just do a concatenation like this. Maybe it works on unix systems, where any absolute path can be converted to relative just by appending another path at the beginning, but it doesn't work on Windows, where path can contain a hard drive letter.

If the path must really be prepended with servlet context path here, maybe it is better to remove the letter from path before doing this? So /C:/temp will become /temp and then src/main/webapp/temp. I can't reason about this because to be honest I don't fully understand this code and why we need to manipulate paths from application.properties this way in tests. If I created a property like my-location=C:\temp and run test with it, I don't expect it to be manipulated in some ways to be changed to point to some another path in a servlet context. Maybe for relative paths, but not for absolute.

P.S. In my case a property was just like this:

my-prop=${java.io.tmpdir}/some/path

This was intentionally done in test properties to make sure the test runs on Windows and Unix machines without changes. And it worked while I has custom @ConfigurationPropertiesBinding Converter bean in my context that convert String to Path by just doing Paths.get(string). Then I removed this custom converter in favor of standard SimpleTypeConverter with PathEditor. And then run into this issue.

[sbrannen]

@xak2000, since this issue has already been closed and the change has already been released, would you please open a new ticket to address your concerns?

Thanks in advance!