Uploaded image for project: 'Tapestry'
  1. Tapestry
  2. TAPESTRY-2547

Field validation is bypassed if form action url is used as a GET url

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Duplicate
    • Affects Version/s: 5.0.13
    • Fix Version/s: None
    • Component/s: tapestry-core
    • Labels:
      None

      Description

      We have a form, the simpliest one is ok, say this one on "TestPage" page :
      <t:form>
      <t:textfield t:id="field" t:validate="required" t:value="value" />
      <t:submit/>
      </t:form>

      This form is supposed to required a a non empty value for value.
      All goes fine if we click on ok, but if a twisted tester try to enter directly the action url in the browser ( t5app/testpage.form), the field level validation are bypassed (but all form events are throws and so the one done in "onValidateFormFrom" arecorrectly performed).

      The result is that the form may be successful with inconsistent data, in our case a null value.

        Issue Links

          Activity

          Hide
          fanf42 Francois Armand added a comment -

          A trivial workaround exists : we just have to perform all validation on onValidateFromForm, but it's really painful. Moreover, data consistency is always a critical area, and T5 is close to a final version, so I choose blocker priority.

          Show
          fanf42 Francois Armand added a comment - A trivial workaround exists : we just have to perform all validation on onValidateFromForm, but it's really painful. Moreover, data consistency is always a critical area, and T5 is close to a final version, so I choose blocker priority.
          Hide
          hlship Howard M. Lewis Ship added a comment -

          Typing a form's URL into as a GET will perform no work, because the t:formdata query parameter will be empty.

          I'm thinking of making Tapestry reject requests that use a GET to invoke a form, or have no t:formdata entries.

          Show
          hlship Howard M. Lewis Ship added a comment - Typing a form's URL into as a GET will perform no work, because the t:formdata query parameter will be empty. I'm thinking of making Tapestry reject requests that use a GET to invoke a form, or have no t:formdata entries.
          Hide
          hlship Howard M. Lewis Ship added a comment -

          Also, there's no validation, because no field updates will occur, because there's no t:formdata.

          Show
          hlship Howard M. Lewis Ship added a comment - Also, there's no validation, because no field updates will occur, because there's no t:formdata.
          Hide
          fanf42 Francois Armand added a comment -

          I believe that rejecting the form is a good option, for now I can't see any cases where it would be a good idea to try to handle such a malformed form.

          Show
          fanf42 Francois Armand added a comment - I believe that rejecting the form is a good option, for now I can't see any cases where it would be a good idea to try to handle such a malformed form.
          Hide
          mlusetti Massimo Lusetti added a comment -

          Well I've external devices (handheld / palm PCs) which are used to collect data and perform operations then when are put back into the cradle there's a custom application which download data from the devices and send it to a custom tapestry page/component which process the data and simply return a status code.

          I've to hardcode (sort of) the t:formdata parameter into the application to let tapestry correctly handle the http post and this turned out to work as expected but this put the upgrade of server side (T5 web application) and client side (T5 IoC swing application) into a nightmare cause t:formadata parameter changes so i cannot upgrade the server side without release and distribute and upgrade to all clients.

          So this could be a use case where having T5 process the form without t:formdata parameter is a very big plus.

          It's just me using the power of T5 this way?

          This is all production code already in use since April.

          Show
          mlusetti Massimo Lusetti added a comment - Well I've external devices (handheld / palm PCs) which are used to collect data and perform operations then when are put back into the cradle there's a custom application which download data from the devices and send it to a custom tapestry page/component which process the data and simply return a status code. I've to hardcode (sort of) the t:formdata parameter into the application to let tapestry correctly handle the http post and this turned out to work as expected but this put the upgrade of server side (T5 web application) and client side (T5 IoC swing application) into a nightmare cause t:formadata parameter changes so i cannot upgrade the server side without release and distribute and upgrade to all clients. So this could be a use case where having T5 process the form without t:formdata parameter is a very big plus. It's just me using the power of T5 this way? This is all production code already in use since April.
          Hide
          joshcanfield Josh Canfield added a comment -

          I was recently working on a page that needed to accept query parameters but where the request was not generated from a form submit. Consider "http://localhost/app/search?field=title&value=hello&sort=asc" as a REST api, where sort is optional. Putting this into an activation context is not so appealing.

          I was thinking that it would be great to have an annotation @QueryParameter, or modify @Parameter to accept query=true. You could attach validators that triggered an error event on failure...

          I ended up coding it up by calling _request.getParameter() directly...

          Show
          joshcanfield Josh Canfield added a comment - I was recently working on a page that needed to accept query parameters but where the request was not generated from a form submit. Consider "http://localhost/app/search?field=title&value=hello&sort=asc" as a REST api, where sort is optional. Putting this into an activation context is not so appealing. I was thinking that it would be great to have an annotation @QueryParameter, or modify @Parameter to accept query=true. You could attach validators that triggered an error event on failure... I ended up coding it up by calling _request.getParameter() directly...
          Hide
          hlship Howard M. Lewis Ship added a comment -

          An outright reject of the form submission should get the job done.

          Show
          hlship Howard M. Lewis Ship added a comment - An outright reject of the form submission should get the job done.
          Hide
          dcouderc David Couderc added a comment -

          You can still bypass validation by forging the parameter, there should be a way to ensure that the parameter has not been tampered with.
          Also, the t:formdata may reveal data that you do not expect : for instance the loop component, used whitout a
          PrimaryKeyEncoder expose the whole object (with private fields or children classes you may not even be aware of).
          Maybe the parameter should be encripted too.

          Show
          dcouderc David Couderc added a comment - You can still bypass validation by forging the parameter, there should be a way to ensure that the parameter has not been tampered with. Also, the t:formdata may reveal data that you do not expect : for instance the loop component, used whitout a PrimaryKeyEncoder expose the whole object (with private fields or children classes you may not even be aware of). Maybe the parameter should be encripted too.

            People

            • Assignee:
              hlship Howard M. Lewis Ship
              Reporter:
              fanf42 Francois Armand
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development