From 95e563a69b540bae2616466ae25646eba50d4afe Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Wed, 12 Mar 2014 21:17:18 +0200 Subject: [PATCH] Prevent adding/setting parent component as child (#12720). Change-Id: I9b7b43f176e88d9edca591001297b94475a31854 --- .../src/com/vaadin/ui/AbstractComponent.java | 20 ++++++ .../vaadin/ui/AbstractComponentContainer.java | 13 ++-- .../ui/AbstractSingleComponentContainer.java | 6 ++ .../AddParentAsChild.java | 64 +++++++++++++++++++ .../SetParentAsContent.java | 45 +++++++++++++ 5 files changed, 139 insertions(+), 9 deletions(-) create mode 100644 server/tests/src/com/vaadin/tests/server/component/abstractcomponentcontainer/AddParentAsChild.java create mode 100644 server/tests/src/com/vaadin/tests/server/component/abstractsinglecomponentcontainer/SetParentAsContent.java diff --git a/server/src/com/vaadin/ui/AbstractComponent.java b/server/src/com/vaadin/ui/AbstractComponent.java index d980c27916..b6289e0b7d 100644 --- a/server/src/com/vaadin/ui/AbstractComponent.java +++ b/server/src/com/vaadin/ui/AbstractComponent.java @@ -991,4 +991,24 @@ public abstract class AbstractComponent extends AbstractClientConnector actionManager.removeAction(shortcut); } } + + /** + * Determine whether a content component is equal to, or the + * ancestor of this component. + * + * @param content + * the potential ancestor element + * @return true if the relationship holds + */ + protected boolean isOrHasAncestor(Component content) { + if (content instanceof HasComponents) { + for (Component parent = this; parent != null; parent = parent + .getParent()) { + if (parent == content) { + return true; + } + } + } + return false; + } } diff --git a/server/src/com/vaadin/ui/AbstractComponentContainer.java b/server/src/com/vaadin/ui/AbstractComponentContainer.java index 0aa185d557..b1e69ba76b 100644 --- a/server/src/com/vaadin/ui/AbstractComponentContainer.java +++ b/server/src/com/vaadin/ui/AbstractComponentContainer.java @@ -196,15 +196,10 @@ public abstract class AbstractComponentContainer extends AbstractComponent */ @Override public void addComponent(Component c) { - if (c instanceof ComponentContainer) { - // Make sure we're not adding the component inside it's own content - for (Component parent = this; parent != null; parent = parent - .getParent()) { - if (parent == c) { - throw new IllegalArgumentException( - "Component cannot be added inside it's own content"); - } - } + // Make sure we're not adding the component inside it's own content + if (isOrHasAncestor(c)) { + throw new IllegalArgumentException( + "Component cannot be added inside it's own content"); } if (c.getParent() != null) { diff --git a/server/src/com/vaadin/ui/AbstractSingleComponentContainer.java b/server/src/com/vaadin/ui/AbstractSingleComponentContainer.java index de1bb29846..ba108fc302 100644 --- a/server/src/com/vaadin/ui/AbstractSingleComponentContainer.java +++ b/server/src/com/vaadin/ui/AbstractSingleComponentContainer.java @@ -123,6 +123,12 @@ public abstract class AbstractSingleComponentContainer extends */ @Override public void setContent(Component content) { + // Make sure we're not adding the component inside it's own content + if (isOrHasAncestor(content)) { + throw new IllegalArgumentException( + "Component cannot be added inside it's own content"); + } + Component oldContent = getContent(); if (oldContent == content) { // do not set the same content twice diff --git a/server/tests/src/com/vaadin/tests/server/component/abstractcomponentcontainer/AddParentAsChild.java b/server/tests/src/com/vaadin/tests/server/component/abstractcomponentcontainer/AddParentAsChild.java new file mode 100644 index 0000000000..67e6d09adc --- /dev/null +++ b/server/tests/src/com/vaadin/tests/server/component/abstractcomponentcontainer/AddParentAsChild.java @@ -0,0 +1,64 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed 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 com.vaadin.tests.server.component.abstractcomponentcontainer; + +import java.util.Iterator; + +import org.easymock.EasyMock; +import org.junit.Test; + +import com.vaadin.ui.AbstractComponentContainer; +import com.vaadin.ui.Component; +import com.vaadin.ui.HasComponents; + +/** + * Tests for avoiding add parent as child for + * {@link AbstractComponentContainer#addComponent(Component)} + * + * @author Vaadin Ltd + */ +public class AddParentAsChild { + + @Test(expected = IllegalArgumentException.class) + public void testAddComponent() { + AbstractComponentContainer container = new ComponentContainer(); + HasComponents hasComponentsMock = EasyMock + .createMock(HasComponents.class); + container.setParent(hasComponentsMock); + + container.addComponent(hasComponentsMock); + } + + class ComponentContainer extends AbstractComponentContainer { + + @Override + public void replaceComponent(Component oldComponent, + Component newComponent) { + } + + @Override + public int getComponentCount() { + return 0; + } + + @Override + public Iterator iterator() { + return null; + } + + } + +} diff --git a/server/tests/src/com/vaadin/tests/server/component/abstractsinglecomponentcontainer/SetParentAsContent.java b/server/tests/src/com/vaadin/tests/server/component/abstractsinglecomponentcontainer/SetParentAsContent.java new file mode 100644 index 0000000000..dae0e57d02 --- /dev/null +++ b/server/tests/src/com/vaadin/tests/server/component/abstractsinglecomponentcontainer/SetParentAsContent.java @@ -0,0 +1,45 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed 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 com.vaadin.tests.server.component.abstractsinglecomponentcontainer; + +import org.easymock.EasyMock; +import org.junit.Test; + +import com.vaadin.ui.AbstractSingleComponentContainer; +import com.vaadin.ui.Component; +import com.vaadin.ui.HasComponents; + +/** + * + * Tests for avoiding set parent as child for + * {@link AbstractSingleComponentContainer#setContent(Component)} + * + * @author Vaadin Ltd + */ +public class SetParentAsContent { + + @Test(expected = IllegalArgumentException.class) + public void testSetContent() { + AbstractSingleComponentContainer container = new AbstractSingleComponentContainer() { + }; + HasComponents hasComponentsMock = EasyMock + .createMock(HasComponents.class); + container.setParent(hasComponentsMock); + + container.setContent(hasComponentsMock); + } + +} -- 2.39.5