Skip to content

Exclude commons-configuration - #4468

Closed
ph33rtehgd wants to merge 1 commit into
spring-cloud:mainfrom
ph33rtehgd:ph33rtehgd-commons-configuration
Closed

Exclude commons-configuration#4468
ph33rtehgd wants to merge 1 commit into
spring-cloud:mainfrom
ph33rtehgd:ph33rtehgd-commons-configuration

Conversation

@ph33rtehgd

@ph33rtehgd ph33rtehgd commented Jul 31, 2025

Copy link
Copy Markdown

This PR is meant to primarily resolve this issue: #4446

My comments in the issue give some more detailed analysis, but from what I see is that while commons-configuration is used by eureka-client there isn't a flow through Spring Cloud Netflix that actually utilizes any classes from eureka-client that will actually utilize commons-configuration. Given this dependency's age, the fact that it's EOL and the fact the Netflix very likely won't be updating this I believe the most prudent thing to do is exclude this dependency as it should not negatively impact this Spring module.

It should also resolve this issue: #4457

I believe this should also be merged into the 4.2.x and 4.3.x branches. I can raise PRs for those too, if that's OK.

Signed-off-by: ph33rtehgd <geedude@gmail.com>
@spencergibb

Copy link
Copy Markdown
Member

Typically you'd create the PR against the lowest version branch and we would merge it forward. See also Netflix/eureka#1601

@spencergibb

Copy link
Copy Markdown
Member

Unfortunately, running the build reveals

 Caused by: java.lang.IllegalStateException: java.lang.NoClassDefFoundError: org/apache/commons/lang/exception/ExceptionUtils

So this will require a new release of eureka with my merged PR mentioned above

@ph33rtehgd

ph33rtehgd commented Aug 1, 2025

Copy link
Copy Markdown
Author

@spencergibb Got it, apologies, this is the first time I've raised a PR against an open source project, I thought it might actually go the other way :) (master down to older branches). Do you want me to close this PR and open a new one starting at 4.2.x (or if there's a different branch that's lower and still supported I can start there)?

As for the commons-lang change to no longer require ExceptionUtils in Eureka is needed. When I was testing out my change locally I had the latest Netflix 2.x branch checked out i n my workspace that already contained your PR merged locally so I didn't notice it. Hopefully since your change is minor on Eureka that can get released in the near future and then we can merge this change.

@cdprete

cdprete commented Sep 5, 2025

Copy link
Copy Markdown

Unfortunately, running the build reveals

 Caused by: java.lang.IllegalStateException: java.lang.NoClassDefFoundError: org/apache/commons/lang/exception/ExceptionUtils

So this will require a new release of eureka with my merged PR mentioned above

There is indeed no fix for it from the Spring Cloud Netflix project.

I tried the same where I work, but then I stumbled on the same issue and I had to add commons-lang as dependency which is also vulnerable.
The only solution I think may work is to, if Eureka itself can't be touched, during the build of spring-cloud-netflix-eureka, is to:

  1. download the sources of commons-lang
  2. unpack them
  3. delete all the sources which are not needed (i.e.: keep only this ExceptionUtils ideally)
  4. attach the new sources as additional sources to the build
  5. package the JAR with the commons-configuration dependency excluded

@spencergibb

Copy link
Copy Markdown
Member

This PR has been merged and we just need a release of eureka Netflix/eureka#1601

@cdprete

cdprete commented Sep 5, 2025

Copy link
Copy Markdown

This PR has been merged and we just need a release of eureka Netflix/eureka#1601

Just out of curiosity, why the string interpolation there has been replaced with String.format?
Like this, all the benefits to not evaluate the parameters if the used logging level is disabled are lost.

@spencergibb

Copy link
Copy Markdown
Member

Because the other doesn't take an exception as an argument

@cdprete

cdprete commented Sep 5, 2025

Copy link
Copy Markdown

Because the other doesn't take an exception as an argument

Ehm, am I missing something?
See https://www.slf4j.org/faq.html#paramException

@spencergibb

Copy link
Copy Markdown
Member

TIL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants