Repository: wicket Updated Branches: refs/heads/master 4b07e9a29 -> 8b6fcd869
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/8b6fcd86 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/8b6fcd86 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/8b6fcd86 Branch: refs/heads/master Commit: 8b6fcd869ceb96f7b4ea003d3d5665a1626390ad Parents: 4b07e9a Author: Martin Tzvetanov Grigorov <[email protected]> Authored: Mon Mar 6 21:18:43 2017 +0100 Committer: Martin Tzvetanov Grigorov <[email protected]> Committed: Mon Mar 6 21:18:43 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/8b6fcd86/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/8b6fcd86/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/8b6fcd86/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/8b6fcd86/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/8b6fcd86/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/8b6fcd86/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/8b6fcd86/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/8b6fcd86/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(); } /**
