-
Notifications
You must be signed in to change notification settings - Fork 754
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
Ignore unresolved property placeholders in url
#1022
base: main
Are you sure you want to change the base?
Ignore unresolved property placeholders in url
#1022
Conversation
What exception is thrown if the placeholder is not ignored? |
It doesn't ignore them, it just doesn't prepend |
hi @spencergibb,
Yes, what I meant by if (!url.contains("://")) {
url = "http://" + url;
}
try {
new URL(url);
}
catch (MalformedURLException e) {
throw new IllegalArgumentException(url + " is malformed", e);
} |
Why isn't that preferred, to error early rather than later? |
I actually would prefer that too but I would expect the behaviour to be consistent between SpEL and placeholders. |
hey @spencergibb @OlgaMaciaszek, |
fixes #1023
FeignClientRegistar
seems to purposefully ignore unresolved URLs that are SpEL but doesn't do so for property placeholders like${placeholder}
.For ignored SpEL strings, the input string is returned, but for placeholders, it tries to parse it into a
URL
object.Up until JDK 20, this would be OK and would not fail - and unresolved placeholder URL would probably fail somewhere later down the flow. This is because the underlying implementation of the
URL
class doesn't actually try to parse the host until unless theURL.openConnection()
is called, which is not called in this method.Since JDK 20, this behaviour has changed and the underlying implementation of the
URL
class now parses the host eagerly without waiting for aURL.openConnection()
call.This was discovered when our tests that use @JsonTest started failing once we upgraded to JDK 20+