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 2017/03/20 20:59:24 UTC

[41/48] wicket git commit: WicketWICKET-6334 WicketObjects#sizeof() should detach Sessions

WicketWICKET-6334 WicketObjects#sizeof() should detach Sessions

CheckingObjectOutputStream now use the IObjectChecker's only when a IManageablePage is being serialized. Anything else won't be checked.


Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/3dc4cf69
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/3dc4cf69
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/3dc4cf69

Branch: refs/heads/master
Commit: 3dc4cf69b3abb852f064129737afde2dbbf8a986
Parents: 2c4b396
Author: Martin Tzvetanov Grigorov <mg...@apache.org>
Authored: Mon Mar 6 21:18:43 2017 +0100
Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
Committed: Mon Mar 20 21:42:32 2017 +0100

----------------------------------------------------------------------
 .../main/java/org/apache/wicket/Session.java    |   3 +-
 .../checker/CheckingObjectOutputStream.java     |  17 +--
 .../checker/DifferentPageCheckerTest.java       |   6 +-
 .../checker/NotDetachedModelCheckerTest.java    | 108 +++++++++++++++++++
 .../objects/checker/SessionCheckerTest.java     |  29 ++++-
 .../serialize/java/JavaSerializerTest.java      |  43 --------
 .../wicket/util/io/SerializableCheckerTest.java |  43 ++++++--
 .../inspector/SessionSizeModelTest.java         |  13 ++-
 8 files changed, 196 insertions(+), 66 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/wicket/blob/3dc4cf69/wicket-core/src/main/java/org/apache/wicket/Session.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/Session.java b/wicket-core/src/main/java/org/apache/wicket/Session.java
index e7f9c14..059805f 100644
--- a/wicket-core/src/main/java/org/apache/wicket/Session.java
+++ b/wicket-core/src/main/java/org/apache/wicket/Session.java
@@ -35,6 +35,7 @@ import org.apache.wicket.event.IEvent;
 import org.apache.wicket.event.IEventSink;
 import org.apache.wicket.feedback.FeedbackMessage;
 import org.apache.wicket.feedback.FeedbackMessages;
+import org.apache.wicket.model.IDetachable;
 import org.apache.wicket.page.IPageManager;
 import org.apache.wicket.page.PageAccessSynchronizer;
 import org.apache.wicket.request.Request;
@@ -106,7 +107,7 @@ import org.slf4j.LoggerFactory;
  * @author Eelco Hillenius
  * @author Igor Vaynberg (ivaynberg)
  */
-public abstract class Session implements IClusterable, IEventSink
+public abstract class Session implements IClusterable, IEventSink, IDetachable
 {
 	private static final long serialVersionUID = 1L;
 

http://git-wip-us.apache.org/repos/asf/wicket/blob/3dc4cf69/wicket-core/src/main/java/org/apache/wicket/core/util/objects/checker/CheckingObjectOutputStream.java
----------------------------------------------------------------------
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..2299e42 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
@@ -36,7 +36,9 @@ import java.util.Map;
 import java.util.Set;
 
 import org.apache.wicket.Component;
+import org.apache.wicket.Page;
 import org.apache.wicket.WicketRuntimeException;
+import org.apache.wicket.page.IManageablePage;
 import org.apache.wicket.util.lang.Classes;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -348,14 +350,17 @@ public class CheckingObjectOutputStream extends ObjectOutputStream
 		nameStack.add(simpleName);
 		traceStack.add(new TraceSlot(obj, fieldDescription));
 
-		for (IObjectChecker checker : checkers)
+		if (obj instanceof IManageablePage || (traceStack.size() > 1 && traceStack.get(0).object instanceof IManageablePage))
 		{
-			IObjectChecker.Result result = checker.check(obj);
-			if (result.status == IObjectChecker.Result.Status.FAILURE)
+			for (IObjectChecker checker : checkers)
 			{
-				String prettyPrintMessage = toPrettyPrintedStack(Classes.name(cls));
-				String exceptionMessage = result.reason + '\n' + prettyPrintMessage;
-				throw new ObjectCheckException(exceptionMessage, result.cause);
+				IObjectChecker.Result result = checker.check(obj);
+				if (result.status == IObjectChecker.Result.Status.FAILURE)
+				{
+					String prettyPrintMessage = toPrettyPrintedStack(Classes.name(cls));
+					String exceptionMessage = result.reason + '\n' + prettyPrintMessage;
+					throw new ObjectCheckException(exceptionMessage, result.cause);
+				}
 			}
 		}
 

http://git-wip-us.apache.org/repos/asf/wicket/blob/3dc4cf69/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/DifferentPageCheckerTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/DifferentPageCheckerTest.java b/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/DifferentPageCheckerTest.java
index 8752d2d..380b00d 100644
--- a/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/DifferentPageCheckerTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/DifferentPageCheckerTest.java
@@ -21,7 +21,7 @@ import java.io.ObjectOutputStream;
 import java.io.OutputStream;
 
 import org.apache.wicket.Component;
-import org.apache.wicket.MockPageWithLink;
+import org.apache.wicket.MockPageWithOneComponent;
 import org.apache.wicket.Page;
 import org.apache.wicket.markup.html.WebComponent;
 import org.apache.wicket.markup.html.form.login.MockHomePage;
@@ -51,8 +51,8 @@ public class DifferentPageCheckerTest extends WicketTestCase
 			}
 		};
 
-		WebComponent component = new ComponentThatKeepsAReferenceToAnotherPage(MockPageWithLink.LINK_ID);
-		MockPageWithLink rootPage = new MockPageWithLink();
+		WebComponent component = new ComponentThatKeepsAReferenceToAnotherPage(MockPageWithOneComponent.COMPONENT_ID);
+		MockPageWithOneComponent rootPage = new MockPageWithOneComponent();
 		rootPage.add(component);
 		byte[] serialized = serializer.serialize(rootPage);
 		assertNull("The produced byte[] must be null if there was an error", serialized);

http://git-wip-us.apache.org/repos/asf/wicket/blob/3dc4cf69/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/NotDetachedModelCheckerTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/NotDetachedModelCheckerTest.java b/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/NotDetachedModelCheckerTest.java
new file mode 100644
index 0000000..260178b
--- /dev/null
+++ b/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/NotDetachedModelCheckerTest.java
@@ -0,0 +1,108 @@
+/*
+ * 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 static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+
+import java.io.IOException;
+import java.io.ObjectOutputStream;
+import java.io.OutputStream;
+
+import org.apache.wicket.MockPageWithOneComponent;
+import org.apache.wicket.markup.html.WebComponent;
+import org.apache.wicket.model.IModel;
+import org.apache.wicket.model.LoadableDetachableModel;
+import org.apache.wicket.serialize.java.JavaSerializer;
+import org.apache.wicket.util.tester.WicketTestCase;
+import org.junit.Test;
+
+/**
+ * Tests for {@link NotDetachedModelChecker}.
+ * <p>
+ * Tests that the serialization fails when a checking ObjectOutputStream is
+ * used with NotDetachedModelChecker and there is a non-detached LoadableDetachableModel
+ * in the object tree.
+ * </p>
+ */
+public class NotDetachedModelCheckerTest extends WicketTestCase
+{
+	/**
+	 * https://issues.apache.org/jira/browse/WICKET-4812
+	 * https://issues.apache.org/jira/browse/WICKET-6334
+	 */
+	@Test
+	public void whenSerializingPage_thenItsComponentsShouldBeChecked() {
+		JavaSerializer serializer = new JavaSerializer("JavaSerializerTest")
+		{
+			@Override
+			protected ObjectOutputStream newObjectOutputStream(OutputStream out) throws IOException
+			{
+				IObjectChecker checker = new NotDetachedModelChecker();
+				return new CheckingObjectOutputStream(out, checker);
+			}
+		};
+
+		MockPageWithOneComponent page = new MockPageWithOneComponent();
+		page.add(new ComponentWithAttachedModel(MockPageWithOneComponent.COMPONENT_ID));
+
+		final byte[] serialized = serializer.serialize(page);
+		assertNull("The produced byte[] must be null if there was an error", serialized);
+	}
+
+	/**
+	 * https://issues.apache.org/jira/browse/WICKET-4812
+	 * https://issues.apache.org/jira/browse/WICKET-6334
+	 */
+	@Test
+	public void whenSerializingNonPageComponent_thenItsSubComponentsShouldNotBeChecked() {
+		JavaSerializer serializer = new JavaSerializer("JavaSerializerTest")
+		{
+			@Override
+			protected ObjectOutputStream newObjectOutputStream(OutputStream out) throws IOException
+			{
+				IObjectChecker checker = new NotDetachedModelChecker();
+				return new CheckingObjectOutputStream(out, checker);
+			}
+		};
+
+		final ComponentWithAttachedModel component = new ComponentWithAttachedModel("id");
+
+		final byte[] serialized = serializer.serialize(component);
+		assertThat(serialized, is(notNullValue()));
+	}
+
+	private static class ComponentWithAttachedModel extends WebComponent
+	{
+		private final IModel<String> member = new LoadableDetachableModel<String>()
+		{
+			@Override
+			protected String load()
+			{
+				return "modelObject";
+			}
+		};
+
+		public ComponentWithAttachedModel(final String id)
+		{
+			super(id);
+
+			// attach the model object
+			member.getObject();
+		}
+	}
+}

http://git-wip-us.apache.org/repos/asf/wicket/blob/3dc4cf69/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/SessionCheckerTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/SessionCheckerTest.java b/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/SessionCheckerTest.java
index 94e8ea7..7a40f3c 100644
--- a/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/SessionCheckerTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/core/util/objects/checker/SessionCheckerTest.java
@@ -16,10 +16,14 @@
  */
 package org.apache.wicket.core.util.objects.checker;
 
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+
 import java.io.IOException;
 import java.io.ObjectOutputStream;
 import java.io.OutputStream;
 
+import org.apache.wicket.MockPageWithOneComponent;
 import org.apache.wicket.Session;
 import org.apache.wicket.markup.html.WebComponent;
 import org.apache.wicket.markup.html.WebMarkupContainer;
@@ -53,15 +57,36 @@ public class SessionCheckerTest extends WicketTestCase
 			}
 		};
 
-		WebMarkupContainer container = new WebMarkupContainer("container");
+		MockPageWithOneComponent page = new MockPageWithOneComponent();
+		WebMarkupContainer container = new WebMarkupContainer(MockPageWithOneComponent.COMPONENT_ID);
+		page.add(container);
 		// WICKET-6196 force container#children to be an array
 		container.add(new Label("id1"));
 		container.add(new ComponentWithAReferenceToTheSession("id2"));
 		
-		byte[] serialized = serializer.serialize(container);
+		byte[] serialized = serializer.serialize(page);
 		assertNull("The produced byte[] must be null if there was an error", serialized);
 	}
 
+	/**
+	 * https://issues.apache.org/jira/browse/WICKET-6334
+	 */
+	@Test
+	public void sessionCheckerShouldNotCheckSerializationOfTheSessionItself() {
+		JavaSerializer serializer = new JavaSerializer("JavaSerializerTest")
+		{
+			@Override
+			protected ObjectOutputStream newObjectOutputStream(OutputStream out) throws IOException
+			{
+				IObjectChecker checker = new SessionChecker();
+				return new CheckingObjectOutputStream(out, checker);
+			}
+		};
+		final Session session = new WebSession(new MockWebRequest(Url.parse("")));
+		final byte[] serialized = serializer.serialize(session);
+		assertThat(serialized, is(notNullValue()));
+	}
+
 	private static class ComponentWithAReferenceToTheSession extends WebComponent
 	{
 		private final Session member = new WebSession(new MockWebRequest(Url.parse("")));

http://git-wip-us.apache.org/repos/asf/wicket/blob/3dc4cf69/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
index c90414c..7c09701 100644
--- 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
@@ -29,11 +29,7 @@ import java.io.Serializable;
 
 import org.apache.wicket.WicketRuntimeException;
 import org.apache.wicket.core.util.objects.checker.CheckingObjectOutputStream;
-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.apache.wicket.util.io.IOUtils;
 import org.apache.wicket.util.tester.WicketTestCase;
 import org.junit.Test;
@@ -45,45 +41,6 @@ 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 CheckingObjectOutputStream(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

http://git-wip-us.apache.org/repos/asf/wicket/blob/3dc4cf69/wicket-core/src/test/java/org/apache/wicket/util/io/SerializableCheckerTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/util/io/SerializableCheckerTest.java b/wicket-core/src/test/java/org/apache/wicket/util/io/SerializableCheckerTest.java
index 0fdce37..171caf1 100644
--- a/wicket-core/src/test/java/org/apache/wicket/util/io/SerializableCheckerTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/util/io/SerializableCheckerTest.java
@@ -21,23 +21,20 @@ import static org.hamcrest.Matchers.is;
 import java.io.IOException;
 import java.io.NotSerializableException;
 import java.io.Serializable;
-import java.util.Map;
-import java.util.Set;
 
-import org.apache.wicket.core.util.objects.checker.CheckingObjectOutputStream;
-import org.apache.wicket.core.util.objects.checker.ObjectSerializationChecker;
 import org.apache.wicket.core.util.objects.checker.AbstractObjectChecker;
 import org.apache.wicket.core.util.objects.checker.CheckingObjectOutputStream;
-import org.apache.wicket.core.util.objects.checker.IObjectChecker;
+import org.apache.wicket.core.util.objects.checker.ObjectSerializationChecker;
+import org.apache.wicket.markup.html.WebPage;
+import org.apache.wicket.page.IManageablePage;
+import org.apache.wicket.util.tester.WicketTestCase;
 import org.apache.wicket.util.value.ValueMap;
-import org.hamcrest.Matchers;
-import org.junit.Assert;
 import org.junit.Test;
 
 /**
  * @author Pedro Santos
  */
-public class SerializableCheckerTest extends Assert
+public class SerializableCheckerTest extends WicketTestCase
 {
 
 	/**
@@ -120,7 +117,33 @@ public class SerializableCheckerTest extends Assert
 		assertTrue(exceptionMessage.contains(NonSerializableType.class.getName()));
 	}
 
-	private static class IdentityTestType implements Serializable
+	private static class ManageablePage implements IManageablePage
+	{
+		@Override
+		public boolean isPageStateless()
+		{
+			return false;
+		}
+
+		@Override
+		public int getPageId()
+		{
+			return 0;
+		}
+
+		@Override
+		public void detach()
+		{
+		}
+
+		@Override
+		public boolean setFreezePageId(boolean freeze)
+		{
+			return false;
+		}
+	}
+
+	private static class IdentityTestType extends ManageablePage
 	{
 		private static final long serialVersionUID = 1L;
 
@@ -133,7 +156,7 @@ public class SerializableCheckerTest extends Assert
 		}
 	}
 
-	private static class TestType2 implements Serializable
+	private static class TestType2 extends WebPage
 	{
 		private static final long serialVersionUID = 1L;
 		ProblematicType problematicType = new ProblematicType();

http://git-wip-us.apache.org/repos/asf/wicket/blob/3dc4cf69/wicket-devutils/src/test/java/org/apache/wicket/devutils/inspector/SessionSizeModelTest.java
----------------------------------------------------------------------
diff --git a/wicket-devutils/src/test/java/org/apache/wicket/devutils/inspector/SessionSizeModelTest.java b/wicket-devutils/src/test/java/org/apache/wicket/devutils/inspector/SessionSizeModelTest.java
index af5b583..3af0218 100644
--- a/wicket-devutils/src/test/java/org/apache/wicket/devutils/inspector/SessionSizeModelTest.java
+++ b/wicket-devutils/src/test/java/org/apache/wicket/devutils/inspector/SessionSizeModelTest.java
@@ -16,6 +16,8 @@
  */
 package org.apache.wicket.devutils.inspector;
 
+import static org.hamcrest.Matchers.containsString;
+
 import org.apache.wicket.Session;
 import org.apache.wicket.mock.MockApplication;
 import org.apache.wicket.protocol.http.WebSession;
@@ -23,13 +25,17 @@ import org.apache.wicket.request.Request;
 import org.apache.wicket.request.Response;
 import org.apache.wicket.util.tester.WicketTester;
 import org.junit.Assert;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 /**
  * @author Pedro Santos
  */
 public class SessionSizeModelTest extends Assert
 {
+	@Rule
+	public ExpectedException expectedException = ExpectedException.none();
 
 	/**
 	 * @see <a href="https://issues.apache.org/jira/browse/WICKET-3355">WICKET-3355</a>
@@ -37,6 +43,11 @@ public class SessionSizeModelTest extends Assert
 	@Test
 	public void testToleranceOnProblematicSessions()
 	{
+		expectedException.expect(IllegalStateException.class);
+		expectedException.expectMessage(containsString("A problem occurred while serializing an object. " +
+				"Please check the earlier logs for more details. Problematic object: " +
+				"org.apache.wicket.devutils.inspector.SessionSizeModelTest$TestSession"));
+
 		new WicketTester(new MockApplication()
 		{
 			@Override
@@ -46,7 +57,7 @@ public class SessionSizeModelTest extends Assert
 			}
 		});
 		SessionSizeModel model = new SessionSizeModel();
-		assertEquals(null, model.getObject());
+		model.getObject();
 	}
 
 	/**