WICKET-4812 Make SerializationChecker easier for extending so custom checks can be added to it
Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/6014d8bb Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/6014d8bb Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/6014d8bb Branch: refs/heads/master Commit: 6014d8bb92595bf486d69f7a5c1c798bcdc252f1 Parents: 40512f8 Author: Martin Tzvetanov Grigorov <[email protected]> Authored: Wed Oct 10 15:47:31 2012 +0300 Committer: Martin Tzvetanov Grigorov <[email protected]> Committed: Wed Oct 10 15:47:31 2012 +0300 ---------------------------------------------------------------------- .../wicket/core/util/io/SerializableChecker.java | 707 +------------- .../core/util/objects/checker/IObjectChecker.java | 94 ++ .../objects/checker/NotDetachedModelChecker.java | 28 + .../core/util/objects/checker/ObjectChecker.java | 739 +++++++++++++++ .../wicket/serialize/java/JavaSerializer.java | 64 ++- .../wicket/serialize/java/JavaSerializerTest.java | 85 ++ 6 files changed, 1050 insertions(+), 667 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/6014d8bb/wicket-core/src/main/java/org/apache/wicket/core/util/io/SerializableChecker.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/io/SerializableChecker.java b/wicket-core/src/main/java/org/apache/wicket/core/util/io/SerializableChecker.java index 57f8317..fb15cbc 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/io/SerializableChecker.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/io/SerializableChecker.java @@ -16,62 +16,34 @@ */ package org.apache.wicket.core.util.io; -import java.io.Externalizable; import java.io.IOException; import java.io.NotSerializableException; -import java.io.ObjectOutput; -import java.io.ObjectOutputStream; -import java.io.ObjectStreamClass; -import java.io.ObjectStreamField; -import java.io.OutputStream; import java.io.Serializable; -import java.lang.reflect.Field; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; import java.lang.reflect.Proxy; -import java.util.Date; -import java.util.HashSet; -import java.util.IdentityHashMap; -import java.util.Iterator; -import java.util.LinkedList; -import java.util.Map; -import java.util.Set; -import java.util.Stack; -import org.apache.wicket.Component; -import org.apache.wicket.WicketRuntimeException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.apache.wicket.core.util.objects.checker.IObjectChecker; +import org.apache.wicket.core.util.objects.checker.ObjectChecker; /** * Utility class that analyzes objects for non-serializable nodes. Construct, then call - * {@link #check(Object)} with the object you want to check. When a non-serializable object is + * {@link #writeObject(Object)} with the object you want to check. When a non-serializable object is * found, a {@link WicketNotSerializableException} is thrown with a message that shows the trace up * to the not-serializable object. The exception is thrown for the first non-serializable instance * it encounters, so multiple problems will not be shown. - * <p> - * As this class depends heavily on JDK's serialization internals using introspection, analyzing may - * not be possible, for instance when the runtime environment does not have sufficient rights to set - * fields accessible that would otherwise be hidden. You should call - * {@link SerializableChecker#isAvailable()} to see whether this class can operate properly. If it - * doesn't, you should fall back to e.g. re-throwing/ printing the {@link NotSerializableException} - * you probably got before using this class. - * </p> * * @author eelcohillenius * @author Al Maw */ -public final class SerializableChecker extends ObjectOutputStream +public class SerializableChecker extends ObjectChecker { - - /** log. */ - private static final Logger log = LoggerFactory.getLogger(SerializableChecker.class); - /** * Exception that is thrown when a non-serializable object was found. + * @deprecated Use ObjectChecker.WicketNotSerializableException instead */ - public static final class WicketNotSerializableException extends WicketRuntimeException + // TODO Wicket 7.0 - remove this class. It is here only for backward binary compatibility + @Deprecated + public static final class WicketNotSerializableException extends ObjectChecker.WicketNotSerializableException { private static final long serialVersionUID = 1L; @@ -82,247 +54,50 @@ public final class SerializableChecker extends ObjectOutputStream } /** - * Does absolutely nothing. + * An implementation of IObjectChecker that checks whether the object + * implements {@link Serializable} interface */ - private static class NoopOutputStream extends OutputStream - { - @Override - public void close() - { - } - - @Override - public void flush() - { - } - - @Override - public void write(byte[] b) - { - } - - @Override - public void write(byte[] b, int i, int l) - { - } - - @Override - public void write(int b) - { - } - } - - private static abstract class ObjectOutputAdaptor implements ObjectOutput + public static class ObjectSerializationChecker implements IObjectChecker { + /** Exception that should be set as the cause when throwing a new exception. */ + private final NotSerializableException cause; - @Override - public void close() throws IOException - { - } - - @Override - public void flush() throws IOException + /** + * A constructor to use when the checker is used before a previous attempt to + * serialize the object. + */ + public ObjectSerializationChecker() { + this(null); } - @Override - public void write(byte[] b) throws IOException + /** + * A constructor to use when there was a previous attempt to serialize the + * object and it failed with the {@code cause}. + * + * @param cause + * the cause of the serialization failure in a previous attempt. + */ + public ObjectSerializationChecker(NotSerializableException cause) { + this.cause = cause; } @Override - public void write(byte[] b, int off, int len) throws IOException + public Result check(Object object) { - } - - @Override - public void write(int b) throws IOException - { - } - - @Override - public void writeBoolean(boolean v) throws IOException - { - } - - @Override - public void writeByte(int v) throws IOException - { - } - - @Override - public void writeBytes(String s) throws IOException - { - } - - @Override - public void writeChar(int v) throws IOException - { - } - - @Override - public void writeChars(String s) throws IOException - { - } - - @Override - public void writeDouble(double v) throws IOException - { - } - - @Override - public void writeFloat(float v) throws IOException - { - } - - @Override - public void writeInt(int v) throws IOException - { - } - - @Override - public void writeLong(long v) throws IOException - { - } - - @Override - public void writeShort(int v) throws IOException - { - } - - @Override - public void writeUTF(String str) throws IOException - { - } - } - - /** Holds information about the field and the resulting object being traced. */ - private static final class TraceSlot - { - private final String fieldDescription; - - private final Object object; - - TraceSlot(Object object, String fieldDescription) - { - super(); - this.object = object; - this.fieldDescription = fieldDescription; - } - - @Override - public String toString() - { - return object.getClass() + " - " + fieldDescription; - } - } - - private static final NoopOutputStream DUMMY_OUTPUT_STREAM = new NoopOutputStream(); - - /** Whether we can execute the tests. If false, check will just return. */ - private static boolean available = true; - - // this hack - accessing the serialization API through introspection - is - // the only way to use Java serialization for our purposes without writing - // the whole thing from scratch (and even then, it would be limited). This - // way of working is of course fragile for internal API changes, but as we - // do an extra check on availability and we report when we can't use this - // introspection fu, we'll find out soon enough and clients on this class - // can fall back on Java's default exception for serialization errors (which - // sucks and is the main reason for this attempt). - private static Method LOOKUP_METHOD; - - private static Method GET_CLASS_DATA_LAYOUT_METHOD; - - private static Method GET_NUM_OBJ_FIELDS_METHOD; - - private static Method GET_OBJ_FIELD_VALUES_METHOD; - - private static Method GET_FIELD_METHOD; - - private static Method HAS_WRITE_REPLACE_METHOD_METHOD; - - private static Method INVOKE_WRITE_REPLACE_METHOD; - - static - { - try - { - LOOKUP_METHOD = ObjectStreamClass.class.getDeclaredMethod("lookup", new Class[] { - Class.class, Boolean.TYPE }); - LOOKUP_METHOD.setAccessible(true); - - GET_CLASS_DATA_LAYOUT_METHOD = ObjectStreamClass.class.getDeclaredMethod( - "getClassDataLayout", (Class[])null); - GET_CLASS_DATA_LAYOUT_METHOD.setAccessible(true); - - GET_NUM_OBJ_FIELDS_METHOD = ObjectStreamClass.class.getDeclaredMethod( - "getNumObjFields", (Class[])null); - GET_NUM_OBJ_FIELDS_METHOD.setAccessible(true); - - GET_OBJ_FIELD_VALUES_METHOD = ObjectStreamClass.class.getDeclaredMethod( - "getObjFieldValues", new Class[] { Object.class, Object[].class }); - GET_OBJ_FIELD_VALUES_METHOD.setAccessible(true); - - GET_FIELD_METHOD = ObjectStreamField.class.getDeclaredMethod("getField", (Class[])null); - GET_FIELD_METHOD.setAccessible(true); - - HAS_WRITE_REPLACE_METHOD_METHOD = ObjectStreamClass.class.getDeclaredMethod( - "hasWriteReplaceMethod", (Class[])null); - HAS_WRITE_REPLACE_METHOD_METHOD.setAccessible(true); + Result result = Result.SUCCESS; + if (!(object instanceof Serializable) && (!Proxy.isProxyClass(object.getClass()))) + { + result = new Result(Result.Status.FAILURE, "The object type is not Serializable!", cause); + } - INVOKE_WRITE_REPLACE_METHOD = ObjectStreamClass.class.getDeclaredMethod( - "invokeWriteReplace", new Class[] { Object.class }); - INVOKE_WRITE_REPLACE_METHOD.setAccessible(true); - } - catch (Exception e) - { - log.warn("SerializableChecker not available", e); - available = false; + return result; } } /** - * Gets whether we can execute the tests. If false, calling {@link #check(Object)} will just - * return and you are advised to rely on the {@link NotSerializableException}. Clients are - * advised to call this method prior to calling the check method. - * - * @return whether security settings and underlying API etc allow for accessing the - * serialization API using introspection - */ - public static boolean isAvailable() - { - return available; - } - - /** object stack that with the trace path. */ - private final LinkedList<TraceSlot> traceStack = new LinkedList<TraceSlot>(); - - /** set for checking circular references. */ - private final Map<Object, Object> checked = new IdentityHashMap<Object, Object>(); - - /** string stack with current names pushed. */ - private final LinkedList<String> nameStack = new LinkedList<String>(); - - /** root object being analyzed. */ - private Object root; - - /** set of classes that had no writeObject methods at lookup (to avoid repeated checking) */ - private final Set<Class<?>> writeObjectMethodMissing = new HashSet<Class<?>>(); - - /** current simple field name. */ - private String simpleName = ""; - - /** current full field description. */ - private String fieldDescription; - - /** Exception that should be set as the cause when throwing a new exception. */ - private final NotSerializableException exception; - - private final Stack<Object> stack = new Stack<Object>(); - - /** - * Construct. + * Constructor. * * @param exception * exception that should be set as the cause when throwing a new exception @@ -331,413 +106,19 @@ public final class SerializableChecker extends ObjectOutputStream */ public SerializableChecker(NotSerializableException exception) throws IOException { - this.exception = exception; - } - - /** - * @see java.io.ObjectOutputStream#reset() - */ - @Override - public void reset() throws IOException - { - root = null; - checked.clear(); - fieldDescription = null; - simpleName = null; - traceStack.clear(); - nameStack.clear(); - writeObjectMethodMissing.clear(); - } - - @Override - public void close() throws IOException - { - // do not call super.close() because SerializableChecker uses ObjectOutputStream's no-arg constructor - - // just null-ify the declared members - reset(); - } - - private void check(Object obj) - { - if (obj == null) - { - return; - } - - try - { - if (stack.contains(obj)) - { - return; - } - } - catch (RuntimeException e) - { - log.warn("Wasn't possible to check the object " + obj.getClass() + - " possible due an problematic implementation of equals method"); - /* - * Can't check if this obj were in stack, giving up because we don't want to throw an - * invaluable exception to user. The main goal of this checker is to find non - * serializable data - */ - return; - } - - stack.push(obj); - try - { - internalCheck(obj); - } - finally - { - stack.pop(); - } - } - - private void internalCheck(Object obj) - { - if (obj == null) - { - return; - } - - Class<?> cls = obj.getClass(); - nameStack.add(simpleName); - traceStack.add(new TraceSlot(obj, fieldDescription)); - - if (!(obj instanceof Serializable) && (!Proxy.isProxyClass(cls))) - { - throw new WicketNotSerializableException( - toPrettyPrintedStack(obj.getClass().getName()), exception); - } - - ObjectStreamClass desc; - for (;;) - { - try - { - desc = (ObjectStreamClass)LOOKUP_METHOD.invoke(null, cls, Boolean.TRUE); - Class<?> repCl; - if (!(Boolean)HAS_WRITE_REPLACE_METHOD_METHOD.invoke(desc, (Object[])null) || - (obj = INVOKE_WRITE_REPLACE_METHOD.invoke(desc, obj)) == null || - (repCl = obj.getClass()) == cls) - { - break; - } - cls = repCl; - } - catch (IllegalAccessException e) - { - throw new RuntimeException(e); - } - catch (InvocationTargetException e) - { - throw new RuntimeException(e); - } - } - - if (cls.isPrimitive()) - { - // skip - } - else if (cls.isArray()) - { - checked.put(obj, null); - Class<?> ccl = cls.getComponentType(); - if (!(ccl.isPrimitive())) - { - Object[] objs = (Object[])obj; - for (int i = 0; i < objs.length; i++) - { - String arrayPos = "[" + i + "]"; - simpleName = arrayPos; - fieldDescription += arrayPos; - check(objs[i]); - } - } - } - else if (obj instanceof Externalizable && (!Proxy.isProxyClass(cls))) - { - Externalizable extObj = (Externalizable)obj; - try - { - extObj.writeExternal(new ObjectOutputAdaptor() - { - private int count = 0; - - @Override - public void writeObject(Object streamObj) throws IOException - { - // Check for circular reference. - if (checked.containsKey(streamObj)) - { - return; - } - - checked.put(streamObj, null); - String arrayPos = "[write:" + count++ + "]"; - simpleName = arrayPos; - fieldDescription += arrayPos; - - check(streamObj); - } - }); - } - catch (Exception e) - { - if (e instanceof WicketNotSerializableException) - { - throw (WicketNotSerializableException)e; - } - log.warn("error delegating to Externalizable : " + e.getMessage() + ", path: " + - currentPath()); - } - } - else - { - Method writeObjectMethod = null; - if (writeObjectMethodMissing.contains(cls) == false) - { - try - { - writeObjectMethod = cls.getDeclaredMethod("writeObject", - new Class[] { java.io.ObjectOutputStream.class }); - } - catch (SecurityException e) - { - // we can't access / set accessible to true - writeObjectMethodMissing.add(cls); - } - catch (NoSuchMethodException e) - { - // cls doesn't have that method - writeObjectMethodMissing.add(cls); - } - } - - final Object original = obj; - if (writeObjectMethod != null) - { - class InterceptingObjectOutputStream extends ObjectOutputStream - { - private int counter; - - InterceptingObjectOutputStream() throws IOException - { - super(DUMMY_OUTPUT_STREAM); - enableReplaceObject(true); - } - - @Override - protected Object replaceObject(Object streamObj) throws IOException - { - if (streamObj == original) - { - return streamObj; - } - - counter++; - // Check for circular reference. - if (checked.containsKey(streamObj)) - { - return null; - } - - checked.put(streamObj, null); - String arrayPos = "[write:" + counter + "]"; - simpleName = arrayPos; - fieldDescription += arrayPos; - check(streamObj); - return streamObj; - } - } - try - { - InterceptingObjectOutputStream ioos = new InterceptingObjectOutputStream(); - ioos.writeObject(obj); - } - catch (Exception e) - { - if (e instanceof WicketNotSerializableException) - { - throw (WicketNotSerializableException)e; - } - log.warn("error delegating to writeObject : " + e.getMessage() + ", path: " + - currentPath()); - } - } - else - { - Object[] slots; - try - { - slots = (Object[])GET_CLASS_DATA_LAYOUT_METHOD.invoke(desc, (Object[])null); - } - catch (Exception e) - { - throw new RuntimeException(e); - } - for (Object slot : slots) - { - ObjectStreamClass slotDesc; - try - { - Field descField = slot.getClass().getDeclaredField("desc"); - descField.setAccessible(true); - slotDesc = (ObjectStreamClass)descField.get(slot); - } - catch (Exception e) - { - throw new RuntimeException(e); - } - checked.put(obj, null); - checkFields(obj, slotDesc); - } - } - } - - traceStack.removeLast(); - nameStack.removeLast(); - } - - private void checkFields(Object obj, ObjectStreamClass desc) - { - int numFields; - try - { - numFields = (Integer)GET_NUM_OBJ_FIELDS_METHOD.invoke(desc, (Object[])null); - } - catch (IllegalAccessException e) - { - throw new RuntimeException(e); - } - catch (InvocationTargetException e) - { - throw new RuntimeException(e); - } - - if (numFields > 0) - { - int numPrimFields; - ObjectStreamField[] fields = desc.getFields(); - Object[] objVals = new Object[numFields]; - numPrimFields = fields.length - objVals.length; - try - { - GET_OBJ_FIELD_VALUES_METHOD.invoke(desc, obj, objVals); - } - catch (IllegalAccessException e) - { - throw new RuntimeException(e); - } - catch (InvocationTargetException e) - { - throw new RuntimeException(e); - } - 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) - { - // filter out common cases - continue; - } - - // Check for circular reference. - if (checked.containsKey(objVals[i])) - { - continue; - } - - ObjectStreamField fieldDesc = fields[numPrimFields + i]; - Field field; - try - { - field = (Field)GET_FIELD_METHOD.invoke(fieldDesc, (Object[])null); - } - catch (IllegalAccessException e) - { - throw new RuntimeException(e); - } - catch (InvocationTargetException e) - { - throw new RuntimeException(e); - } - - field.getName(); - simpleName = field.getName(); - fieldDescription = field.toString(); - check(objVals[i]); - } - } - } - - /** - * @return name from root to current node concatenated with slashes - */ - private StringBuilder currentPath() - { - StringBuilder b = new StringBuilder(); - for (Iterator<String> it = nameStack.iterator(); it.hasNext();) - { - b.append(it.next()); - if (it.hasNext()) - { - b.append('/'); - } - } - return b; + super(new ObjectSerializationChecker(exception)); } /** - * Dump with indentation. + * Delegate to preserve binary compatibility. * - * @param type - * the type that couldn't be serialized - * @return A very pretty dump + * @return {@code true} if the checker can be used + * @deprecated Use ObjectChecker#isAvailable() instead */ - private final String toPrettyPrintedStack(String type) - { - StringBuilder result = new StringBuilder(); - StringBuilder spaces = new StringBuilder(); - result.append("Unable to serialize class: "); - result.append(type); - result.append("\nField hierarchy is:"); - for (Iterator<TraceSlot> i = traceStack.listIterator(); i.hasNext();) - { - spaces.append(" "); - TraceSlot slot = i.next(); - result.append("\n").append(spaces).append(slot.fieldDescription); - result.append(" [class=").append(slot.object.getClass().getName()); - if (slot.object instanceof Component) - { - Component component = (Component)slot.object; - result.append(", path=").append(component.getPath()); - } - result.append("]"); - } - result.append(" <----- field that is not serializable"); - return result.toString(); - } - - /** - * @see java.io.ObjectOutputStream#writeObjectOverride(java.lang.Object) - */ - @Override - protected final void writeObjectOverride(Object obj) throws IOException + // TODO Wicket 7.0 - remove this method + @Deprecated + public static boolean isAvailable() { - if (!available) - { - return; - } - root = obj; - if (fieldDescription == null) - { - fieldDescription = (root instanceof Component) ? ((Component)root).getPath() : ""; - } - - check(root); + return ObjectChecker.isAvailable(); } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/wicket/blob/6014d8bb/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/IObjectChecker.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/IObjectChecker.java b/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/IObjectChecker.java new file mode 100644 index 0000000..85ff014 --- /dev/null +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/IObjectChecker.java @@ -0,0 +1,94 @@ +package org.apache.wicket.core.util.objects.checker; + +import org.apache.wicket.util.lang.Args; + +/** + * IObjectChecker can be used to check whether an object has/has not given state + * before serializing it. The serialization will be stopped if the object doesn't pass + * the {@code #check(Object) check}. + */ +public interface IObjectChecker +{ + /** + * Represents the result of a check. + */ + public static class Result + { + public static enum Status + { + /** + * The check is successful + */ + SUCCESS, + + /** + * The check failed for some reason + */ + FAILURE + } + + /** + * A singleton that can be used for successful checks + */ + public static final Result SUCCESS = new Result(Status.SUCCESS, ""); + + /** + * The status of the check. + */ + public final Status status; + + /** + * The reason why a check succeeded/failed. Mandatory in failure case. + */ + public final String reason; + + /** + * An optional cause of a failure. + */ + public final Throwable cause; + + /** + * Constructor. + * + * @param status + * the status of the result + * @param reason + * the reason of successful/failed check + */ + public Result(Status status, String reason) + { + this(status, reason, null); + } + + + /** + * Constructor. + * + * @param status + * the status of the result + * @param reason + * the reason of successful/failed check + * @param cause + * the cause of a failure. Optional. + */ + public Result(Status status, String reason, Throwable cause) + { + if (status == Status.FAILURE) + { + Args.notEmpty(reason, "reason"); + } + this.status = status; + this.reason = reason; + this.cause = cause; + } + } + + /** + * Checks an object that it meets some requirements before serializing it + * + * @param object + * the object to check + * @return a Result object describing whether the check is successful or not + */ + Result check(Object object); +} http://git-wip-us.apache.org/repos/asf/wicket/blob/6014d8bb/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/NotDetachedModelChecker.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/NotDetachedModelChecker.java b/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/NotDetachedModelChecker.java new file mode 100644 index 0000000..969a00a --- /dev/null +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/NotDetachedModelChecker.java @@ -0,0 +1,28 @@ +package org.apache.wicket.core.util.objects.checker; + +import org.apache.wicket.model.LoadableDetachableModel; + +/** + * An implementation of {@link IObjectChecker} that returns a failure + * result when the checked object is a {@link LoadableDetachableModel} + * and it is model object is still attached. + */ +public class NotDetachedModelChecker implements IObjectChecker +{ + @Override + public Result check(Object obj) + { + Result result = Result.SUCCESS; + + if (obj instanceof LoadableDetachableModel<?>) + { + LoadableDetachableModel<?> model = (LoadableDetachableModel<?>) obj; + if (model.isAttached()) + { + result = new Result(Result.Status.FAILURE, "Not detached model found!"); + } + } + + return result; + } +} http://git-wip-us.apache.org/repos/asf/wicket/blob/6014d8bb/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/ObjectChecker.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/ObjectChecker.java b/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/ObjectChecker.java new file mode 100644 index 0000000..629e580 --- /dev/null +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/ObjectChecker.java @@ -0,0 +1,739 @@ +package org.apache.wicket.core.util.objects.checker; + +import java.io.Externalizable; +import java.io.IOException; +import java.io.ObjectOutput; +import java.io.ObjectOutputStream; +import java.io.ObjectStreamClass; +import java.io.ObjectStreamField; +import java.io.OutputStream; +import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.util.Date; +import java.util.HashSet; +import java.util.IdentityHashMap; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.Map; +import java.util.Set; +import java.util.Stack; + +import org.apache.wicket.Component; +import org.apache.wicket.WicketRuntimeException; +import org.apache.wicket.util.lang.Classes; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Checks an object tree during serialization for wrong state by delegating the work + * to the used {@link IObjectChecker IObjectChecker}s. + * <p> + * As this class depends heavily on JDK's serialization internals using introspection, analyzing may + * not be possible, for instance when the runtime environment does not have sufficient rights to set + * fields accessible that would otherwise be hidden. You should call + * {@link ObjectChecker#isAvailable()} to see whether this class can operate properly. + * </p> + */ +public class ObjectChecker extends ObjectOutputStream +{ + private static final Logger log = LoggerFactory.getLogger(ObjectChecker.class); + + public static class ObjectCheckException extends WicketRuntimeException + { + public ObjectCheckException(String message) + { + super(message); + } + + public ObjectCheckException(String message, Throwable cause) + { + super(message, cause); + } + } + + /** + * Exception that is thrown when a non-serializable object was found. + */ + public static class WicketNotSerializableException extends WicketRuntimeException + { + private static final long serialVersionUID = 1L; + + protected WicketNotSerializableException(String message, Throwable cause) + { + super(message, cause); + } + } + + /** + * Does absolutely nothing. + */ + private static class NoopOutputStream extends OutputStream + { + @Override + public void close() + { + } + + @Override + public void flush() + { + } + + @Override + public void write(byte[] b) + { + } + + @Override + public void write(byte[] b, int i, int l) + { + } + + @Override + public void write(int b) + { + } + } + + private static abstract class ObjectOutputAdaptor implements ObjectOutput + { + + @Override + public void close() throws IOException + { + } + + @Override + public void flush() throws IOException + { + } + + @Override + public void write(byte[] b) throws IOException + { + } + + @Override + public void write(byte[] b, int off, int len) throws IOException + { + } + + @Override + public void write(int b) throws IOException + { + } + + @Override + public void writeBoolean(boolean v) throws IOException + { + } + + @Override + public void writeByte(int v) throws IOException + { + } + + @Override + public void writeBytes(String s) throws IOException + { + } + + @Override + public void writeChar(int v) throws IOException + { + } + + @Override + public void writeChars(String s) throws IOException + { + } + + @Override + public void writeDouble(double v) throws IOException + { + } + + @Override + public void writeFloat(float v) throws IOException + { + } + + @Override + public void writeInt(int v) throws IOException + { + } + + @Override + public void writeLong(long v) throws IOException + { + } + + @Override + public void writeShort(int v) throws IOException + { + } + + @Override + public void writeUTF(String str) throws IOException + { + } + } + + /** Holds information about the field and the resulting object being traced. */ + private static final class TraceSlot + { + private final String fieldDescription; + + private final Object object; + + TraceSlot(Object object, String fieldDescription) + { + super(); + this.object = object; + this.fieldDescription = fieldDescription; + } + + @Override + public String toString() + { + return object.getClass() + " - " + fieldDescription; + } + } + + private static final NoopOutputStream DUMMY_OUTPUT_STREAM = new NoopOutputStream(); + + /** Whether we can execute the tests. If false, check will just return. */ + private static boolean available = true; + + // this hack - accessing the serialization API through introspection - is + // the only way to use Java serialization for our purposes without writing + // the whole thing from scratch (and even then, it would be limited). This + // way of working is of course fragile for internal API changes, but as we + // do an extra check on availability and we report when we can't use this + // introspection fu, we'll find out soon enough and clients on this class + // can fall back on Java's default exception for serialization errors (which + // sucks and is the main reason for this attempt). + private static Method LOOKUP_METHOD; + + private static Method GET_CLASS_DATA_LAYOUT_METHOD; + + private static Method GET_NUM_OBJ_FIELDS_METHOD; + + private static Method GET_OBJ_FIELD_VALUES_METHOD; + + private static Method GET_FIELD_METHOD; + + private static Method HAS_WRITE_REPLACE_METHOD_METHOD; + + private static Method INVOKE_WRITE_REPLACE_METHOD; + + static + { + try + { + LOOKUP_METHOD = ObjectStreamClass.class.getDeclaredMethod("lookup", new Class[] { + Class.class, Boolean.TYPE }); + LOOKUP_METHOD.setAccessible(true); + + GET_CLASS_DATA_LAYOUT_METHOD = ObjectStreamClass.class.getDeclaredMethod( + "getClassDataLayout", (Class[])null); + GET_CLASS_DATA_LAYOUT_METHOD.setAccessible(true); + + GET_NUM_OBJ_FIELDS_METHOD = ObjectStreamClass.class.getDeclaredMethod( + "getNumObjFields", (Class[])null); + GET_NUM_OBJ_FIELDS_METHOD.setAccessible(true); + + GET_OBJ_FIELD_VALUES_METHOD = ObjectStreamClass.class.getDeclaredMethod( + "getObjFieldValues", new Class[] { Object.class, Object[].class }); + GET_OBJ_FIELD_VALUES_METHOD.setAccessible(true); + + GET_FIELD_METHOD = ObjectStreamField.class.getDeclaredMethod("getField", (Class[])null); + GET_FIELD_METHOD.setAccessible(true); + + HAS_WRITE_REPLACE_METHOD_METHOD = ObjectStreamClass.class.getDeclaredMethod( + "hasWriteReplaceMethod", (Class[])null); + HAS_WRITE_REPLACE_METHOD_METHOD.setAccessible(true); + + INVOKE_WRITE_REPLACE_METHOD = ObjectStreamClass.class.getDeclaredMethod( + "invokeWriteReplace", new Class[] { Object.class }); + INVOKE_WRITE_REPLACE_METHOD.setAccessible(true); + } + catch (Exception e) + { + log.warn("SerializableChecker not available", e); + available = false; + } + } + + private final IObjectChecker[] checkers; + + /** + * Gets whether we can execute the tests. If false, calling {@link #check(Object)} will just + * return and you are advised to rely on the {@link java.io.NotSerializableException}. Clients are + * advised to call this method prior to calling the check method. + * + * @return whether security settings and underlying API etc allow for accessing the + * serialization API using introspection + */ + public static boolean isAvailable() + { + return available; + } + + /** object stack that with the trace path. */ + private final LinkedList<TraceSlot> traceStack = new LinkedList<TraceSlot>(); + + /** set for checking circular references. */ + private final Map<Object, Object> checked = new IdentityHashMap<Object, Object>(); + + /** string stack with current names pushed. */ + private final LinkedList<String> nameStack = new LinkedList<String>(); + + /** root object being analyzed. */ + private Object root; + + /** set of classes that had no writeObject methods at lookup (to avoid repeated checking) */ + private final Set<Class<?>> writeObjectMethodMissing = new HashSet<Class<?>>(); + + /** current simple field name. */ + private String simpleName = ""; + + /** current full field description. */ + private String fieldDescription; + + private final Stack<Object> stack = new Stack<Object>(); + + /** + * Constructor. + * + * @throws IOException + * @throws SecurityException + */ + public ObjectChecker(final IObjectChecker... checkers) throws IOException, SecurityException + { + this.checkers = checkers; + } + + private void check(Object obj) + { + if (obj == null) + { + return; + } + + try + { + if (stack.contains(obj)) + { + return; + } + } + catch (RuntimeException e) + { + log.warn("Wasn't possible to check the object " + obj.getClass() + + " possible due an problematic implementation of equals method"); + /* + * Can't check if this obj were in stack, giving up because we don't want to throw an + * invaluable exception to user. The main goal of this checker is to find non + * serializable data + */ + return; + } + + stack.push(obj); + try + { + internalCheck(obj); + } + finally + { + stack.pop(); + } + } + + private void internalCheck(Object obj) + { + if (obj == null) + { + return; + } + + Class<?> cls = obj.getClass(); + nameStack.add(simpleName); + traceStack.add(new TraceSlot(obj, fieldDescription)); + + for (IObjectChecker checker : checkers) + { + IObjectChecker.Result result = checker.check(obj); + if (result.status == IObjectChecker.Result.Status.FAILURE) + { + ObjectCheckException ocx; + String prettyPrintMessage = toPrettyPrintedStack(Classes.name(cls)); + String exceptionMessage = result.reason + '\n' + prettyPrintMessage; + if (result.cause != null) + { + ocx = new ObjectCheckException(exceptionMessage, result.cause); + } + else + { + ocx = new ObjectCheckException(exceptionMessage); + } + throw ocx; + } + } + + ObjectStreamClass desc; + for (;;) + { + try + { + desc = (ObjectStreamClass)LOOKUP_METHOD.invoke(null, cls, Boolean.TRUE); + Class<?> repCl; + if (!(Boolean)HAS_WRITE_REPLACE_METHOD_METHOD.invoke(desc, (Object[])null) || + (obj = INVOKE_WRITE_REPLACE_METHOD.invoke(desc, obj)) == null || + (repCl = obj.getClass()) == cls) + { + break; + } + cls = repCl; + } + catch (IllegalAccessException e) + { + throw new RuntimeException(e); + } + catch (InvocationTargetException e) + { + throw new RuntimeException(e); + } + } + + if (cls.isPrimitive()) + { + // skip + } + else if (cls.isArray()) + { + checked.put(obj, null); + Class<?> ccl = cls.getComponentType(); + if (!(ccl.isPrimitive())) + { + Object[] objs = (Object[])obj; + for (int i = 0; i < objs.length; i++) + { + String arrayPos = "[" + i + "]"; + simpleName = arrayPos; + fieldDescription += arrayPos; + check(objs[i]); + } + } + } + else if (obj instanceof Externalizable && (!Proxy.isProxyClass(cls))) + { + Externalizable extObj = (Externalizable)obj; + try + { + extObj.writeExternal(new ObjectOutputAdaptor() + { + private int count = 0; + + @Override + public void writeObject(Object streamObj) throws IOException + { + // Check for circular reference. + if (checked.containsKey(streamObj)) + { + return; + } + + checked.put(streamObj, null); + String arrayPos = "[write:" + count++ + "]"; + simpleName = arrayPos; + fieldDescription += arrayPos; + + check(streamObj); + } + }); + } + catch (Exception e) + { + if (e instanceof WicketNotSerializableException) + { + throw (WicketNotSerializableException)e; + } + log.warn("error delegating to Externalizable : " + e.getMessage() + ", path: " + + currentPath()); + } + } + else + { + Method writeObjectMethod = null; + if (writeObjectMethodMissing.contains(cls) == false) + { + try + { + writeObjectMethod = cls.getDeclaredMethod("writeObject", + new Class[] { java.io.ObjectOutputStream.class }); + } + catch (SecurityException e) + { + // we can't access / set accessible to true + writeObjectMethodMissing.add(cls); + } + catch (NoSuchMethodException e) + { + // cls doesn't have that method + writeObjectMethodMissing.add(cls); + } + } + + final Object original = obj; + if (writeObjectMethod != null) + { + class InterceptingObjectOutputStream extends ObjectOutputStream + { + private int counter; + + InterceptingObjectOutputStream() throws IOException + { + super(DUMMY_OUTPUT_STREAM); + enableReplaceObject(true); + } + + @Override + protected Object replaceObject(Object streamObj) throws IOException + { + if (streamObj == original) + { + return streamObj; + } + + counter++; + // Check for circular reference. + if (checked.containsKey(streamObj)) + { + return null; + } + + checked.put(streamObj, null); + String arrayPos = "[write:" + counter + "]"; + simpleName = arrayPos; + fieldDescription += arrayPos; + check(streamObj); + return streamObj; + } + } + try + { + InterceptingObjectOutputStream ioos = new InterceptingObjectOutputStream(); + ioos.writeObject(obj); + } + catch (Exception e) + { + if (e instanceof WicketNotSerializableException) + { + throw (WicketNotSerializableException)e; + } + log.warn("error delegating to writeObject : " + e.getMessage() + ", path: " + + currentPath()); + } + } + else + { + Object[] slots; + try + { + slots = (Object[])GET_CLASS_DATA_LAYOUT_METHOD.invoke(desc, (Object[])null); + } + catch (Exception e) + { + throw new RuntimeException(e); + } + for (Object slot : slots) + { + ObjectStreamClass slotDesc; + try + { + Field descField = slot.getClass().getDeclaredField("desc"); + descField.setAccessible(true); + slotDesc = (ObjectStreamClass)descField.get(slot); + } + catch (Exception e) + { + throw new RuntimeException(e); + } + checked.put(obj, null); + checkFields(obj, slotDesc); + } + } + } + + traceStack.removeLast(); + nameStack.removeLast(); + } + + private void checkFields(Object obj, ObjectStreamClass desc) + { + int numFields; + try + { + numFields = (Integer)GET_NUM_OBJ_FIELDS_METHOD.invoke(desc, (Object[])null); + } + catch (IllegalAccessException e) + { + throw new RuntimeException(e); + } + catch (InvocationTargetException e) + { + throw new RuntimeException(e); + } + + if (numFields > 0) + { + int numPrimFields; + ObjectStreamField[] fields = desc.getFields(); + Object[] objVals = new Object[numFields]; + numPrimFields = fields.length - objVals.length; + try + { + GET_OBJ_FIELD_VALUES_METHOD.invoke(desc, obj, objVals); + } + catch (IllegalAccessException e) + { + throw new RuntimeException(e); + } + catch (InvocationTargetException e) + { + throw new RuntimeException(e); + } + 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) + { + // filter out common cases + continue; + } + + // Check for circular reference. + if (checked.containsKey(objVals[i])) + { + continue; + } + + ObjectStreamField fieldDesc = fields[numPrimFields + i]; + Field field; + try + { + field = (Field)GET_FIELD_METHOD.invoke(fieldDesc, (Object[])null); + } + catch (IllegalAccessException e) + { + throw new RuntimeException(e); + } + catch (InvocationTargetException e) + { + throw new RuntimeException(e); + } + + field.getName(); + simpleName = field.getName(); + fieldDescription = field.toString(); + check(objVals[i]); + } + } + } + + /** + * @return name from root to current node concatenated with slashes + */ + private StringBuilder currentPath() + { + StringBuilder b = new StringBuilder(); + for (Iterator<String> it = nameStack.iterator(); it.hasNext();) + { + b.append(it.next()); + if (it.hasNext()) + { + b.append('/'); + } + } + return b; + } + + /** + * Dump with indentation. + * + * @param type + * the type that couldn't be serialized + * @return A very pretty dump + */ + protected final String toPrettyPrintedStack(String type) + { + StringBuilder result = new StringBuilder(); + StringBuilder spaces = new StringBuilder(); + result.append("A problem occurred while checking object with type: "); + result.append(type); + result.append("\nField hierarchy is:"); + for (TraceSlot slot : traceStack) + { + spaces.append(" "); + result.append("\n").append(spaces).append(slot.fieldDescription); + result.append(" [class=").append(slot.object.getClass().getName()); + if (slot.object instanceof Component) + { + Component component = (Component)slot.object; + result.append(", path=").append(component.getPath()); + } + result.append("]"); + } + result.append(" <----- field that is causing the problem"); + return result.toString(); + } + + /** + * @see java.io.ObjectOutputStream#writeObjectOverride(java.lang.Object) + */ + @Override + protected final void writeObjectOverride(Object obj) throws IOException + { + if (!available) + { + return; + } + root = obj; + if (fieldDescription == null) + { + fieldDescription = (root instanceof Component) ? ((Component)root).getPath() : ""; + } + + check(root); + } + + /** + * @see java.io.ObjectOutputStream#reset() + */ + @Override + public void reset() throws IOException + { + root = null; + checked.clear(); + fieldDescription = null; + simpleName = null; + traceStack.clear(); + nameStack.clear(); + writeObjectMethodMissing.clear(); + } + + @Override + public void close() throws IOException + { + // do not call super.close() because SerializableChecker uses ObjectOutputStream's no-arg constructor + + // just null-ify the declared members + reset(); + } + +} http://git-wip-us.apache.org/repos/asf/wicket/blob/6014d8bb/wicket-core/src/main/java/org/apache/wicket/serialize/java/JavaSerializer.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/serialize/java/JavaSerializer.java b/wicket-core/src/main/java/org/apache/wicket/serialize/java/JavaSerializer.java index 57ff0c7..bb23879 100644 --- a/wicket-core/src/main/java/org/apache/wicket/serialize/java/JavaSerializer.java +++ b/wicket-core/src/main/java/org/apache/wicket/serialize/java/JavaSerializer.java @@ -31,6 +31,8 @@ import org.apache.wicket.ThreadContext; import org.apache.wicket.WicketRuntimeException; import org.apache.wicket.application.IClassResolver; import org.apache.wicket.core.util.io.SerializableChecker; +import org.apache.wicket.core.util.objects.checker.IObjectChecker; +import org.apache.wicket.core.util.objects.checker.ObjectChecker; import org.apache.wicket.serialize.ISerializer; import org.apache.wicket.settings.IApplicationSettings; import org.apache.wicket.util.io.IOUtils; @@ -172,7 +174,7 @@ public class JavaSerializer implements ISerializer */ protected ObjectOutputStream newObjectOutputStream(OutputStream out) throws IOException { - return new CheckerObjectOutputStream(out); + return new SerializationCheckerObjectOutputStream(out); } /** @@ -234,11 +236,11 @@ public class JavaSerializer implements ISerializer * Write objects to the wrapped output stream and log a meaningful message for serialization * problems */ - private static class CheckerObjectOutputStream extends ObjectOutputStream + private static class SerializationCheckerObjectOutputStream extends ObjectOutputStream { private final ObjectOutputStream oos; - public CheckerObjectOutputStream(OutputStream out) throws IOException + public SerializationCheckerObjectOutputStream(OutputStream out) throws IOException { oos = new ObjectOutputStream(out); } @@ -252,7 +254,7 @@ public class JavaSerializer implements ISerializer } catch (NotSerializableException nsx) { - if (SerializableChecker.isAvailable()) + if (ObjectChecker.isAvailable()) { // trigger serialization again, but this time gather // some more info @@ -282,4 +284,58 @@ public class JavaSerializer implements ISerializer oos.close(); } } + + /** + * An ObjectOutputStream that uses {@link IObjectChecker IObjectChecker}s to check the + * state of the object before serializing it. If the checker returns + * {@link org.apache.wicket.core.util.objects.checker.IObjectChecker.Result.Status#FAILURE} + * then the serialization process is stopped and the error is logged. + */ + public static class ObjectCheckerObjectOutputStream extends ObjectOutputStream + { + private final ObjectOutputStream oos; + + /** + * The {@link IObjectChecker checkers} to use during the serialization + */ + private final IObjectChecker[] checkers; + + public ObjectCheckerObjectOutputStream(OutputStream out, IObjectChecker... checkers) throws IOException + { + oos = new ObjectOutputStream(out); + this.checkers = checkers; + } + + @Override + protected final void writeObjectOverride(Object obj) throws IOException + { + try + { + if (ObjectChecker.isAvailable()) + { + ObjectChecker checker = new ObjectChecker(checkers); + checker.writeObject(obj); + } + + oos.writeObject(obj); + } + catch (Exception e) + { + log.error("error writing object " + obj + ": " + e.getMessage(), e); + throw new WicketRuntimeException(e); + } + } + + @Override + public void flush() throws IOException + { + oos.flush(); + } + + @Override + public void close() throws IOException + { + oos.close(); + } + } } http://git-wip-us.apache.org/repos/asf/wicket/blob/6014d8bb/wicket-core/src/test/java/org/apache/wicket/serialize/java/JavaSerializerTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/serialize/java/JavaSerializerTest.java b/wicket-core/src/test/java/org/apache/wicket/serialize/java/JavaSerializerTest.java new file mode 100644 index 0000000..31086b3 --- /dev/null +++ b/wicket-core/src/test/java/org/apache/wicket/serialize/java/JavaSerializerTest.java @@ -0,0 +1,85 @@ +package org.apache.wicket.serialize.java; + +import java.io.IOException; +import java.io.ObjectOutputStream; +import java.io.OutputStream; + +import org.apache.wicket.WicketTestCase; +import org.apache.wicket.core.util.objects.checker.IObjectChecker; +import org.apache.wicket.core.util.objects.checker.NotDetachedModelChecker; +import org.apache.wicket.markup.html.WebComponent; +import org.apache.wicket.model.IModel; +import org.apache.wicket.model.LoadableDetachableModel; +import org.junit.Test; + +/** + * + */ +public class JavaSerializerTest extends WicketTestCase +{ + /** + * https://issues.apache.org/jira/browse/WICKET-4812 + * + * Tests that the serialization fails when a checking ObjectOutputStream is + * used with NotDetachedModelChecker and there is a non-detached LoadableDetachableModel + * in the object tree. + */ + @Test + public void notDetachedModel() + { + JavaSerializer serializer = new JavaSerializer("JavaSerializerTest") + { + @Override + protected ObjectOutputStream newObjectOutputStream(OutputStream out) throws IOException + { + IObjectChecker checker = new NotDetachedModelChecker(); + return new ObjectCheckerObjectOutputStream(out, checker); + } + }; + + IModel<String> model = new NotDetachedModel(); + model.getObject(); + WebComponent component = new WebComponent("id", model); + byte[] serialized = serializer.serialize(component); + assertNull("The produced byte[] must be null if there was an error", serialized); + } + + /** + * A Model used for #notDetachedModel() test + */ + private static class NotDetachedModel extends LoadableDetachableModel<String> + { + @Override + protected String load() + { + return "loaded"; + } + } + + /** + * https://issues.apache.org/jira/browse/WICKET-4812 + * + * Tests that serialization fails when using the default ObjectOutputStream in + * JavaSerializer and some object in the tree is not Serializable + */ + @Test + public void notSerializable() + { + JavaSerializer serializer = new JavaSerializer("JavaSerializerTest"); + WebComponent component = new NotSerializableComponent("id"); + byte[] serialized = serializer.serialize(component); + assertNull("The produced byte[] must be null if there was an error", serialized); + } + + private static class NotSerializableComponent extends WebComponent + { + private final NotSerializableObject member = new NotSerializableObject(); + + public NotSerializableComponent(final String id) + { + super(id); + } + } + + private static class NotSerializableObject {} +}
