Skip to content

Revamp Log4j tests #12304

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

Merged
merged 15 commits into from
Aug 12, 2024
Merged

Revamp Log4j tests #12304

merged 15 commits into from
Aug 12, 2024

Conversation

vy
Copy link
Contributor

@vy vy commented Aug 2, 2024

I am an Apache Logging Services (the Apache Software Foundation project responsible for Log4j, Log4cxx, Log4net, etc. subprojects) PMC member – see the team. This PR revamps the Log4j tests with following changes:

  • Replaced existing tests1 with new, improved ones in the fuzzing branch2 of the official Log4j repository. This will not only ensure that tests will be maintained as a part of the official project, also enables maintainers to introduce new tests, make changes, etc. without touching the OSS-Fuzz code base.

  • Introduced JSON dictionary and seed corpus for fuzzing JSON-related Log4j components

  • Rewrote Dockerfile to match the JDK version to the one used by the official Log4j project.

  • Delegated build.sh to the one provided in the official Log4j distribution. This enables maintainers to introduce new tests, make changes, etc. without touching the OSS-Fuzz code base.

  • Rewrote project.yaml such that

    • Interested PMC members were added as primary_contact
    • Removed existing vendor_ccs, since found vulnerabilities should not be disclosed to individuals outside the PMC
  • Renamed log4j2 project folder to apache-logging-log4j2 to comply with the naming convention followed by other ASF projects integrated with OSS-Fuzz

1 Existing tests were not even compiling anymore due to bit rot.
2 I choose using a dedicated branch to carry out the development. Once I have a ClusterFuzz account and see things moving, I will merge the fuzzing branch to 2.x, the branch serving Log4j 2 releases.

Copy link

google-cla bot commented Aug 2, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link

github-actions bot commented Aug 2, 2024

vy is a new contributor to projects/log4j2. The PR must be approved by known contributors before it can be merged. The past contributors are: fmeum, henryrneh, aschaich, 0roman
vy is integrating a new project:
- Main repo: https://github.com/apache/logging-log4j2
- Criticality score: N/A

Copy link
Contributor

@fmeum fmeum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I also looked at the fuzz tests on the fuzzing branch in the log4j repo and they look much, much better than the basic tests I added as part of the initial integration. Great work! You may want to apply for an integration reward.

@vy vy requested a review from fmeum August 2, 2024 15:19
@vy
Copy link
Contributor Author

vy commented Aug 6, 2024

Hey @DavidKorczynski! Thanks for triggering the CI. There, out of 11, only the build (libfuzzer, address, x86_64) check has failed, but the error message doesn't make much sense to me. Do you mind helping out, please?

@vy vy requested a review from fmeum August 9, 2024 08:11
Copy link
Contributor

@fmeum fmeum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DavidKorczynski Could you review for approval?

Copy link
Collaborator

@DavidKorczynski DavidKorczynski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @vy and @fmeum !!

Copy link
Collaborator

@DavidKorczynski DavidKorczynski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed log4j2 project folder to apache-logging-log4j2 to comply with the naming convention followed by other ASF projects integrated with OSS-Fuzz

Could we avoid doing this please, and keep it in the same folder? Due to the backend running OSS-Fuzz we prefer to not change the name

@DavidKorczynski DavidKorczynski merged commit f6878f4 into google:master Aug 12, 2024
15 checks passed
@vy vy mentioned this pull request Aug 14, 2024
@vy vy deleted the log4j branch August 15, 2024 09:39
jonathanmetzman pushed a commit that referenced this pull request Sep 18, 2024
In #12304, we used `fuzzing` branch of the `apache/logging-log4j2`
repository while developing the Log4j 2 integration. This work was
successful and we eventually merged the `fuzzing` branch to
`2.x`<sup>1</sup> in apache/logging-log4j2#2949. Now we can point
OSS-Fuzz to the permanent location of the Log4j 2 fuzz tests.

<sup>1</sup> [`2.x` is the main branch where Log4j 2 development takes
place.](https://logging.apache.org/log4j/2.x/development.html#branching)
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

Successfully merging this pull request may close these issues.

3 participants