You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "David Shepherdson (JIRA)" <ji...@apache.org> on 2007/12/06 11:23:43 UTC

[jira] Commented: (WICKET-1094) Values over-escaped by FormTester

    [ https://issues.apache.org/jira/browse/WICKET-1094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12548982 ] 

David Shepherdson commented on WICKET-1094:
-------------------------------------------

Hello Johan,

(Sorry for the delay in getting back to you -- I've been busy on some other things.)

Were you thinking something along the lines of:

Hello Johan,

(Sorry for the delay in getting back to you -- I've been busy on some other things.)

Were you thinking something along the lines of the following?

    String value = formCompoennt.getValue();
    if (formComponent.getEscapeModelStrings()) {
        value = Strings.unescapeMarkup(value);
    }

There are two reasons I didn't do it that way in my workaround: firstly, there is no Strings.unescapeMarkup(...) method :-) and secondly, we don't know from the outside of getValue() whether the value inside really has been escaped or not: if there wasn't any raw input, the value returned will be from the model, without any escaping regardless of the setting. We'd have to do something like:

    String value = formComponent.getValue();
    if (formComponent.getEscapeModelStrings() && formComponent.hasRawInput()) {
        value = Strings.unescapeMarkup(value);
    }

(Assuming Strings.unescapeMarkup(...) existed, again.)

That seems to me like it's giving the FormTester knowledge that it shouldn't really have of the way getValue() works inside. That's why I felt the approach of ensuring that model strings wouldn't be escaped before calling getValue() (and then restoring the original setting afterwards) was safest and kind of cleanest.

But then, I'm just a user. As long as it can be made to work, you developers can do it however you like! :-)

David

> Values over-escaped by FormTester
> ---------------------------------
>
>                 Key: WICKET-1094
>                 URL: https://issues.apache.org/jira/browse/WICKET-1094
>             Project: Wicket
>          Issue Type: Bug
>    Affects Versions: 1.3.0-beta4
>            Reporter: David Shepherdson
>             Fix For: 1.3.0-rc2
>
>
> FormTester's constructor contains code that visits all components on the form and calls setFormComponentValue(formComponent, formComponent.getValue()) (or variations thereof), to store the component's value in the servlet request parameters. However, by default, FormComponent's getValue() method uses Strings.escapeMarkup(...) to encode any special characters, such as angle brackets. This is fine in a 'real' environment, where a Web browser will be responsible for displaying the escaped characters with their proper values, and so the proper value will be the one that comes through when the form is submitted; however, in the Wicket test environment, there isn't anything to do that extra level of 'un-escaping', meaning there's a danger of the form components being given escaped values when the form is submitted.
> For example, we have a form that contains a text area, whose value contains a URI enclosed in angle brackets, like so:
>     < http://test.com/ >
> When we submit the form with a Web browser, the value set on the model is exactly that string -- '< http://test.com/ >'. However, when we test our page with FormTester, the FormTester constructor calls getValue() on the component, which by default returns the escaped form:
>     &lt; http://test.com/ &gt;
> When the form is submitted, this is the value set on the model, and so comparisons to the original string fail.
> (Extra spaces inserted into the strings above to make them display properly in JIRA.)
> However, if FormTester were to call setEscapeModelStrings(false) on the form component before calling getValue() (and then restore the original escaping setting afterwards), then the value that ends up being provided to the component at the end would be the correct (unescaped) value, matching the behaviour when using the page in a browser.
> We have worked around this issue by overriding FormTester with a class that performs a second traversal of the form component hierarchy after calling the FormTester constructor, replacing the incorrectly escaped values with the proper ones (changes marked with '// O-P'):
>     public OurFormTester(String path, Form workingForm, BaseWicketTester wicketTester, boolean fillBlankString) {
>         super(path, workingForm, wicketTester, fillBlankString);
>         fixFormParameterValues(workingForm, fillBlankString);
>     }
>     
>     protected void fixFormParameterValues(Form workingForm, final boolean fillBlankString) {
>         workingForm.visitFormComponents(new FormComponent.AbstractVisitor()
>         {
>             public void onFormComponent(final FormComponent formComponent)
>             {
>                 // do nothing for invisible component
>                 if (!formComponent.isVisibleInHierarchy())
>                 {
>                     return;
>                 }
>                 // O-P Preserve old escaping value, then turn escaping off
>                 // so that values aren't escaped unnecessarily.
>                 boolean oldEscaping = formComponent.getEscapeModelStrings();
>                 formComponent.setEscapeModelStrings(false);
>                 
>                 // if component is text field and do not have exist value, fill
>                 // blank String if required
>                 if (formComponent instanceof AbstractTextComponent)
>                 {
>                     if (Strings.isEmpty(formComponent.getValue()))
>                     {
>                         if (fillBlankString)
>                         {
>                             setFormComponentValue(formComponent, "");
>                         }
>                     }
>                     else
>                     {
>                         setFormComponentValue(formComponent, formComponent.getValue());
>                     }
>                 }
>                 else if ( (formComponent instanceof DropDownChoice) ||
>                         (formComponent instanceof RadioChoice) ||
>                         (formComponent instanceof CheckBox))
>                 {
>                     setFormComponentValue(formComponent, formComponent.getValue());
>                 }
>                 else if (formComponent instanceof ListMultipleChoice)
>                 {
>                     final String[] modelValues = formComponent.getValue().split(FormComponent.VALUE_SEPARATOR);
>                     for (int i = 0; i < modelValues.length; i++)
>                     {
>                         addFormComponentValue(formComponent, modelValues[i]);
>                     }
>                 }
>                 else if (formComponent instanceof CheckGroup)
>                 {
>                     final Collection checkGroupValues = (Collection) formComponent.getModelObject();
>                     formComponent.visitChildren(Check.class, new IVisitor()
>                     {
>                         public Object component(Component component)
>                         {
>                             
>                             if (checkGroupValues.contains(component.getModelObject()))
>                             {
>                                 // O-P Preserve old escaping value, then turn escaping off
>                                 // so that values aren't escaped unnecessarily.
>                                 boolean oldEscaping = component.getEscapeModelStrings();
>                                 component.setEscapeModelStrings(false);
>                                 
>                                 addFormComponentValue(formComponent, ((Check) component).getValue());
>                                 
>                                 // O-P Restore the previous escaping setting.
>                                 component.setEscapeModelStrings(oldEscaping);
>                            }
>                             return CONTINUE_TRAVERSAL;
>                         }
>                     });
>                 }
>                 
>                 // O-P Restore the previous escaping setting.
>                 formComponent.setEscapeModelStrings(oldEscaping);
>             }
>         });
>     }

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.