You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@click.apache.org by sa...@apache.org on 2010/11/20 07:20:53 UTC

svn commit: r1037119 - in /click/trunk/click: examples/webapp/assets/js/imports.js framework/src/META-INF/resources/click/control.js framework/src/org/apache/click/control/Form.java framework/test/org/apache/click/control/FormTest.java

Author: sabob
Date: Sat Nov 20 06:20:53 2010
New Revision: 1037119

URL: http://svn.apache.org/viewvc?rev=1037119&view=rev
Log:
removed bypass validation as it raised security concerns. CLK-726

Modified:
    click/trunk/click/examples/webapp/assets/js/imports.js
    click/trunk/click/framework/src/META-INF/resources/click/control.js
    click/trunk/click/framework/src/org/apache/click/control/Form.java
    click/trunk/click/framework/test/org/apache/click/control/FormTest.java

Modified: click/trunk/click/examples/webapp/assets/js/imports.js
URL: http://svn.apache.org/viewvc/click/trunk/click/examples/webapp/assets/js/imports.js?rev=1037119&r1=1037118&r2=1037119&view=diff
==============================================================================
--- click/trunk/click/examples/webapp/assets/js/imports.js (original)
+++ click/trunk/click/examples/webapp/assets/js/imports.js Sat Nov 20 06:20:53 2010
@@ -387,37 +387,29 @@ function validateForm(msgs, id, align, s
 }
 
 /**
- * Submit the form and optionally validate the fields.
+ * Submit the form and checks that no field or button is called 'submit' as
+ * it causes JS exceptions.
  *
- * Usage: <input onclick="Click.submit(form, false)">
+ * Usage: <input onclick="Click.submit(form)">
  */
-Click.submit=function(form, validate) {
+Click.submit=function(form) {
     if (typeof form == 'undefined') {
-        alert('Error: form is undefined. Usage: Click.submit(formName)');
+        alert('Error: form is undefined. Usage: Click.submit(form)');
         return false;
     }
 
-    // Validate is true by default
-    validate = (typeof validate == 'undefined') ? true : validate;
-
-    if (form) {
+if (form) {
         var formElements = form.elements;
         for (var i=0; i < formElements.length; i++) {
             var el = formElements[i];
     		    if(el.name=='submit'){
-               alert('Error: In order to submit the Form through JavaScript, buttons and fields must not be named "submit". Please rename the button/field called "submit".');
-               return false;
-           }
-        }
-        if (!validate) {
-            // Bypass JS validation
-            var input = document.getElementById(form.id + '_bypass_validation');
-            if (input) {
-                input.value='true';
+                alert('Error: In order to submit the Form through JavaScript, buttons and fields must not be named "submit". Please rename the button/field called "submit".');
+                return false;
             }
         }
-        form.submit();
     }
+    form.submit();
+    return true;
 }
 
 /* Validate Functions */

Modified: click/trunk/click/framework/src/META-INF/resources/click/control.js
URL: http://svn.apache.org/viewvc/click/trunk/click/framework/src/META-INF/resources/click/control.js?rev=1037119&r1=1037118&r2=1037119&view=diff
==============================================================================
--- click/trunk/click/framework/src/META-INF/resources/click/control.js (original)
+++ click/trunk/click/framework/src/META-INF/resources/click/control.js Sat Nov 20 06:20:53 2010
@@ -387,35 +387,27 @@ function validateForm(msgs, id, align, s
 }
 
 /**
- * Submit the form and optionally validate the fields.
+ * Submit the form and checks that no field or button is called 'submit' as
+ * it causes JS exceptions.
  *
- * Usage: <input onclick="Click.submit(form, false)">
+ * Usage: <input onclick="Click.submit(form)">
  */
-Click.submit=function(form, validate) {
+Click.submit=function(form) {
     if (typeof form == 'undefined') {
         alert('Error: form is undefined. Usage: Click.submit(form)');
         return false;
     }
 
-    // Validate is true by default
-    validate = (typeof validate == 'undefined') ? true : validate;
-
-    if (form) {
+if (form) {
         var formElements = form.elements;
         for (var i=0; i < formElements.length; i++) {
             var el = formElements[i];
     		    if(el.name=='submit'){
-               alert('Error: In order to submit the Form through JavaScript, buttons and fields must not be named "submit". Please rename the button/field called "submit".');
-               return false;
-           }
-        }
-        if (!validate) {
-            // Bypass JS validation
-            var input = document.getElementById(form.id + '_bypass_validation');
-            if (input) {
-                input.value='true';
+                alert('Error: In order to submit the Form through JavaScript, buttons and fields must not be named "submit". Please rename the button/field called "submit".');
+                return false;
             }
         }
-        form.submit();
     }
+    form.submit();
+    return true;
 }

Modified: click/trunk/click/framework/src/org/apache/click/control/Form.java
URL: http://svn.apache.org/viewvc/click/trunk/click/framework/src/org/apache/click/control/Form.java?rev=1037119&r1=1037118&r2=1037119&view=diff
==============================================================================
--- click/trunk/click/framework/src/org/apache/click/control/Form.java (original)
+++ click/trunk/click/framework/src/org/apache/click/control/Form.java Sat Nov 20 06:20:53 2010
@@ -471,31 +471,34 @@ import org.apache.commons.lang.StringUti
  * and in a hidden field in the form to ensure a form post isn't replayed.
  *
  * <a name="dynamic-forms"></a>
- * <h3>Dynamic Forms and bypassing validation</h3>
+ * <h3>Dynamic Forms and <em>not</em> validating a request</h3>
  *
  * A common use case for web applications is to create Form fields dynamically
  * based upon user selection. For example if a checkbox is ticked another Field
  * is added to the Form. A simple way to achieve this is using JavaScript
  * to submit the Form when the Field is changed or clicked.
  * <p/>
- * When submitting a Form using JavaScript, it is often undesirable to validate
- * the fields since the user is still filling out the form.
- * To cater for this use case, Form provides the ability to bypass the validation
- * step. A special hidden field called, {@link #BYPASS_VALIDATION}, is added to
- * the Form which controls whether validation should be bypassed or not.
- * <p/>
- * Form also provides a JavaScript function (part of the <tt>"/click/control.js"</tt>
- * resource) that will submit the Form and optionally bypasses validation. The
- * JavaScript function is:
- * <tt>"Click.submit(form, validate)"</tt> and can be used as follows:
+ * When submitting a Form using JavaScript, it is often desirable to <em>not</em>
+ * validate the fields since the user is still filling out the form.
+ * To cater for this use case, Form provides the {@link #setValidate(boolean)}
+ * to switch off form and field validation. For example:
+ *
  * <pre class="prettyprint">
- * Form form = new Form("myform");
+ * public void onInit() {
+ *     checkbox.setAttribute("onclick", "form.submit()");
  *
- * // The second argument to Click.submit is "false", meaning validation is bypassed
- * checkbox.setAttribute("onclick", "Click.submit(form, false)");
+ *     // Since onInit occurs before the onProcess event,
+ *     // we have to explicitly bind the submit button in the onInit event if we
+ *     // want to check if it was clicked.
+ *     // If the submit button wasn't clicked it means the Form was submitted
+ *     // using JavaScript and we don't want to validate yet
+ *     ClickUtils.bind(submit);
  *
- * // For a Select field use the "onchange" JavaScript event instead
- * select.setAttribute("onchange", "Click.submit(form, false)"); </pre>
+ *     // If submit was not clicked, don't validate
+ *     if(form.isFormSubmission() && !submit.isClicked()) {
+ *         form.setValidate(false);
+ *     }
+ * } </pre>
  *
  * <p>&nbsp;<p/>
  * See also the W3C HTML reference:
@@ -549,12 +552,6 @@ public class Form extends AbstractContai
      */
     public static final String FORM_NAME = "form_name";
 
-    /**
-     * The parameter name for bypassing form validation: &nbsp;
-     * <tt>"bypass_validation"</tt>.
-     */
-    public static final String BYPASS_VALIDATION = "bypass_validation";
-
     /** The HTTP content type header for multipart forms. */
     public static final String MULTIPART_FORM_DATA = "multipart/form-data";
 
@@ -670,9 +667,6 @@ public class Form extends AbstractContai
      */
     private int insertIndexOffset; // Ensures hiddenFields added by Form are always at the end of the controlList
 
-    /** Flag indicating whether validation is bypassed or not. */
-    private Boolean bypassValidation = null;
-
     // Constructors -----------------------------------------------------------
 
     /**
@@ -1299,26 +1293,6 @@ public class Form extends AbstractContai
     }
 
     /**
-     * Return whether or not validation is bypassed for this request. This flag
-     * is controlled by a request parameter called {@link #BYPASS_VALIDATION}.
-     * If this request parameter is "<tt>true</tt>", validation will be bypassed
-     * for this request.
-     * <p/>
-     * This feature is useful for dynamic forms where JavaScript events are
-     * registered on Controls which in turn submits the Form.
-     * <p/>
-     * For more details see
-     *
-     * @return true if validation should be bypassed, false otherwise
-     */
-    public boolean isBypassValidation() {
-        if (bypassValidation == null) {
-            bypassValidation = "true".equals(getContext().getRequestParameter(BYPASS_VALIDATION));
-        }
-        return bypassValidation;
-    }
-
-    /**
      * Set the name of the form.
      *
      * @see org.apache.click.Control#setName(String)
@@ -1334,15 +1308,6 @@ public class Form extends AbstractContai
         this.name = name;
 
         // TODO: Remove with stateful pages
-        HiddenField bypassValidationField = (HiddenField) getField(BYPASS_VALIDATION);
-        if (bypassValidationField == null) {
-            // Create a hidden field which name and value cannot be change
-            bypassValidationField = new ImmutableHiddenField(BYPASS_VALIDATION, Boolean.FALSE);
-            add(bypassValidationField);
-            insertIndexOffset++;
-        }
-
-        // TODO: Remove with stateful pages
         HiddenField nameField = (HiddenField) getField(FORM_NAME);
         if (nameField == null) {
             // Create a hidden field that won't be processed and name cannot change
@@ -1422,17 +1387,11 @@ public class Form extends AbstractContai
     /**
      * Return true if the Form fields should validate themselves when being
      * processed.
-     * <p/>
-     * If {@link #isBypassValidation()} returns true, Form fields will not be
-     * validated.
      *
      * @return true if the form fields should perform validation when being
      *  processed
      */
     public boolean getValidate() {
-        if (isBypassValidation()) {
-            return false;
-        }
         return validate;
     }
 
@@ -2028,7 +1987,6 @@ public class Form extends AbstractContai
     public void onDestroy() {
         super.onDestroy();
 
-        bypassValidation = null;
         formSubmission = null;
         setError(null);
     }
@@ -2495,19 +2453,16 @@ public class Form extends AbstractContai
      */
     protected void renderFields(HtmlStringBuffer buffer) {
 
-        // If Form contains only FORM_NAME and BYPASS_VALIDATION HiddenFields,
-        // exit early
-        if (getControls().size() == 2) {
+        // If Form contains only the FORM_NAME HiddenField, exit early
+        if (getControls().size() == 1) {
 
             // getControlMap is cheaper than getFieldMap, so check that first
-            if (getControlMap().containsKey(FORM_NAME)
-                && getControlMap().containsKey(BYPASS_VALIDATION)) {
+            if (getControlMap().containsKey(FORM_NAME)) {
                 return;
 
             } else {
                 Map<String, Field> fieldMap = ContainerUtils.getFieldMap(this);
-                if (fieldMap.containsKey(FORM_NAME)
-                    && fieldMap.containsKey(BYPASS_VALIDATION)) {
+                if (fieldMap.containsKey(FORM_NAME)) {
                     return;
                 }
             }
@@ -2820,9 +2775,6 @@ public class Form extends AbstractContai
             buffer.append("</td></tr>\n");
         }
 
-        // Reset bypass flag to ensure it does not influence the validate flag
-        bypassValidation = Boolean.FALSE;
-
         // Render JavaScript form validation code
         if (isJavaScriptValidation()) {
             buffer.append("<tr style=\"display:none\" id=\"");
@@ -2945,9 +2897,6 @@ public class Form extends AbstractContai
      */
     protected void renderValidationJavaScript(HtmlStringBuffer buffer, List<Field> formFields) {
 
-        // Reset bypass flag to ensure it does not influence the validate flag
-        bypassValidation = Boolean.FALSE;
-
         // Render JavaScript form validation code
         if (isJavaScriptValidation()) {
             List<String> functionNames = new ArrayList<String>();

Modified: click/trunk/click/framework/test/org/apache/click/control/FormTest.java
URL: http://svn.apache.org/viewvc/click/trunk/click/framework/test/org/apache/click/control/FormTest.java?rev=1037119&r1=1037118&r2=1037119&view=diff
==============================================================================
--- click/trunk/click/framework/test/org/apache/click/control/FormTest.java (original)
+++ click/trunk/click/framework/test/org/apache/click/control/FormTest.java Sat Nov 20 06:20:53 2010
@@ -178,10 +178,9 @@ public class FormTest extends TestCase {
         // Create form.
         testForm = new Form("form");
 
-        // Form automatically creates and adds two HiddenFields. One for storing
-        // the form anem between requests and one for tracking if form validation
-        // is bypassed. The form name field is at index 1 at the start
-        // of each test.
+        // Form automatically creates and adds one HiddenField for storing
+        // the form name between requests. The form name field is at index 1
+        // at the start of each test.
         // The tests below checks the trackField index position in the Form
         // for the lists Form#controls, Form#fieldList and Form#buttonList
         trackField = (HiddenField) testForm.getField(Form.FORM_NAME);
@@ -268,9 +267,9 @@ public class FormTest extends TestCase {
      * Form#buttonList should not.
      */
     public void testInsertOrderAfterInsertingButton() {
-        // Check that the fieldList includes only two hidden fields (FORM_NAME
-        // and BYPASS_VALIDATION) thus size is 2
-        assertTrue(testForm.getFieldList().size() == 2);
+        // Check that the fieldList includes only hidden field, FORM_NAME,
+        // thus size is 1
+        assertTrue(testForm.getFieldList().size() == 1);
 
         Button button = new Button("button1");
 
@@ -281,7 +280,7 @@ public class FormTest extends TestCase {
         // button index: #buttonList=0
         assertTrue(testForm.getButtonList().indexOf(button) == 0);
         // Check that button was not added to fieldList accidentally
-        assertTrue(testForm.getFieldList().size() == 2);
+        assertTrue(testForm.getFieldList().size() == 1);
     }
 
     /**
@@ -315,33 +314,36 @@ public class FormTest extends TestCase {
 
         testForm.insert(field, 0);
 
+        int expectedIndex = 0;
         // field index: #controls=0
-        assertTrue(testForm.getControls().indexOf(field) == 0);
+        assertEquals(expectedIndex, testForm.getControls().indexOf(field));
         // field index: #fieldList=0
-        assertTrue(testForm.getFieldList().indexOf(field) == 0);
+        assertEquals(expectedIndex, testForm.getFieldList().indexOf(field));
 
+        int expectedSize = 1;
         // trackField index: #controls=1
-        assertTrue(testForm.getControls().indexOf(trackField) == 1);
+        assertEquals(expectedSize, testForm.getControls().indexOf(trackField));
         // trackField index: #fieldList=1
-        assertTrue(testForm.getFieldList().indexOf(trackField) == 1);
+        assertEquals(expectedSize, testForm.getFieldList().indexOf(trackField));
 
-        int expectedSize = 3;
-        // Check the list sizes to be 3
-        assertTrue(testForm.getControls().size() == expectedSize);
-        assertTrue(testForm.getFieldList().size() == expectedSize);
+        expectedSize = 2;
+        // Check the list sizes to be 2
+        assertEquals(expectedSize, testForm.getControls().size());
+        assertEquals(expectedSize, testForm.getFieldList().size());
 
         // Removing field should shift up trackField index
         testForm.remove(field);
 
-        expectedSize = 2;
-        // Check the list sizes to be 2
-        assertTrue(testForm.getControls().size() == expectedSize);
-        assertTrue(testForm.getFieldList().size() == expectedSize);
+        expectedSize = 1;
+        // Check the list sizes to be 1
+        assertEquals(expectedSize, testForm.getControls().size());
+        assertEquals(expectedSize, testForm.getFieldList().size());
 
         // trackField index: #controls=0
-        assertTrue(testForm.getControls().indexOf(trackField) == 0);
+        expectedSize = 0;
+        assertEquals(expectedSize, testForm.getControls().indexOf(trackField));
         // trackField index: #fieldList=0
-        assertTrue(testForm.getFieldList().indexOf(trackField) == 0);
+        assertEquals(expectedSize, testForm.getFieldList().indexOf(trackField));
     }
 
     /**
@@ -756,14 +758,14 @@ public class FormTest extends TestCase {
     public void testReplaceFields() {
         Form form = new Form("form");
 
-        // Add two fields named child1 and child2
+        // Add two fields named child1 and child2 (form auto adds FORM_NAME HiddenField as well)
         Field child1 = new TextField("child1");
         Field child2 = new TextField("child2");
         form.add(child1);
         form.add(child2);
-        assertEquals(4, form.getControlMap().size());
-        assertEquals(4, form.getControls().size());
-        assertEquals(4, form.getFields().size());
+        assertEquals(3, form.getControlMap().size());
+        assertEquals(3, form.getControls().size());
+        assertEquals(3, form.getFields().size());
         assertSame(child1, form.getControls().get(0));
         assertSame(child2, form.getControls().get(1));
         assertSame(child1, form.getFieldList().get(0));
@@ -775,9 +777,9 @@ public class FormTest extends TestCase {
         child2 = new TextField("child2");
         form.add(child1);
         form.add(child2);
-        assertEquals(4, form.getControlMap().size());
-        assertEquals(4, form.getControls().size());
-        assertEquals(4, form.getFields().size());
+        assertEquals(3, form.getControlMap().size());
+        assertEquals(3, form.getControls().size());
+        assertEquals(3, form.getFields().size());
         assertSame(child1, form.getControls().get(0));
         assertSame(child2, form.getControls().get(1));
         assertSame(child1, form.getFieldList().get(0));
@@ -792,14 +794,14 @@ public class FormTest extends TestCase {
     public void testReplaceButtons() {
         Form form = new Form("form");
 
-        // Add two fields named child1 and child2
+        // Add two fields named child1 and child2 (form auto adds FORM_NAME HiddenField as well)
         Button child1 = new Button("child1");
         Button child2 = new Button("child2");
         form.add(child1);
         form.add(child2);
-        assertEquals(4, form.getControlMap().size());
-        assertEquals(4, form.getControls().size());
-        assertEquals(4, form.getFields().size());
+        assertEquals(3, form.getControlMap().size());
+        assertEquals(3, form.getControls().size());
+        assertEquals(3, form.getFields().size());
         assertSame(child1, form.getControls().get(0));
         assertSame(child2, form.getControls().get(1));
         assertSame(child1, form.getButtonList().get(0));
@@ -811,9 +813,9 @@ public class FormTest extends TestCase {
         child2 = new Button("child2");
         form.add(child1);
         form.add(child2);
-        assertEquals(4, form.getControlMap().size());
-        assertEquals(4, form.getControls().size());
-        assertEquals(4, form.getFields().size());
+        assertEquals(3, form.getControlMap().size());
+        assertEquals(3, form.getControls().size());
+        assertEquals(3, form.getFields().size());
         assertSame(child1, form.getControls().get(0));
         assertSame(child2, form.getControls().get(1));
         assertSame(child1, form.getButtonList().get(0));