You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by hl...@apache.org on 2008/07/19 19:49:47 UTC

svn commit: r678191 - in /tapestry/tapestry5/trunk/tapestry-core/src: main/java/org/apache/tapestry5/corelib/components/ main/java/org/apache/tapestry5/internal/bindings/ main/java/org/apache/tapestry5/internal/services/ site/apt/ test/app1/ test/java/...

Author: hlship
Date: Sat Jul 19 10:49:47 2008
New Revision: 678191

URL: http://svn.apache.org/viewvc?rev=678191&view=rev
Log:
TAPESTRY-2528: It should not be possible for component classes to be directly instantiated

Removed:
    tapestry/tapestry5/trunk/tapestry-core/src/site/apt/upgrade.apt
Modified:
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBindingFactory.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentClassTransformerImpl.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformationImpl.java
    tapestry/tapestry5/trunk/tapestry-core/src/test/app1/Start.tml
    tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/IntegrationTests.java
    tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Start.java
    tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/bindings/PropBindingFactoryTest.java

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java?rev=678191&r1=678190&r2=678191&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java Sat Jul 19 10:49:47 2008
@@ -226,8 +226,11 @@
 
         // Save the form element for later, in case we want to write an encoding type attribute.
 
-        form = writer
-                .element("form", "name", name, "id", name, "method", "post", "action", link);
+        form = writer.element("form",
+                              "name", name,
+                              "id", name,
+                              "method", "post",
+                              "action", link);
 
         componentInvocationMap.store(form, link);
 
@@ -239,7 +242,10 @@
         {
             String value = link.getParameterValue(parameterName);
 
-            writer.element("input", "type", "hidden", "name", parameterName, "value", value);
+            writer.element("input",
+                           "type", "hidden",
+                           "name", parameterName,
+                           "value", value);
             writer.end();
         }
 
@@ -262,11 +268,8 @@
         writer.end(); // form
 
         div.element("input",
-
                     "type", "hidden",
-
                     "name", FORM_DATA,
-
                     "value", actionSink.toBase64());
     }
 
@@ -281,7 +284,7 @@
         tracker = environment.pop(ValidationTracker.class);
     }
 
-    @SuppressWarnings({ "unchecked", "InfiniteLoopStatement" })
+    @SuppressWarnings({"unchecked", "InfiniteLoopStatement"})
     @Log
     Object onAction(EventContext context) throws IOException
     {
@@ -300,7 +303,6 @@
 
         try
         {
-
             ComponentResultProcessorWrapper callback = new ComponentResultProcessorWrapper(
                     componentEventResultProcessor);
 

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBindingFactory.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBindingFactory.java?rev=678191&r1=678190&r2=678191&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBindingFactory.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBindingFactory.java Sat Jul 19 10:49:47 2008
@@ -1,4 +1,4 @@
-// Copyright 2006, 2007 The Apache Software Foundation
+// Copyright 2006, 2007, 2008 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.
@@ -18,7 +18,6 @@
 import org.apache.tapestry5.ComponentResources;
 import org.apache.tapestry5.PropertyConduit;
 import org.apache.tapestry5.ioc.Location;
-import org.apache.tapestry5.ioc.internal.util.TapestryException;
 import org.apache.tapestry5.services.BindingFactory;
 import org.apache.tapestry5.services.PropertyConduitSource;
 
@@ -43,18 +42,11 @@
         Object target = container.getComponent();
         Class targetClass = target.getClass();
 
-        try
-        {
-            PropertyConduit conduit = source.create(targetClass, expression);
-
-            String toString = String.format("PropBinding[%s %s(%s)]", description, container
-                    .getCompleteId(), expression);
-
-            return new PropBinding(target, conduit, toString, location);
-        }
-        catch (Throwable ex)
-        {
-            throw new TapestryException(ex.getMessage(), location, ex);
-        }
+        PropertyConduit conduit = source.create(targetClass, expression);
+
+        String toString = String.format("PropBinding[%s %s(%s)]", description, container
+                .getCompleteId(), expression);
+
+        return new PropBinding(target, conduit, toString, location);
     }
 }

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentClassTransformerImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentClassTransformerImpl.java?rev=678191&r1=678190&r2=678191&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentClassTransformerImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentClassTransformerImpl.java Sat Jul 19 10:49:47 2008
@@ -57,10 +57,10 @@
 
     private final ComponentClassCache componentClassCache;
 
-    private final String[] SUBPACKAGES = { "." + InternalConstants.PAGES_SUBPACKAGE + ".",
+    private final String[] SUBPACKAGES = {"." + InternalConstants.PAGES_SUBPACKAGE + ".",
             "." + InternalConstants.COMPONENTS_SUBPACKAGE + ".",
             "." + InternalConstants.MIXINS_SUBPACKAGE + ".",
-            "." + InternalConstants.BASE_SUBPACKAGE + "." };
+            "." + InternalConstants.BASE_SUBPACKAGE + "."};
 
     /**
      * @param workerChain         the ordered list of class transform works as a chain of command instance
@@ -110,6 +110,13 @@
             return;
         }
 
+        // Is it an inner class (does the class name contain a '$')?
+        // Inner classes are loaded by the same class loader as the component, but are
+        // not components and are not transformed.
+
+
+        if (ctClass.getName().contains("$")) return;
+
         // Force the creation of the parent class.
 
         try

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformationImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformationImpl.java?rev=678191&r1=678190&r2=678191&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformationImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformationImpl.java Sat Jul 19 10:49:47 2008
@@ -125,8 +125,8 @@
      * Signature for newInstance() method of Instantiator.
      */
     private static final MethodSignature NEW_INSTANCE_SIGNATURE = new MethodSignature(Component.class, "newInstance",
-                                                                                      new Class[] {
-                                                                                              InternalComponentResources.class },
+                                                                                      new Class[]{
+                                                                                              InternalComponentResources.class},
                                                                                       null);
 
     /**
@@ -1329,6 +1329,14 @@
             CtMethod initializerMethod = defaultConstructor.toMethod(initializer, ctClass);
 
             ctClass.addMethod(initializerMethod);
+
+            // Replace the constructor body with one that fails.  This leaves, as an open question,
+            // what to do about any other constructors.
+
+            String body = String.format("throw new RuntimeException(\"%s\");",
+                                        ServicesMessages.forbidInstantiateComponentClass(getClassName()));
+
+            defaultConstructor.setBody(body);
         }
         catch (Exception ex)
         {
@@ -1576,7 +1584,7 @@
         String fieldType = getFieldType(fieldName);
 
         TransformMethodSignature sig = new TransformMethodSignature(Modifier.PRIVATE, "void", methodName,
-                                                                    new String[] { fieldType }, null);
+                                                                    new String[]{fieldType}, null);
 
         String message = ServicesMessages.readOnlyField(ctClass.getName(), fieldName);
 

Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/app1/Start.tml
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/app1/Start.tml?rev=678191&r1=678190&r2=678191&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/app1/Start.tml (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/app1/Start.tml Sat Jul 19 10:49:47 2008
@@ -62,6 +62,12 @@
             -- demo proper reporting of exceptions during class transformation
         </li>
 
+        <li>
+            <t:actionlink t:id="instantiatePage">Instantiate Page</t:actionlink>
+            --
+            attempt to instantiate (and return) a page instance
+        </li>
+
     </ul>
 
 </html>                      

Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/IntegrationTests.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/IntegrationTests.java?rev=678191&r1=678190&r2=678191&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/IntegrationTests.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/IntegrationTests.java Sat Jul 19 10:49:47 2008
@@ -31,7 +31,7 @@
  * Note: If these tests fail with BindException when starting Jetty, it could be Skype. At least on my system, Skype is
  * listening on localhost:80.
  */
-@Test(timeOut = 50000, sequential = true, groups = { "integration" })
+@Test(timeOut = 50000, sequential = true, groups = {"integration"})
 public class IntegrationTests extends AbstractIntegrationTestSuite
 {
     public IntegrationTests()
@@ -197,7 +197,7 @@
     {
         start("InstanceMixin");
 
-        final String[] dates = { "Jun 13, 1999", "Jul 15, 2001", "Dec 4, 2005" };
+        final String[] dates = {"Jun 13, 1999", "Jul 15, 2001", "Dec 4, 2005"};
 
         for (String date : dates)
         {
@@ -1339,7 +1339,7 @@
     {
         start("Disabled Fields");
 
-        String[] paths = new String[] { "//input[@id='textfield']",
+        String[] paths = new String[]{"//input[@id='textfield']",
 
                 "//input[@id='passwordfield']",
 
@@ -1363,7 +1363,7 @@
 
                 "//select[@id='palette']",
 
-                "//input[@id='submit']" };
+                "//input[@id='submit']"};
 
         for (String path : paths)
         {
@@ -2092,4 +2092,13 @@
                 "Parameter 'object' of component NullParameterDemo:beandisplay is bound to null.");
     }
 
+
+    @Test
+    public void component_classes_may_not_be_directly_instantiated()
+    {
+        start("Instantiate Page");
+
+        assertTextPresent(
+                "Component class org.apache.tapestry5.integration.app1.pages.Music may not be instantiated directly.");
+    }
 }

Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Start.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Start.java?rev=678191&r1=678190&r2=678191&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Start.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Start.java Sat Jul 19 10:49:47 2008
@@ -304,4 +304,10 @@
     {
         return Arrays.asList(1, 2, 3);
     }
+
+    /* This will fail, because component classes are not instantiable. */
+    public Object onActionFromInstantiatePage()
+    {
+        return new Music();
+    }
 }

Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/bindings/PropBindingFactoryTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/bindings/PropBindingFactoryTest.java?rev=678191&r1=678190&r2=678191&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/bindings/PropBindingFactoryTest.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/bindings/PropBindingFactoryTest.java Sat Jul 19 10:49:47 2008
@@ -399,13 +399,12 @@
             factory.newBinding("test binding", resources, null, propertyPath, l);
             unreachable();
         }
-        catch (TapestryException ex)
+        catch (RuntimeException ex)
         {
             assertEquals(
                     ex.getMessage(),
                     "Class org.apache.tapestry5.internal.bindings.StringHolder does not contain a property named \'missingProperty\' "
                             + "(within property expression \'stringHolder.missingProperty.terminalProperty\').  Available properties: value.");
-            assertSame(ex.getLocation(), l);
         }
 
         verify();
@@ -429,12 +428,11 @@
             factory.newBinding("test binding", resources, null, propertyPath, l);
             unreachable();
         }
-        catch (TapestryException ex)
+        catch (RuntimeException ex)
         {
             assertEquals(
                     ex.getMessage(),
                     "Property \'writeOnly\' of class org.apache.tapestry5.internal.bindings.TargetBean (within property expression \'writeOnly.terminalProperty\') is not readable (it has no read accessor method).");
-            assertSame(ex.getLocation(), l);
         }
 
         verify();
@@ -541,14 +539,13 @@
             factory.newBinding("test binding", resources, null, "missingProperty", l);
             unreachable();
         }
-        catch (TapestryException ex)
+        catch (RuntimeException ex)
         {
             assertEquals(
                     ex.getMessage(),
                     "Class org.apache.tapestry5.internal.bindings.TargetBean does not contain a property named \'missingProperty\' "
                             + "(within property expression \'missingProperty\').  "
                             + "Available properties: class, componentResources, intValue, objectValue, readOnly, stringHolder, writeOnly.");
-            assertSame(ex.getLocation(), l);
         }
 
         verify();
@@ -594,26 +591,26 @@
     {
         return new Object[][]
                 {
-                        { "true", true, },
-                        { "True", true, },
-                        { " true ", true, },
-                        { "false", false },
-                        { "null", null },
-                        { "3", 3l },
-                        { " 37 ", 37l },
-                        { " -227", -227l },
-                        { " 5.", 5d },
-                        { " -100.", -100d },
-                        { " -0.0 ", -0d },
-                        { "1..10", new IntegerRange(1, 10) },
-                        { " -20 .. -30 ", new IntegerRange(-20, -30) },
-                        { "0.", 0d },
-                        { " 227.75", 227.75d },
-                        { " -10123.67", -10123.67d },
-                        { "'Hello World'", "Hello World" },
-                        { " 'Whitespace Ignored' ", "Whitespace Ignored" },
-                        { " ' Inside ' ", " Inside " },
-                        { " 'Nested ' Quotes ' Inside'", "Nested ' Quotes ' Inside" },
-                        { "'''", "'" } };
+                        {"true", true,},
+                        {"True", true,},
+                        {" true ", true,},
+                        {"false", false},
+                        {"null", null},
+                        {"3", 3l},
+                        {" 37 ", 37l},
+                        {" -227", -227l},
+                        {" 5.", 5d},
+                        {" -100.", -100d},
+                        {" -0.0 ", -0d},
+                        {"1..10", new IntegerRange(1, 10)},
+                        {" -20 .. -30 ", new IntegerRange(-20, -30)},
+                        {"0.", 0d},
+                        {" 227.75", 227.75d},
+                        {" -10123.67", -10123.67d},
+                        {"'Hello World'", "Hello World"},
+                        {" 'Whitespace Ignored' ", "Whitespace Ignored"},
+                        {" ' Inside ' ", " Inside "},
+                        {" 'Nested ' Quotes ' Inside'", "Nested ' Quotes ' Inside"},
+                        {"'''", "'"}};
     }
 }