Fix invalid proxy setting when the port ends with a whitespace

[IvanBaricic]

Scenario:

A user configures the proxy address (for example in application.properties, .env file etc) and there is a whitespace at the end of the settings like here:

'quarkus.rest-client.proxy-address=localhost:8080 '

Error:

After splitting the proxy address, part with the port is a string value like "8080 " and Integer.parseInt("8080 ") will throw a NumberFormatException:

Invalid proxy setting. The port is not a number in 'localhost:8080 '

[gsmet]

Thanks for the pull request! That's indeed something that needs fixing.

Actually, what we should do is fix the config property using @WithConverter(TrimmedStringConverter.class) instead. That way it's fixed at the source once and for all and we don't have to think about trimming it at all.

There might be other properties there for which it makes sense to use it.

Would you be interested in adjusting your patch to do that instead?

[IvanBaricic]

Thanks for your hint! I have added this annotation to proxyAddress configuration property. I personally find only this one to be useful, .. due to parsing a number from the string.

This is my very first commitment to Quarkus framework, .. so I hope, I am done it properly.

[gastaldi]

LGTM, thanks for the PR!

[gsmet]

Sorry, I had an hesitation before approving and I checked in our code base and for the new @ConfigMapping config framework, the annotation should be added in the container instead of on the property.

So if you can just make this tiny change, check that everything is OK on your side with this change, and amend your commit, that would be awesome!

[gsmet]

Suggested change

	@WithConverter(TrimmedStringConverter.class)
	Optional<String> proxyAddress();
	Optional<@WithConverter(TrimmedStringConverter.class) String> proxyAddress();

[IvanBaricic]

Thanks for your hint, .. I am sorry, I am too late, .. geoand has already changed it and merge the pull request into quarkusio:main. Great! If I look after the same annotation & Optional, I find 38x occurrences in the codebase; so theoretical I could refactor them to use the same approach everywhere, could not I.

[github-actions]

🎊 PR Preview 3ac8d23 has been successfully built and deployed to https://quarkus-pr-main-44708-preview.surge.sh/version/main/guides/

• Images of blog posts older than 3 months are not available.
• Newsletters older than 3 months are not available.

[gastaldi]

Nice catch, also, here is a real example using it:

quarkus/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcTenantConfig.java

Line 679 in 3f10b51

Optional<@WithConverter(TrimmedStringConverter.class) List<String>> forwardParams();

Waiting for the change above

[geoand]

Thanks for this!

I updated the PR so it should be good to go now

comment addressed

[quarkus-bot]

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 2654019.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 2654019.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.