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

Update commons-lang3 to 3.17.0 #42740

Merged
merged 1 commit into from
Aug 30, 2024
Merged

Conversation

manofthepeace
Copy link
Contributor

@manofthepeace manofthepeace commented Aug 23, 2024

Draft until 3.17.0 actually exists.

Fixes: #42686

Looks like the dust settled on this and 3.17 will be OK

When compared to 3.14, 3 new singleton instances are available;

Random*Utils.secure() that uses SecureRandom()
Random*Utils.secureStrong() that uses SecureRandom.getInstanceStrong()
Random*Utils.insecure() that uses ThreadLocalRandom as 3.14 was using

This PR uses insecure() to have the same behaviour as 3.14.

I personally do not think it is worthwhile to get rid of RandomStringUtils because a mistake happened in the lib, these things happen.

@quarkus-bot quarkus-bot bot added area/core area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/testing labels Aug 23, 2024
@famod
Copy link
Member

famod commented Aug 26, 2024

Thanks for taking an early stab at this.

AFAICS, this will have to wait (also) for Liquibase 4.29.2 to land in Quarkus: liquibase/liquibase#6179

Furthermore, there is still the potential issue of other libs not using the new singleton instances / factory methods yet.

@gsmet
Copy link
Member

gsmet commented Aug 26, 2024

Yeah, so just to be clear: this won't be for either 3.14 or 3.15.

But we can merge in main as it's going to be for 3.16 as soon as the Liquibase update lands.

@manofthepeace
Copy link
Contributor Author

manofthepeace commented Aug 26, 2024

At least in 3.17, using Random*Utils.random() will use secure and not secureStrong so it should not be draining the system's entropy pool. Might not play well with native though.

@famod
Copy link
Member

famod commented Aug 27, 2024

@manofthepeace so they also restored all the old >= 3.14.0 methods to not use the "strong" approach?
If that's the case we could merge earlier, but then again there is no hurry and better to be save than sorry.

@manofthepeace manofthepeace force-pushed the bumpCommonsLang3 branch 2 times, most recently from 8fc52d1 to 8b0d72f Compare August 29, 2024 23:12
@gsmet gsmet marked this pull request as ready for review August 30, 2024 06:39
@gsmet
Copy link
Member

gsmet commented Aug 30, 2024

I think we can get this in main, marked as ready.

@quarkus-bot
Copy link

quarkus-bot bot commented Aug 30, 2024

Status for workflow Quarkus CI

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

✅ 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.

@gsmet gsmet merged commit 9ce1059 into quarkusio:main Aug 30, 2024
67 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.16 - main milestone Aug 30, 2024
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Aug 30, 2024
@manofthepeace manofthepeace deleted the bumpCommonsLang3 branch September 4, 2024 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/testing kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update commons-lang3 to > 3.14.0 eventually
3 participants