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 <marti...@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(); + } + }); + } + } + +}