Author: jkuhnert
Date: Mon May 28 11:57:36 2007
New Revision: 542297
URL: http://svn.apache.org/viewvc?view=rev&rev=542297
Log:
Fixes TAPESTRY-638. Added detection of component reference recursion to avoid
stack overflows as well as properly report the line precise location of the
recursive reference.
Modified:
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageLoader.java
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageloadMessages.java
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageloadStrings.properties
tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/pageload/PageLoaderTest.java
Modified:
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageLoader.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageLoader.java?view=diff&rev=542297&r1=542296&r2=542297
==============================================================================
---
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageLoader.java
(original)
+++
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageLoader.java
Mon May 28 11:57:36 2007
@@ -31,10 +31,7 @@
import org.apache.tapestry.services.ComponentTemplateLoader;
import org.apache.tapestry.spec.*;
-import java.util.ArrayList;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Locale;
+import java.util.*;
/**
* Implementation of tapestry.page.PageLoader. Runs the process of building the
@@ -42,7 +39,8 @@
* <p>
* This implementation is not threadsafe, therefore the pooled service model
* must be used.
- *
+ * </p>
+ *
* @author Howard Lewis Ship
*/
@@ -147,6 +145,12 @@
private ClassResolver _classResolver;
+ /**
+ * As each component is constructed it is placed on to the component
stack, when construction is finished it is pushed
+ * back off the stack. This helps in detecting component nesting and
properly reporting errors.
+ */
+ private Stack _componentStack = new Stack();
+
public void initializeService()
{
@@ -349,6 +353,8 @@
if (_depth > _maxDepth)
_maxDepth = _depth;
+ beginConstructComponent(container, containerSpec);
+
String defaultBindingPrefix =
_componentPropertySource.getComponentProperty(container,
TapestryConstants.DEFAULT_BINDING_PREFIX_NAME);
List ids = new ArrayList(containerSpec.getComponentIds());
@@ -364,7 +370,7 @@
// container's specification.
IContainedComponent contained = containerSpec.getComponent(id);
-
+
String type = contained.getType();
Location location = contained.getLocation();
@@ -415,12 +421,57 @@
{
throw new
ApplicationRuntimeException(PageloadMessages.unableToInstantiateComponent(container,
ex),
container, null, ex);
+ } finally {
+
+ endConstructComponent(container);
}
_depth--;
}
/**
+ * Checks the component stack to ensure that the specified component
hasn't been improperly nested
+ * and referenced recursively within itself.
+ *
+ * @param component
+ * The component to add to the current component stack and check
for recursion.
+ * @param specification
+ * The specification of the specified component.
+ */
+ void beginConstructComponent(IComponent component, IComponentSpecification
specification)
+ {
+ // check recursion
+
+ int position = _componentStack.search(component);
+ if (position > -1)
+ {
+ Location location = specification.getLocation();
+
+ // try to get the more precise container position location that
was referenced
+ // in the template to properly report the precise position of the
recursive reference
+
+ IContainedComponent container = component.getContainedComponent();
+ if (container != null)
+ location = container.getLocation();
+
+ throw new
ApplicationRuntimeException(PageloadMessages.recursiveComponent(component),
location, null);
+ }
+
+ _componentStack.push(component);
+ }
+
+ /**
+ * Pops the current component off the stack.
+ *
+ * @param component
+ * The component that has just been constructed.
+ */
+ void endConstructComponent(IComponent component)
+ {
+ _componentStack.pop();
+ }
+
+ /**
* Invoked to create an implicit component (one which is defined in the
* containing component's template, rather that in the containing
* component's specification).
@@ -572,6 +623,7 @@
_count = 0;
_depth = 0;
_maxDepth = 0;
+ _componentStack.clear();
_locale = _threadLocale.getLocale();
Modified:
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageloadMessages.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageloadMessages.java?view=diff&rev=542297&r1=542296&r2=542297
==============================================================================
---
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageloadMessages.java
(original)
+++
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageloadMessages.java
Mon May 28 11:57:36 2007
@@ -146,4 +146,9 @@
{
return _formatter.format("component-not-found", id);
}
+
+ public static String recursiveComponent(IComponent component)
+ {
+ return _formatter.format("recursive-component", component);
+ }
}
Modified:
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageloadStrings.properties
URL:
http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageloadStrings.properties?view=diff&rev=542297&r1=542296&r2=542297
==============================================================================
---
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageloadStrings.properties
(original)
+++
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageloadStrings.properties
Mon May 28 11:57:36 2007
@@ -30,3 +30,4 @@
used-parameter-alias=Parameter {2} (for component {1}, at {0}) was bound; this
parameter has been deprecated, bind parameter {3} instead.
deprecated-parameter=Parameter {0} (at {1}) has been deprecated, and may be
removed in a future release. Consult the documentation for component {2} to
determine an appropriate replacement.
component-not-found=No component found in tree for EventListener binding with
a matching component id of {0}.
+recursive-component=Component {0} has recursive reference to itself. This is
most likely caused by referencing the same component in your html template.
Modified:
tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/pageload/PageLoaderTest.java
URL:
http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/pageload/PageLoaderTest.java?view=diff&rev=542297&r1=542296&r2=542297
==============================================================================
---
tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/pageload/PageLoaderTest.java
(original)
+++
tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/pageload/PageLoaderTest.java
Mon May 28 11:57:36 2007
@@ -118,12 +118,6 @@
verify();
}
- private void trainCreateBinding(BindingSource source, IComponent
container, String description,
- String expression, String defaultBindingPrefix, Location l,
IBinding binding)
- {
- expect(source.createBinding(container, description, expression,
defaultBindingPrefix, l)).andReturn(binding);
- }
-
private void trainCreateBinding(BindingSource source, IComponent
container, IParameterSpecification ps, String description,
String expression, String defaultBindingPrefix, Location l,
IBinding binding)
{
