Hi my team has encountered an issue with the latest version of spring-core and unfortunately we don't have a fix but we can replicate it with the sample project below. Let me outline the problem.
Given application.properties like so
gsm\://one=result
key-one=one
key-two=${gsm://${key-one}}
key-three=${gsm://one}
It appears spring-core 6.1.14 would translate property keys key-two and key-three to be result but when upgrading to 6.2.1 it appears the output of these properties have changed. Unfortunately this is causing a breaking change for us and I believe it's probably a bug in spring-core. We narrowed it down to changes in how placeholders are resolved in PlaceholderParse but were unsuccessful in coming up with a reasonable fix.
In addition to the properties above we tried escaping the : as that's a Spring separator but that doesn't seem to fix the issue. The test.zip contains a test which if you switch between spring framework 6.1.14 and 6.2.1 will show the problem.
Please let me know if you have additional questions.
Comment From: bclozel
Duplicates https://github.com/spring-projects/spring-framework/issues/34124
Comment From: mbazos
Thanks @bclozel I was searching for issues to see if they were related and didn't find anything. Looks like this is going to be fixed next week with Spring Framework 6.2.2.
Comment From: snicoll
In addition to the properties above we tried escaping the : as that's a Spring separator but that doesn't seem to fix the issue.
The escaping should happen in the placehoder itself (i.e. ${gsm\://one} would resolve the gsm://one key). The key itself shouldn't be escaped since it isn't a placeholder.
While this is going to be "fixed" for backward compatible reason, such usage is strongly discouraged. Do not use a reserved character in your property key, we may revisit this in the future so that it doesn't work in a new major release.
Comment From: mbazos
Thanks @snicoll I am going to talk to my team based on this guidance I think we will change the placeholder from ${gsm\://one} to ${gsm//one} which removed the reserved keyword.
I assume this could be stricter in future versions of spring-framework or even removed in a major version upgrade.
Comment From: mbazos
@snicoll We are working to remove the reserved keyword : form our property placeholder lookup, but was curious why was this changed/broken in spring-core? I know regardless we needed to escape the : but was this to fix a bug? is it due to security reasons or something else?
Just looking for an explanation why this is happening so I can properly communicate that to my colleagues. Spring has been great over the years for us and breaking changes like this almost never happen unless there is a major upgrade and it's usually documented clearly.
Also I was curious and did test this with Spring Framework 6.2.2 with a property like this:
${gsm\\://${key-one}}
This does not seem to work, but this does work in 6.2.2 whereas in 6.2.1 this was broken:
${gsm\\://one}
Either way we are getting rid of the reserved keyword : just wanted to know if there is more to this change then it being a reserved keyword.
Comment From: snicoll
why was this changed/broken in spring-core?
It wasn't changed on purpose if that's what you are asking. In 6.2 the parser was completely rewritten to only attempt to resolve a part when needed. Previously we had weird bugs where if you had something like ${key:my ${fallback} value} it would attempt to resolve fallback even if keywas present. Another weird behavior is the one that I missed (trying the full placeholder content even if it is a valid placeholder structure before parsing it). We unfortunately had no test for it so I suspect this worked by accident, and so your use of, e.g. ${gsm://one}.
Ironically enough, the escape feature was added in the new parser in case someone really has to use : in the key!
Spring has been great over the years for us and breaking changes like this almost never happen unless there is a major upgrade and it's usually documented clearly.
What are you talking about? The issue has been reported and fixed within a month. Or are you saying it doesn't work for you?
As for your example, I am not sure what you mean. You can create a new issue with a demo if you think that escaping no longer works in 6.2.2
Comment From: mbazos
What are you talking about? The issue has been reported and fixed within a month. Or are you saying it doesn't work for you?
@snicoll Just saying over my 17+ years of using Spring these kinds of issues almost never happen. You all did a great job fixing it within a very timely manner, but I did test with Spring 6.2.2 and I think it's still an issue there. Please take a look at my zip.
6.1.14 works fine for both unit tests (remove escaping from properties because it's not required)
6.2.2 fails for this
org.opentest4j.AssertionFailedError:
Expected :result
Actual ://one
We are asking all the teams to change from gsm:// to gsm// which works fine with all version but wanted to be clear something is still broken in Spring 6.2.2 if the goal was to maintain full backwards compatibility.
Comment From: snicoll
Just saying over my 17+ years of using Spring these kinds of issues almost never happen.
I am not sure what that means or why you bring this up. To me, this can only be read as a criticism or a passive aggressive statement. Let's keep in mind that your use of Spring was incorrect to begin with: placeholders use : to separate the key from the fallback for as long as I can remember (2009 by the looks of it when configured in plain Spring Framework, a bit later with Spring Boot doing this automatically).
but I did test with Spring 6.2.2 and I think it's still an issue there. Please take a look at my zip.
Backslash in properties file (like in a String in Java) needs to be doubled. This should be changed to key-two=${gsm\\://${key-one}}.
but wanted to be clear something is still broken in Spring 6.2.2 if the goal was to maintain full backwards compatibility.
Full backward compatibility means the use of : as part of the key would still work and that's impossible to do while fixing legitimate bugs. That's why the escaping has been introduced.
Comment From: mbazos
Thanks @snicoll for the explanation.
I didn't mean any criticism and was not being passive aggressive. My apologies if it came off like that. What I meant by it really is how great of a job you and your team have been doing over the years managing Spring with such a large community. I really do appreciate it.