You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by mg...@apache.org on 2019/10/09 13:07:20 UTC
[wicket] branch wicket-7.x updated: WICKET-6704
JavaSerializer.serialize causes the JVM crash ! (#385)
This is an automated email from the ASF dual-hosted git repository.
mgrigorov pushed a commit to branch wicket-7.x
in repository https://gitbox.apache.org/repos/asf/wicket.git
The following commit(s) were added to refs/heads/wicket-7.x by this push:
new 0fc1b2a WICKET-6704 JavaSerializer.serialize causes the JVM crash ! (#385)
0fc1b2a is described below
commit 0fc1b2ad814ad360cf0ca13ead225110fb10131b
Author: Martin Grigorov <ma...@users.noreply.github.com>
AuthorDate: Wed Oct 9 16:06:17 2019 +0300
WICKET-6704 JavaSerializer.serialize causes the JVM crash ! (#385)
* WICKET-6704 JavaSerializer.serialize causes the JVM crash !
Do not check instances of PropertyChangeSupport whether they are Serializable because PropertyChangeSupport#writeObject() adds extra fields which confuse CheckingObjectOutputStream
* WICKET-6704 Add more special types which use ObjectOutputStream.PutField in their writeObject() method
(cherry picked from commit 4ff101fca5fac22dfab9dda4dcfe9075247ee46a)
---
.../checker/CheckingObjectOutputStream.java | 63 ++++++++++++++--
...bjectOutputStreamPropertyChangeSupportTest.java | 84 ++++++++++++++++++++++
2 files changed, 140 insertions(+), 7 deletions(-)
diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/CheckingObjectOutputStream.java b/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/CheckingObjectOutputStream.java
index 4978c0e..c0f8b7c 100644
--- a/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/CheckingObjectOutputStream.java
+++ b/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/CheckingObjectOutputStream.java
@@ -16,6 +16,9 @@
*/
package org.apache.wicket.core.util.objects.checker;
+import javax.security.auth.Subject;
+import java.beans.PropertyChangeSupport;
+import java.beans.VetoableChangeSupport;
import java.io.Externalizable;
import java.io.IOException;
import java.io.ObjectOutput;
@@ -27,13 +30,23 @@ import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
+import java.net.InetAddress;
+import java.net.SocketAddress;
+import java.security.Permission;
+import java.security.Permissions;
+import java.util.BitSet;
import java.util.Date;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.LinkedList;
+import java.util.Locale;
import java.util.Map;
+import java.util.Random;
import java.util.Set;
+import java.util.Vector;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ThreadLocalRandom;
import org.apache.wicket.Component;
import org.apache.wicket.WicketRuntimeException;
@@ -393,10 +406,12 @@ public class CheckingObjectOutputStream extends ObjectOutputStream
Object[] objs = (Object[])obj;
for (int i = 0; i < objs.length; i++)
{
- CharSequence arrayPos = new StringBuilder(4).append('[').append(i).append(']');
- simpleName = arrayPos;
- fieldDescription += arrayPos;
- check(objs[i]);
+ if (!isKnownToBeSerializable(objs[i])) {
+ CharSequence arrayPos = new StringBuilder(4).append('[').append(i).append(']');
+ simpleName = arrayPos;
+ fieldDescription += arrayPos;
+ check(objs[i]);
+ }
}
}
}
@@ -564,9 +579,7 @@ public class CheckingObjectOutputStream extends ObjectOutputStream
}
for (int i = 0; i < objVals.length; i++)
{
- if (objVals[i] instanceof String || objVals[i] instanceof Number ||
- objVals[i] instanceof Date || objVals[i] instanceof Boolean ||
- objVals[i] instanceof Class)
+ if (isKnownToBeSerializable(objVals[i]))
{
// filter out common cases
continue;
@@ -596,6 +609,42 @@ public class CheckingObjectOutputStream extends ObjectOutputStream
}
}
+ private boolean isKnownToBeSerializable(Object obj) {
+ return isCommonClass(obj) || hasCustomSerialization(obj);
+ }
+
+ private boolean isCommonClass(Object obj) {
+ return obj instanceof String || obj instanceof Number ||
+ obj instanceof Date || obj instanceof Boolean ||
+ obj instanceof Class || obj instanceof Throwable;
+ }
+
+ /**
+ * Some classes use {@link ObjectOutputStream.PutField} in their implementation of
+ * <em>private void writeObject(ObjectOutputStream s) throws IOException</em> and this
+ * (sometimes) breaks the introspection done by this class, and even crashes the JVM!
+ *
+ * @see <a href="https://issues.apache.org/jira/browse/WICKET-6704">WICKET-6704</a>
+ * @param obj The object to check
+ * @return {@code true} if the object type is one of these special ones
+ */
+ private boolean hasCustomSerialization(Object obj) {
+ return obj instanceof PropertyChangeSupport ||
+ obj instanceof VetoableChangeSupport ||
+ obj instanceof Permission ||
+ obj instanceof Permissions ||
+ obj instanceof BitSet ||
+ obj instanceof ConcurrentHashMap ||
+ obj instanceof Vector ||
+ obj instanceof InetAddress ||
+ obj instanceof SocketAddress ||
+ obj instanceof Locale ||
+ obj instanceof Random ||
+ obj instanceof ThreadLocalRandom ||
+ obj instanceof StringBuffer ||
+ obj instanceof Subject;
+ }
+
/**
* @return name from root to current node concatenated with slashes
*/
diff --git a/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/CheckingObjectOutputStreamPropertyChangeSupportTest.java b/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/CheckingObjectOutputStreamPropertyChangeSupportTest.java
new file mode 100644
index 0000000..a81eed2
--- /dev/null
+++ b/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/CheckingObjectOutputStreamPropertyChangeSupportTest.java
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.core.util.objects.checker;
+
+import java.beans.PropertyChangeSupport;
+import java.io.Serializable;
+import java.util.concurrent.Callable;
+import java.util.concurrent.Future;
+import java.util.concurrent.FutureTask;
+
+import org.apache.wicket.serialize.java.JavaSerializer;
+import org.junit.Test;
+
+/**
+ * Test for https://issues.apache.org/jira/browse/WICKET-6704
+ */
+public class CheckingObjectOutputStreamPropertyChangeSupportTest {
+
+ /**
+ * The test should either pass and log an ERROR
+ * or cause a JVM crash
+ */
+ @Test
+ public void serializePropertyChangeSupport()
+ {
+ JavaSerializer serializer = new JavaSerializer("test");
+ serializer.serialize(new ObjectToPersist());
+ }
+
+ static abstract class AbstractObjectToPersist implements Serializable {
+
+ private static final long serialVersionUID = 1L;
+
+ // if we move this field to the child class, the JVM crash is not reproducible, weird !
+ private PropertyChangeSupport propertyChangeSupport;
+
+ protected AbstractObjectToPersist() {
+ super();
+ // if we use PropertyChangeSupport directly, the JVM crash is not reproducible, weird !
+ propertyChangeSupport = new ExtendedPropertyChangeSupport(this);
+ }
+
+ }
+
+ static class ExtendedPropertyChangeSupport extends PropertyChangeSupport {
+
+ ExtendedPropertyChangeSupport(Object sourceBean) {
+ super(sourceBean);
+ }
+
+ }
+
+ class ObjectToPersist extends AbstractObjectToPersist {
+
+ // 1. this field is INTENTIONALLY not serializable to be able to trigger JVM crash
+ // 2. normally wicket handle this correctly by throwing the NotSerializableException, but in this example the JVM crash
+ private Future<Object> future;
+
+ ObjectToPersist() {
+ super();
+
+ future = new FutureTask<Object>(new Callable() {
+ public Object call() throws Exception {
+ return new Object();
+ }
+ });
+ }
+ }
+
+}