Skip to content

Use SLF4J instead of Log4J directly #32

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

Closed
ghost opened this issue Sep 26, 2019 · 15 comments
Closed

Use SLF4J instead of Log4J directly #32

ghost opened this issue Sep 26, 2019 · 15 comments
Milestone

Comments

@ghost
Copy link

ghost commented Sep 26, 2019

Having this depend specifically on Log4J can produce a weird situation when used in projects that use other logging frameworks (which is the problem SLF4J, or depending only on API jars, tackle).

As far as I could see, there isn't a lot of complex things going on here that would require direct access to some Log4J-specific features, so I would like to know if you'd be willing to migrate this to SLF4J (or Log4J2-API).

I can port it myself, but I wanted to know your positioning on the matter before forking and creating a PR

@cowtowncoder
Copy link
Owner

cowtowncoder commented Sep 27, 2019

Yes, makes sense. Use of log4j is due package predating slf4j.

But do note that log4j is "provided" dependency, and not used by default.
So what might make more sense is to add another backend implementation for slf4j to go with existing log4j and java.util.logging varieties?

@ghost
Copy link
Author

ghost commented Sep 27, 2019

In my opinion, given what SLF4J is, the best, I think, would just to depend on slf4j-api for compilation, use only SLF4J, and let the user use some SLF4J implementation/bridge/whatever to use the logging framework of their choice.

The whole point of SLF4J is to not depend directly on logger implementation and let the user choose

@ghost
Copy link
Author

ghost commented Sep 27, 2019

Even though the dependency is in provided scope, IIRC, it still has to be in the classpath for compiling projects that depend on it.

I either have to add log4j or log4j-over-slf4j at compile time for to my projects, for example.

I think fully moving to SLF4J is the way to go (but then release the change as a major, potentially breaking change, I believe).

Looking into some other usages of this project here on GitHub, I see many people are doing log4j -> slf4j -> their logging framework of choice (same thing I'm doing right now)

@andrebrait
Copy link

I deleted my other GitHub account. Contact this account if you need.

@hanson76
Copy link

Looks like there is a java modules problem with the log4j dependency.

The module-info.java file has 'requires log4j;'
That gives the following error when you try to add com.fasterxml.uuid as a java module.

"java.lang.module.FindException: Module log4j not found, required by com.fasterxml.uuid"

@andrebrait
Copy link

@hanson76 I'll test with jigsaw tomorrow. It makes sense to not require that anymore, also.

@andrebrait
Copy link

@hanson76 Could you test it with the latest version from my fork? See #33
@cowtowncoder I would like to know if the OSGi thing is important and if I did it correctly (I never did anything with OSGi before)

@hanson76
Copy link

Sorry for late answer but been busy.
I've now tested your fork and it works where 3.2.0 does not.

@andrebrait
Copy link

@hanson76 does that include log messages from this library?

@hanson76
Copy link

Yes

@hanson76
Copy link

@cowtowncoder Is there a plan to merge the fix in #33 and release a new version anytime soon?

@ndimitry
Copy link

ndimitry commented Feb 12, 2020

There one more good reason for the migration to SLF4J.
java-uuid-generator currently uses the log4j 1.2.17 dependency, which is vulnerable to https://nvd.nist.gov/vuln/detail/CVE-2019-17571, so migrating to another package which is able to use a newer version of log4j (i.e. 2.x) might be a good idea.

@andrebrait
Copy link

It seems there is a legitimate reason to migrate then, @cowtowncoder

@cowtowncoder
Copy link
Owner

That CVE may be a practical concern: from quick look it does not seem like actual security issue in itself or via JUG, but security tools whine incessantly about such dependency so as to make it a practical problem nonetheless (no, I am not big fan of current handling of CVEs as a maintainer).
Or put another way: dependency only becomes problem if a serialization framework enables deserialization of log4j types; in that case lack of log4j class can prevent that particular security hole. But there are literally dozens of others, a small number even in JDK itself.
Still, due to simplistic security evaluation tools this will no doubt be brought up again and again :-(

Having said that... My only concern so far has been that of trying to avoid adding strong dependency (i.e. being able to leave dependency as provided) -- possibly by actually simply dropping any implementation aside from JavaUtilLogger that relies on JDK's logger.

So perhaps the simplest and best course is to just move to using slf4j api; and since this is fundamental dependency change, increase major version to 4. I'll go over the PR and see if I some suggestions or questions.

@cowtowncoder cowtowncoder added this to the 4.0 milestone Feb 22, 2020
@cowtowncoder
Copy link
Owner

Implemented, released as new major version, 4.0, available via Maven Central.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants