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

Reply via email to