Skip to content

[LOG4J2-3680] Allow deserialization of arrays #2259

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 2 commits into from
Feb 5, 2024

Conversation

ppkarwasz
Copy link
Contributor

The DefaultObjectInputFilter is too restrictive and it does not allow the deserialization of arrays of whitelisted classes.

Most notably RuntimeException can not be deserialized, since it contains an array of StackTraceElements. Both classes are allowed, but deserialization of the StackTraceElement[] array fails.

We also replace all manual serializations/deserializations in test classes to calls to SerialUtil. This guarantees a consistent behavior of all tests under Java 8 and Java 9+.

@ppkarwasz ppkarwasz added this to the 2.23.0 milestone Feb 2, 2024
@ppkarwasz ppkarwasz self-assigned this Feb 2, 2024
Copy link
Member

@vy vy left a comment

Choose a reason for hiding this comment

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

LGTM. But I would have expected stripArray() to thrown an exception on unknown array type.

The `DefaultObjectInputFilter` is too restrictive and it does not allow
the deserialization of **arrays** of classes in the
`SerializationUtil#REQUIRED_JAVA_CLASSES` and
`SerializationUtil#REQUIRED_JAVA_PACKAGES` white lists.

Most notably `RuntimeException` can not be deserialized, since it
contains an array of `StackTraceElement`s. Both classes are allowed, but
deserialization of the array fails.
@ppkarwasz ppkarwasz force-pushed the fix/3680_serialize_arrays branch from 2b50e38 to 249a4f1 Compare February 5, 2024 21:09
@ppkarwasz ppkarwasz merged commit 249a4f1 into 2.x Feb 5, 2024
@ppkarwasz ppkarwasz deleted the fix/3680_serialize_arrays branch February 5, 2024 21:51
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.

2 participants