Repository: tapestry-5 Updated Branches: refs/heads/master 7345bfb29 -> d9b3813a0
TAP5-1481: In production mode, component event requests that reference an unknown component should respond with a 404 Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/d9b3813a Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/d9b3813a Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/d9b3813a Branch: refs/heads/master Commit: d9b3813a09c0d685b28b36192f578b01744af725 Parents: 7345bfb Author: Howard M. Lewis Ship <[email protected]> Authored: Fri Aug 1 10:11:11 2014 -0700 Committer: Howard M. Lewis Ship <[email protected]> Committed: Fri Aug 1 10:11:11 2014 -0700 ---------------------------------------------------------------------- .../tapestry5/internal/InternalConstants.java | 12 ++++ .../services/ComponentEventDispatcher.java | 11 +++- .../ComponentEventRequestHandlerImpl.java | 2 - .../internal/services/PageRenderDispatcher.java | 11 +++- .../ProductionModeUnknownComponentFilter.java | 64 ++++++++++++++++++++ .../tapestry5/internal/structure/Page.java | 3 +- .../tapestry5/modules/TapestryModule.java | 58 ++++++++++-------- tapestry-core/src/test/conf/testng.xml | 2 +- .../integration/app5/ProductionModeTests.groovy | 23 +++++++ .../integration/app5/services/AppModule.groovy | 2 +- .../services/ComponentEventDispatcherTest.java | 10 ++- 11 files changed, 162 insertions(+), 36 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/d9b3813a/tapestry-core/src/main/java/org/apache/tapestry5/internal/InternalConstants.java ---------------------------------------------------------------------- diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/InternalConstants.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/InternalConstants.java index c069f11..7fa95e9 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/InternalConstants.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/InternalConstants.java @@ -186,4 +186,16 @@ public final class InternalConstants * @since 5.4 */ public static final String SUPPRESS_CORE_STYLESHEETS = "tapestry.suppress-core-stylesheets"; + + /** + * A bit of a hack that allows, in production mode, for a component event request to "unwind" when + * the component referenced in the URL does not exist. This is related to TAP5-1481. This situation + * can most likely occur when a web spider, such as Google, uses an old component event URI from + * a prior deployment, which no longer works in a new deployment, due to structural changes. Since + * changing the APIs that significantly is forbidden, a non-null value is added as an + * {@link org.apache.tapestry5.services.Request} attribute. + * + * @since 5.4 + */ + public static final String REFERENCED_COMPONENT_NOT_FOUND = "tapestry.referenced-component-not-found"; } http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/d9b3813a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventDispatcher.java ---------------------------------------------------------------------- diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventDispatcher.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventDispatcher.java index ef16591..1d0bd5d 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventDispatcher.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventDispatcher.java @@ -1,5 +1,3 @@ -// Copyright 2006, 2007, 2008, 2009 The Apache Software Foundation -// // 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 @@ -14,6 +12,7 @@ package org.apache.tapestry5.internal.services; +import org.apache.tapestry5.internal.InternalConstants; import org.apache.tapestry5.services.*; import java.io.IOException; @@ -43,8 +42,16 @@ public class ComponentEventDispatcher implements Dispatcher if (parameters == null) return false; + // Inside this pipeline, may find that the component id does not exist (this check only occurs in production + // mode) ... + componentRequestHandler.handleComponentEvent(parameters); + // ... in which case, this attribute is set. + if (request.getAttribute(InternalConstants.REFERENCED_COMPONENT_NOT_FOUND) != null) { + return false; + } + return true; } } http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/d9b3813a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventRequestHandlerImpl.java ---------------------------------------------------------------------- diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventRequestHandlerImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventRequestHandlerImpl.java index 9450722..e1247c7 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventRequestHandlerImpl.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventRequestHandlerImpl.java @@ -1,5 +1,3 @@ -// Copyright 2006, 2007, 2008, 2009, 2010, 2011 The Apache Software Foundation -// // 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://git-wip-us.apache.org/repos/asf/tapestry-5/blob/d9b3813a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageRenderDispatcher.java ---------------------------------------------------------------------- diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageRenderDispatcher.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageRenderDispatcher.java index 39da112..1fb7b3e 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageRenderDispatcher.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageRenderDispatcher.java @@ -1,5 +1,3 @@ -// Copyright 2006, 2007, 2008, 2009 The Apache Software Foundation -// // 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 @@ -14,6 +12,7 @@ package org.apache.tapestry5.internal.services; +import org.apache.tapestry5.internal.InternalConstants; import org.apache.tapestry5.services.*; import java.io.IOException; @@ -37,6 +36,14 @@ public class PageRenderDispatcher implements Dispatcher public boolean dispatch(Request request, final Response response) throws IOException { + // If a component event request arrives (in production) + // with an invalid component id, then we want it to be a 404 + // See TAP5-1481 + + if (request.getAttribute(InternalConstants.REFERENCED_COMPONENT_NOT_FOUND) != null) + { + return false; + } PageRenderRequestParameters parameters = linkEncoder.decodePageRenderRequest(request); http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/d9b3813a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ProductionModeUnknownComponentFilter.java ---------------------------------------------------------------------- diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ProductionModeUnknownComponentFilter.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ProductionModeUnknownComponentFilter.java new file mode 100644 index 0000000..9f95796 --- /dev/null +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ProductionModeUnknownComponentFilter.java @@ -0,0 +1,64 @@ +// 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 org.apache.tapestry5.internal.services; + +import org.apache.tapestry5.internal.InternalConstants; +import org.apache.tapestry5.internal.structure.Page; +import org.apache.tapestry5.ioc.util.UnknownValueException; +import org.apache.tapestry5.services.*; + +import java.io.IOException; + +/** + * A filter, used only in production mode, that does a "pre-flight check" that the indicated component + * actually exists. If it does not, then the handling of the component event is aborted and other + * hooks will ensure that request ultimately becomes a 404. + * + * @since 5.4 + */ +public class ProductionModeUnknownComponentFilter implements ComponentRequestFilter +{ + private final Request request; + + private final RequestPageCache cache; + + public ProductionModeUnknownComponentFilter(Request request, RequestPageCache cache) + { + this.request = request; + this.cache = cache; + } + + @Override + public void handleComponentEvent(ComponentEventRequestParameters parameters, ComponentRequestHandler handler) throws IOException + { + Page containerPage = cache.get(parameters.getContainingPageName()); + + try + { + containerPage.getComponentElementByNestedId(parameters.getNestedComponentId()); + + handler.handleComponentEvent(parameters); + + } catch (UnknownValueException ex) + { + request.setAttribute(InternalConstants.REFERENCED_COMPONENT_NOT_FOUND, true); + } + } + + @Override + public void handlePageRender(PageRenderRequestParameters parameters, ComponentRequestHandler handler) throws IOException + { + // Pass these through to the default handler. + handler.handlePageRender(parameters); + } +} http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/d9b3813a/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/Page.java ---------------------------------------------------------------------- diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/Page.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/Page.java index 48d1d64..3099c72 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/Page.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/Page.java @@ -15,6 +15,7 @@ package org.apache.tapestry5.internal.structure; import org.apache.tapestry5.ComponentResources; import org.apache.tapestry5.beaneditor.NonVisual; import org.apache.tapestry5.ioc.services.PerthreadManager; +import org.apache.tapestry5.ioc.util.UnknownValueException; import org.apache.tapestry5.runtime.Component; import org.apache.tapestry5.runtime.PageLifecycleCallbackHub; import org.apache.tapestry5.runtime.PageLifecycleListener; @@ -170,7 +171,7 @@ public interface Page extends PageLifecycleCallbackHub * string) returns the root * element of the page. * - * @throws IllegalArgumentException + * @throws UnknownValueException * if the nestedId does not correspond to a component */ ComponentPageElement getComponentElementByNestedId(String nestedId); http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/d9b3813a/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java ---------------------------------------------------------------------- diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java b/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java index 2d852a9..a90003f 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java @@ -97,7 +97,6 @@ import org.slf4j.Logger; import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - import java.io.IOException; import java.lang.annotation.Annotation; import java.math.BigDecimal; @@ -1729,7 +1728,7 @@ public final class TapestryModule @Symbol(SymbolConstants.PRODUCTION_MODE) boolean productionMode, - + @Symbol(SymbolConstants.INCLUDE_CORE_STACK) final boolean includeCoreStack, @@ -1812,8 +1811,9 @@ public final class TapestryModule configuration.add("ClientBehaviorSupport", clientBehaviorSupport, "after:JavaScriptSupport"); configuration.add("Heartbeat", heartbeat); configuration.add("ValidationDecorator", defaultValidationDecorator); - - if (includeCoreStack) { + + if (includeCoreStack) + { configuration.add("ImportCoreStack", importCoreStack); } @@ -2057,8 +2057,6 @@ public final class TapestryModule configuration.add(SymbolConstants.MIN_GZIP_SIZE, 100); - Random random = new Random(System.currentTimeMillis()); - configuration.add(SymbolConstants.APPLICATION_VERSION, "0.0.1"); configuration.add(SymbolConstants.OMIT_GENERATOR_META, false); @@ -2116,25 +2114,25 @@ public final class TapestryModule // TAP5-2070 keep the old behavior, defaults to false configuration.add(MetaDataConstants.UNKNOWN_ACTIVATION_CONTEXT_CHECK, false); - + // TAP5-2197 configuration.add(SymbolConstants.INCLUDE_CORE_STACK, true); - + // TAP5-2182 configuration.add(SymbolConstants.FORM_GROUP_WRAPPER_CSS_CLASS, "form-group"); configuration.add(SymbolConstants.FORM_GROUP_LABEL_CSS_CLASS, "control-label"); configuration.add(SymbolConstants.FORM_GROUP_FORM_FIELD_WRAPPER_ELEMENT_NAME, ""); configuration.add(SymbolConstants.FORM_GROUP_FORM_FIELD_WRAPPER_ELEMENT_CSS_CLASS, ""); configuration.add(SymbolConstants.FORM_FIELD_CSS_CLASS, "form-control"); - + // TAP5-1998 configuration.add(SymbolConstants.LENIENT_DATE_FORMAT, false); - + // TAP5-2187 configuration.add(SymbolConstants.STRICT_CSS_URL_REWRITING, false); configuration.add(SymbolConstants.EXCEPTION_REPORTS_DIR, "build/exceptions"); - + // TAP5-1815 configuration.add(SymbolConstants.ENABLE_HTML5_SUPPORT, false); @@ -2442,6 +2440,8 @@ public final class TapestryModule * <dl> * <dt>OperationTracker</dt> * <dd>Tracks general information about the request using {@link OperationTracker}</dd> + * <dt>UnknownComponentFilter (production mode only)</dt> + * <dd>{@link org.apache.tapestry5.internal.services.ProductionModeUnknownComponentFilter} - Detects request with unknown component and aborts handling to ultimately deliver a 404 response</dd> * <dt>InitializeActivePageName * <dd>{@link InitializeActivePageName} * <dt>DeferredResponseRenderer</dt> @@ -2450,9 +2450,15 @@ public final class TapestryModule * * @since 5.2.0 */ - public void contributeComponentRequestHandler(OrderedConfiguration<ComponentRequestFilter> configuration) + public void contributeComponentRequestHandler(OrderedConfiguration<ComponentRequestFilter> configuration, @Symbol(SymbolConstants.PRODUCTION_MODE) boolean productionMode) { configuration.addInstance("OperationTracker", RequestOperationTracker.class); + + if (productionMode) + { + configuration.addInstance("UnknownComponentFilter", ProductionModeUnknownComponentFilter.class); + } + configuration.addInstance("InitializeActivePageName", InitializeActivePageName.class); configuration.addInstance("DeferredResponseRenderer", DeferredResponseRenderer.class); } @@ -2670,13 +2676,15 @@ public final class TapestryModule { return strategyBuilder.build(ValueLabelProvider.class, configuration); } - + @Advise(serviceInterface = ComponentInstantiatorSource.class) - public static void componentReplacer(MethodAdviceReceiver methodAdviceReceiver, - final ComponentOverride componentReplacer) throws NoSuchMethodException, SecurityException { - - if (componentReplacer.getReplacements().size() > 0) { - + public static void componentReplacer(MethodAdviceReceiver methodAdviceReceiver, + final ComponentOverride componentReplacer) throws NoSuchMethodException, SecurityException + { + + if (componentReplacer.getReplacements().size() > 0) + { + MethodAdvice advice = new MethodAdvice() { @Override @@ -2684,30 +2692,30 @@ public final class TapestryModule { String className = (String) invocation.getParameter(0); final Class<?> replacement = componentReplacer.getReplacement(className); - if (replacement != null) + if (replacement != null) { invocation.setParameter(0, replacement.getName()); } invocation.proceed(); } }; - + methodAdviceReceiver.adviseMethod( ComponentInstantiatorSource.class.getMethod("getInstantiator", String.class), advice); - + } } - + public static ComponentLibraryInfoSource buildComponentLibraryInfoSource(List<ComponentLibraryInfoSource> configuration, - ChainBuilder chainBuilder) + ChainBuilder chainBuilder) { return chainBuilder.build(ComponentLibraryInfoSource.class, configuration); } - + @Contribute(ComponentLibraryInfoSource.class) public static void addMavenComponentLibraryInfoSource(OrderedConfiguration<ComponentLibraryInfoSource> configuration) { configuration.addInstance("Maven", MavenComponentLibraryInfoSource.class); } - + } http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/d9b3813a/tapestry-core/src/test/conf/testng.xml ---------------------------------------------------------------------- diff --git a/tapestry-core/src/test/conf/testng.xml b/tapestry-core/src/test/conf/testng.xml index 9e8921a..6a6acac 100644 --- a/tapestry-core/src/test/conf/testng.xml +++ b/tapestry-core/src/test/conf/testng.xml @@ -78,7 +78,7 @@ </packages> </test> - <test name="App Skinning Tests" enabled="true"> + <test name="App Skinning and Misc. Tests" enabled="true"> <packages> <package name="org.apache.tapestry5.integration.app5"/> </packages> http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/d9b3813a/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app5/ProductionModeTests.groovy ---------------------------------------------------------------------- diff --git a/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app5/ProductionModeTests.groovy b/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app5/ProductionModeTests.groovy new file mode 100644 index 0000000..265ac0b --- /dev/null +++ b/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app5/ProductionModeTests.groovy @@ -0,0 +1,23 @@ +package org.apache.tapestry5.integration.app5 + +import org.apache.tapestry5.integration.TapestryCoreTestCase +import org.apache.tapestry5.test.TapestryTestConfiguration +import org.testng.annotations.Test + + +@TapestryTestConfiguration(webAppFolder = "src/test/app5") +class ProductionModeTests extends TapestryCoreTestCase { + + @Test + void invalid_component_id_is_404() + { + openBaseURL() + + assertTitle "Default Layout" + + def invalid = new URL("${baseURL}index.missing") + + assertEquals 404, invalid.openConnection().responseCode + } + +} http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/d9b3813a/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app5/services/AppModule.groovy ---------------------------------------------------------------------- diff --git a/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app5/services/AppModule.groovy b/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app5/services/AppModule.groovy index 49f784c..04c413a 100644 --- a/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app5/services/AppModule.groovy +++ b/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app5/services/AppModule.groovy @@ -31,7 +31,7 @@ import org.apache.tapestry5.services.pageload.ComponentResourceSelector class AppModule { void contributeApplicationDefaults(MappedConfiguration conf) { - conf.add(SymbolConstants.PRODUCTION_MODE, false) + conf.add(SymbolConstants.PRODUCTION_MODE, true) conf.add(SymbolConstants.SUPPORTED_LOCALES, "en,fr") // Override to test TAP5-2361 conf.add(SymbolConstants.BOOTSTRAP_ROOT, "context:bootstrap") http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/d9b3813a/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ComponentEventDispatcherTest.java ---------------------------------------------------------------------- diff --git a/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ComponentEventDispatcherTest.java b/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ComponentEventDispatcherTest.java index 416b588..b3f17a1 100644 --- a/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ComponentEventDispatcherTest.java +++ b/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ComponentEventDispatcherTest.java @@ -1,5 +1,3 @@ -// Copyright 2007, 2008, 2009, 2011, 2012 The Apache Software Foundation -// // 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 @@ -162,6 +160,8 @@ public class ComponentEventDispatcherTest extends InternalBaseTestCase train_getParameter(request, InternalConstants.CONTAINER_PAGE_NAME, null); + expect(request.getAttribute(InternalConstants.REFERENCED_COMPONENT_NOT_FOUND)).andStubReturn(null); + train_for_request_locale(request, ls); handler.handleComponentEvent(expectedParameters); @@ -203,6 +203,8 @@ public class ComponentEventDispatcherTest extends InternalBaseTestCase train_getParameter(request, InternalConstants.CONTAINER_PAGE_NAME, "mypage"); + expect(request.getAttribute(InternalConstants.REFERENCED_COMPONENT_NOT_FOUND)).andStubReturn(null); + train_canonicalizePageName(resolver, "mypage", "mypage"); train_for_request_locale(request, ls); @@ -271,6 +273,8 @@ public class ComponentEventDispatcherTest extends InternalBaseTestCase train_getParameter(request, InternalConstants.CONTAINER_PAGE_NAME, null); + expect(request.getAttribute(InternalConstants.REFERENCED_COMPONENT_NOT_FOUND)).andStubReturn(null); + handler.handleComponentEvent(expectedParameters); train_for_request_locale(request, localizationSetter); @@ -326,6 +330,8 @@ public class ComponentEventDispatcherTest extends InternalBaseTestCase train_getParameter(request, InternalConstants.CONTAINER_PAGE_NAME, null); + expect(request.getAttribute(InternalConstants.REFERENCED_COMPONENT_NOT_FOUND)).andStubReturn(null); + train_for_request_locale(request, localizationSetter); handler.handleComponentEvent(expectedParameters);
