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

Removes support for EventData #2076

Merged
merged 3 commits into from
Dec 28, 2023
Merged

Conversation

ppkarwasz
Copy link
Contributor

The org.slf4j.ext.EventData class was a primitive equivalent of Log4j's StructuredDataMessage. QOS.CH published CVE-2018-8088 against this class and removed it from slf4j-ext version 1.7.26.

For a long time we kept support code to convert EventData to its Log4j equivalent for compatibility's sake: users that relied on this class could still use Log4j Core, but the functionality was automatically disabled once they upgraded slf4j-ext.

Since 5 years passed since slfj4-ext 1.7.26 was published, we can safely assume that all the users upgraded the library and the support code can be removed.

@ppkarwasz
Copy link
Contributor Author

@rgoers,

I have two doubts about the support for slfj4-ext:

  • should I backport this change to 2.x?
  • once I remove the support for EventData, log4j-slf4j-impl has no code specifically written for slf4j-ext. Should I remove the tests that use XLogger too? The way I see it these tests can only discover errors in slf4j-ext itself, not our own code.

@ppkarwasz ppkarwasz added this to the 3.0.0 milestone Dec 8, 2023
@ppkarwasz ppkarwasz requested a review from rgoers December 8, 2023 10:14
@vy vy self-requested a review December 8, 2023 10:25
@vy vy added bug Incorrect, unexpected, or unintended behavior of existing code slf4j Affects SLF4J integration labels Dec 8, 2023
@vy
Copy link
Member

vy commented Dec 8, 2023

@ppkarwasz, LGTM.

I support the idea of removing XLogger tests too and backporting all this to 2.x.

@ppkarwasz ppkarwasz modified the milestones: 3.0.0, 2.22.1, 2.23.0 Dec 11, 2023
The `org.slf4j.ext.EventData` class was a primitive equivalent of
Log4j's `StructuredDataMessage`. QOS.CH published CVE-2018-8088 against
this class and removed it from `slf4j-ext` version 1.7.26.

For a long time we kept support code to convert `EventData` to its Log4j
equivalent for compatibility's sake: users that relied on this class
could still use Log4j Core, but the functionality was **automatically**
disabled once they upgraded `slf4j-ext`.

Since 5 years passed since `slfj4-ext` 1.7.26 was published, we can
safely assume that all the users upgraded the library and the support
code can be removed.
After the removal of `EventData`, the only usage of `slf4j-ext` was in
tests.

However, since all `slf4j-ext` method calls delegate to `slfj4-api`
method calls there is no sense in testing it directly.
@ppkarwasz ppkarwasz merged commit 7385605 into apache:main Dec 28, 2023
7 checks passed
@ppkarwasz ppkarwasz deleted the remove-event-data branch January 2, 2024 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect, unexpected, or unintended behavior of existing code slf4j Affects SLF4J integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants