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

Add support for double backslashes to StringUtils#cleanPath #32962

Closed
floringolintchi opened this issue Jun 5, 2024 · 5 comments
Closed

Add support for double backslashes to StringUtils#cleanPath #32962

floringolintchi opened this issue Jun 5, 2024 · 5 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Milestone

Comments

@floringolintchi
Copy link

floringolintchi commented Jun 5, 2024

Affects: 6.1.8

While upgrading a project from spring boot 3.1.12 to spring boot 3.2.6 I've encountered issues with my @Value injection path to system file on my Windows machine being rebuilt incorrectly by StringUtils.cleanPath(...). I believe this concerns the way Spring handles \ characters when loading maven properties in application.yml. I've created a demo I'll attach showcasing the issue.

My file is loaded like this:

    @Value("${path.to.a.file}")
    private RSAPublicKey resource;

where the property in application.yml looks like this:

path.to.a.file: file:@path.to.a.key.file@

the maven property referenced is:

<properties>
        <main.basedir>${project.basedir}${file.separator}..</main.basedir>
        <path.to.a.key.file>${main.basedir}${file.separator}lib${file.separator}someFile</path.to.a.key.file>
</properties>

(the original project is a multi-module project)

From my testing this is related to the change to https://github.com/spring-projects/spring-framework/blob/v6.1.8/spring-core/src/main/java/org/springframework/util/ResourceUtils.java#L412
in previous major version this would build directly without any changes to location:
https://github.com/spring-projects/spring-framework/blob/v6.0.21/spring-core/src/main/java/org/springframework/util/ResourceUtils.java#L419

I don't think this is an issue with StringUtils.cleanPath(...), but with the fact that Spring will inject this maven property with double \\, rather than just one \. Example images from the demo project I have attached, how application.yml looks like in target folder:

Screenshot 2024-06-05 173850

And debugging, what the String is equal to since I was confused by the IntelliJ output:

Screenshot 2024-06-05 174205

how cleanPath outputs the wrong path when given input with double \:

Screenshot 2024-06-05 174536

after removing one escaped \, cleanPath works as expected:

Screenshot 2024-06-05 174644

From what I can tell maven won't use double backslashes so these are added by Spring I believe (output of maven command to check how properties look like):

Screenshot 2024-06-05 175306

Here is the stack trace:

Error starting ApplicationContext. To display the condition evaluation report re-run your application with 'debug' enabled.
2024-06-05T18:05:06.429+03:00 ERROR 28320 --- [ main] o.s.boot.SpringApplication : Application run failed

org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'com.example.Main$ExampleComponent': Unsatisfied dependency expressed through field 'resource': Failed to convert value of type 'java.lang.String' to required type 'java.security.interfaces.RSAPublicKey'; Failed to convert from type [java.lang.String] to type [@org.springframework.beans.factory.annotation.Value java.security.interfaces.RSAPublicKey] for value [file:C:\Users\...\IdeaProjects\demo_resourceUtils_issue\child\..\lib\someFile]
at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor$AutowiredFieldElement.resolveFieldValue(AutowiredAnnotationBeanPostProcessor.java:787) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor$AutowiredFieldElement.inject(AutowiredAnnotationBeanPostProcessor.java:767) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.beans.factory.annotation.InjectionMetadata.inject(InjectionMetadata.java:145) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor.postProcessProperties(AutowiredAnnotationBeanPostProcessor.java:508) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.populateBean(AbstractAutowireCapableBeanFactory.java:1421) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:599) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:522) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:337) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:234) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:335) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:200) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:975) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:962) ~[spring-context-6.1.8.jar:6.1.8]
at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:624) ~[spring-context-6.1.8.jar:6.1.8]
at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.refresh(ServletWebServerApplicationContext.java:146) ~[spring-boot-3.2.6.jar:3.2.6]
at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:754) ~[spring-boot-3.2.6.jar:3.2.6]
at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:456) ~[spring-boot-3.2.6.jar:3.2.6]
at org.springframework.boot.SpringApplication.run(SpringApplication.java:335) ~[spring-boot-3.2.6.jar:3.2.6]
at org.springframework.boot.builder.SpringApplicationBuilder.run(SpringApplicationBuilder.java:149) ~[spring-boot-3.2.6.jar:3.2.6]
at com.example.Main.main(Main.java:14) ~[classes/:na]
Caused by: org.springframework.beans.TypeMismatchException: Failed to convert value of type 'java.lang.String' to required type 'java.security.interfaces.RSAPublicKey'; Failed to convert from type [java.lang.String] to type [@org.springframework.beans.factory.annotation.Value java.security.interfaces.RSAPublicKey] for value [file:C:\Users\...\IdeaProjects\demo_resourceUtils_issue\child\..\lib\someFile]
at org.springframework.beans.TypeConverterSupport.convertIfNecessary(TypeConverterSupport.java:87) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.beans.TypeConverterSupport.convertIfNecessary(TypeConverterSupport.java:71) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.beans.factory.support.DefaultListableBeanFactory.doResolveDependency(DefaultListableBeanFactory.java:1381) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveDependency(DefaultListableBeanFactory.java:1353) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor$AutowiredFieldElement.resolveFieldValue(AutowiredAnnotationBeanPostProcessor.java:784) ~[spring-beans-6.1.8.jar:6.1.8]
... 19 common frames omitted
Caused by: org.springframework.core.convert.ConversionFailedException: Failed to convert from type [java.lang.String] to type [@org.springframework.beans.factory.annotation.Value java.security.interfaces.RSAPublicKey] for value [file:C:\Users\...\IdeaProjects\demo_resourceUtils_issue\child\..\lib\someFile]
at org.springframework.core.convert.support.ConversionUtils.invokeConverter(ConversionUtils.java:47) ~[spring-core-6.1.8.jar:6.1.8]
at org.springframework.core.convert.support.GenericConversionService.convert(GenericConversionService.java:182) ~[spring-core-6.1.8.jar:6.1.8]
at org.springframework.beans.TypeConverterDelegate.convertIfNecessary(TypeConverterDelegate.java:131) ~[spring-beans-6.1.8.jar:6.1.8]
at org.springframework.beans.TypeConverterSupport.convertIfNecessary(TypeConverterSupport.java:80) ~[spring-beans-6.1.8.jar:6.1.8]
... 23 common frames omitted
Caused by: java.io.UncheckedIOException: java.io.FileNotFoundException: C:\Users...\IdeaProjects\demo_resourceUtils_issue\child\lib\someFile (The system cannot find the path specified)
at org.springframework.security.config.crypto.RsaKeyConversionServicePostProcessor$ResourceKeyConverterAdapter.toInputStream(RsaKeyConversionServicePostProcessor.java:158) ~[spring-security-config-6.2.4.jar:6.2.4]
at org.springframework.security.config.crypto.RsaKeyConversionServicePostProcessor$ResourceKeyConverterAdapter.lambda$pemInputStreamConverter$0(RsaKeyConversionServicePostProcessor.java:146) ~[spring-security-config-6.2.4.jar:6.2.4]
at org.springframework.core.convert.converter.Converter.lambda$andThen$0(Converter.java:62) ~[spring-core-6.1.8.jar:6.1.8]
at org.springframework.security.config.crypto.RsaKeyConversionServicePostProcessor$ResourceKeyConverterAdapter.convert(RsaKeyConversionServicePostProcessor.java:132) ~[spring-security-config-6.2.4.jar:6.2.4]
at org.springframework.security.config.crypto.RsaKeyConversionServicePostProcessor$ResourceKeyConverterAdapter.convert(RsaKeyConversionServicePostProcessor.java:113) ~[spring-security-config-6.2.4.jar:6.2.4]
at org.springframework.core.convert.support.GenericConversionService$ConverterAdapter.convert(GenericConversionService.java:358) ~[spring-core-6.1.8.jar:6.1.8]
at org.springframework.core.convert.support.ConversionUtils.invokeConverter(ConversionUtils.java:41) ~[spring-core-6.1.8.jar:6.1.8]
... 26 common frames omitted
Caused by: java.io.FileNotFoundException: C:\Users...\IdeaProjects\demo_resourceUtils_issue\child\lib\someFile (The system cannot find the path specified)
at java.base/java.io.FileInputStream.open0(Native Method) ~[na:na]
at java.base/java.io.FileInputStream.open(FileInputStream.java:216) ~[na:na]
at java.base/java.io.FileInputStream.(FileInputStream.java:157) ~[na:na]
at java.base/java.io.FileInputStream.(FileInputStream.java:111) ~[na:na]
at java.base/sun.net.www.protocol.file.FileURLConnection.connect(FileURLConnection.java:86) ~[na:na]
at java.base/sun.net.www.protocol.file.FileURLConnection.getInputStream(FileURLConnection.java:189) ~[na:na]
at org.springframework.core.io.UrlResource.getInputStream(UrlResource.java:233) ~[spring-core-6.1.8.jar:6.1.8]
at org.springframework.security.config.crypto.RsaKeyConversionServicePostProcessor$ResourceKeyConverterAdapter.toInputStream(RsaKeyConversionServicePostProcessor.java:155) ~[spring-security-config-6.2.4.jar:6.2.4]
... 32 common frames omitted

This can easily be fixed by editing the string with SpEL: @Value("#{'${path.to.a.file}'.replace('\\\\', '\\')}") or just telling maven to add the files to my classpath, but I am opening this in case this is an issue worth considering, I apologize otherwise.

Demo project: demo_resourceUtils_issue.zip

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 5, 2024
@sdeleuze sdeleuze self-assigned this Jun 7, 2024
@sdeleuze
Copy link
Contributor

sdeleuze commented Jun 7, 2024

Hi, I am not on Windows but what likely happens is that:

Even if this double backslash comes from a (kind of surprising to me) Maven feature enabled by default, the fact that \\ can be used as a separator on Windows, combined with the fact this is a popular arrangement (enabled by default with Boot parent POM) and that this is a change of behavior on Spring side makes me think that maybe we should handle this use case and convert \\ to / like we convert \ to /.

@sdeleuze sdeleuze added in: core Issues in core modules (aop, beans, core, context, expression) status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team labels Jun 7, 2024
@sdeleuze sdeleuze added this to the 6.1.9 milestone Jun 10, 2024
@sdeleuze sdeleuze changed the title Spring doubles backslash characters when loading maven property in application.yml leading to StringUtils.cleanPath issues Add support for double backslashes to StringUtils#cleanPath Jun 10, 2024
@sdeleuze
Copy link
Contributor

sdeleuze commented Jun 10, 2024

Based on https://en.wikipedia.org/wiki/File_URI_scheme, it should not be needed to support the leading \\ use case. Since there was some performance regression visible in the JMH benchmark, I have implemented an optimization for the case where there is no \ in the path.

@floringolintchi Please check your repro is fine on Windows with Spring Framework 6.1.9-SNAPSHOT since I am not able to test on Windows.

@floringolintchi
Copy link
Author

floringolintchi commented Jun 11, 2024

@sdeleuze Tried it in test project and it works fine now, thank you for the fix. 🍻

demo_updated_snapshot.zip

@stevo-devo
Copy link

This change has broken the ability to use a network path as a URI in windows. eg. if we want to access a path such as \\networklocation\path we would specify \\\\networklocation/path and this previously worked.

Now it is always failing due to the path being "cleaned" as /networklocation/path due to this change.

@sbrannen
Copy link
Member

@stevo-devo, since this issue is closed (with the changes released in 6.1.9), can you please create a new issue to report the regression you are experiencing?

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) type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

5 participants