FreeAndNil commented on code in PR #4365:
URL: https://github.com/apache/logging-log4j2/pull/4365#discussion_r4137444081
##########
log4j-api-test/src/test/java/org/apache/logging/log4j/util/internal/SerializationUtilTest.java:
##########
@@ -51,4 +60,30 @@ void stripArrayClass(final Class<?> arrayClass, final
Class<?> componentClazz) {
void stripArrayString(final Class<?> arrayClass, final Class<?>
componentClazz) {
assertThat(SerializationUtil.stripArray(arrayClass.getName())).isEqualTo(componentClazz.getName());
}
+
+ static final class Payload implements Serializable {
+ private static final long serialVersionUID = 1L;
+ }
+
+ @Test
+ @EnabledForJreRange(min = JRE.JAVA_9)
+ void readWrappedObjectUsesObjectInputFilterOfOuterStream() throws
Exception {
+ final ByteArrayOutputStream bout = new ByteArrayOutputStream();
+ try (final ObjectOutputStream out = new ObjectOutputStream(bout)) {
+ out.writeObject(new ObjectMessage(new Payload()));
+ }
+ final byte[] data = bout.toByteArray();
+
+ final ObjectInputStream unfiltered = new ObjectInputStream(new
ByteArrayInputStream(data));
+ assertThat(((ObjectMessage)
unfiltered.readObject()).getParameter()).isInstanceOf(Payload.class);
+
+ // Java 9 API accessed through reflection, since this module targets
Java 8
+ final Class<?> filterClass =
Class.forName("java.io.ObjectInputFilter");
+ final Object filter = Class.forName("java.io.ObjectInputFilter$Config")
+ .getMethod("createFilter", String.class)
+ .invoke(null, "!" + Payload.class.getName());
+ final ObjectInputStream filtered = new ObjectInputStream(new
ByteArrayInputStream(data));
+ ObjectInputStream.class.getMethod("setObjectInputFilter",
filterClass).invoke(filtered, filter);
+ assertThat(((ObjectMessage)
filtered.readObject()).getParameter()).isNull();
Review Comment:
**Testability**: This test also passes on the code before this PR, so it
pins neither of the two behaviour changes.
`Payload` lives in `org.apache.logging.log4j.`, which the removed
`DefaultObjectInputFilter` allowed anyway, so the unfiltered read returned a
`Payload` before as well. The filtered read returned `null` before too, because
`DefaultObjectInputFilter` asked its delegate, the outer filter, first. Two
cases would fail on the old code: a nested class outside the allowlist, say
`java.net.URI`, read through a plain `ObjectInputStream` (old: rejected,
`null`; new: the `URI`), and the `!Payload` filter set on a
`FilteredObjectInputStream` (old: `Payload`, since the nested stream got no
filter; new: `null`). The second one is the path the new
`copyObjectInputFilter` call adds and nothing covers it yet.
Something like the following would pin both. It needs `java.io.IOException`,
`java.net.URI` and `org.apache.logging.log4j.util.FilteredObjectInputStream`
imported, and replaces everything from `Payload` on. The first two tests fail
on the code before this PR and pass after it (checked on JDK 17); the third is
the existing test and passes on both.
```java
static final class Payload implements Serializable {
private static final long serialVersionUID = 1L;
}
@Test
void readWrappedObjectAddsNoAllowlistOfItsOwn() throws Exception {
// `java.net` is outside the allowlist of `FilteredObjectInputStream`
final URI uri = URI.create("https://logging.apache.org/");
final byte[] data = serialize(new ObjectMessage(uri));
assertThat(readParameter(new ObjectInputStream(new
ByteArrayInputStream(data))))
.isEqualTo(uri);
}
@Test
@EnabledForJreRange(min = JRE.JAVA_9)
void readWrappedObjectUsesObjectInputFilterOfOuterStream() throws
Exception {
final byte[] data = serialize(new ObjectMessage(new Payload()));
final ObjectInputStream in = new ObjectInputStream(new
ByteArrayInputStream(data));
setObjectInputFilter(in, "!" + Payload.class.getName());
assertThat(readParameter(in)).isNull();
}
@Test
@EnabledForJreRange(min = JRE.JAVA_9)
@SuppressWarnings("deprecation")
void readWrappedObjectUsesObjectInputFilterOfOuterFilteredStream()
throws Exception {
// `Payload` is in `org.apache.logging.log4j.`, so only the filter
can reject it
final byte[] data = serialize(new ObjectMessage(new Payload()));
final ObjectInputStream in = new FilteredObjectInputStream(new
ByteArrayInputStream(data));
setObjectInputFilter(in, "!" + Payload.class.getName());
assertThat(readParameter(in)).isNull();
}
private static byte[] serialize(final Serializable obj) throws
IOException {
final ByteArrayOutputStream bout = new ByteArrayOutputStream();
try (final ObjectOutputStream out = new ObjectOutputStream(bout)) {
out.writeObject(obj);
}
return bout.toByteArray();
}
private static Object readParameter(final ObjectInputStream in) throws
IOException, ClassNotFoundException {
return ((ObjectMessage) in.readObject()).getParameter();
}
/**
* Java 9 API accessed through reflection, since this module targets
Java 8.
*/
private static void setObjectInputFilter(final ObjectInputStream in,
final String pattern) throws Exception {
final Class<?> filterClass =
Class.forName("java.io.ObjectInputFilter");
final Object filter =
Class.forName("java.io.ObjectInputFilter$Config")
.getMethod("createFilter", String.class)
.invoke(null, pattern);
ObjectInputStream.class.getMethod("setObjectInputFilter",
filterClass).invoke(in, filter);
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]