[jira] Commented: (VELTOOLS-52) ValidatorTool javascript generator can generate invalid Javascript

["Nathan Bubna (JIRA)" <ve...@apache.org>]

    [ http://issues.apache.org/jira/browse/VELTOOLS-52?page=comments#action_12332243 ] 

Nathan Bubna commented on VELTOOLS-52:
--------------------------------------

Hmm.  Interesting.  Looking over the JavascriptValidatorTag code in Struts 1.2.7 and Struts 1.3 shows a whole varying degree of escaping going on for var values.

- Our ValidatorTool is largely a copy/paste of previous JavascriptValidatorTag code and only escapes backslashes in var values.
- In Struts 1.2.7, the tag escapes backslashes and double quotes.
- In the current Struts head (1.3.x, i believe), the tag escapes backslashes, double quotes, and single quotes.

Now you would have us add \n and \r to that list. :)  Well, seeing as this appears to take the general trend to the next level, i'm inclined to accept the patch.

I do have a few questions though (for any takers):

Has concern over escaping \n and \r been brought up on dev@struts?  I'd be curious to hear their thoughts, and they might appreciate the tip before they push a 1.3 final out the door.

Struts just uses ValidatorUtils.replace() multiple times to do its escaping.   Why shouldn't we use the ValidatorUtils.replace() method to do this escaping rather than roll our own javascriptEscape() method?  I'm guessing the reason is performance.  Is javascriptEscape() enough faster to warrant maintaining it ourselves?   And, if so, I'm betting the commons-validator team would appreciate the contribution.

> ValidatorTool javascript generator can generate invalid Javascript
> ------------------------------------------------------------------
>
>          Key: VELTOOLS-52
>          URL: http://issues.apache.org/jira/browse/VELTOOLS-52
>      Project: VelocityTools
>         Type: Bug
>   Components: VelocityStruts
>     Versions: 1.2
>  Environment: Using JDK1.4.2 / Linux 2.4 kernel / Tomcat 4.1
>     Reporter: Christopher Schultz
>     Assignee: Nathan Bubna
>      Fix For: 1.2
>  Attachments: ValidatorTool.diff
>
> ValidatorTool can create invalid javascript in a few situations.
> Here is an example of such a situation and also an example of the invalid javascript it generates.
> Suppose you have the following dynamic action form validation rules defined (this is actually text field which is intended to be used as an "other" input when a drop-down has the value of "Other").
> <pre>
>       <field property="selectOther"
>              depends="validwhen,maxlength"
> 	     page="1">
>         <arg0 key="prompt.selectOther"/>
> 	<arg1 name="maxlength" key="${var:maxlength}" resource="false" />
> 	<var><var-name>maxlength</var-name><var-value>255</var-value></var>
>         <var>
> 	    <var-name>test</var-name>
> 	    <var-value>
>                 (((select == "Other") and (*this* != null)) or
> 		(select != "Other"))
> 	    </var-value>
> 	</var>
>       </field>
> </pre>
> When ValidatorTool generates Javascript for this, you get the following:
> <pre>
>     .
>     .
>     .
>     this.a3 = new Array("orgTypeOther", "The field Organization Type cannot be greater than 255 characters.", new Function ("varName", "this.maxlength='255'; this.test='(((orgType == "Other") and (*this* != null)) or
> 		(orgType != "Other"))';  return this[varName];"));
>     .
>     .
>     .
> </pre>
> Note that there is a newline in the string literal (invalid) and that the double-quotes used in my "validwhen" rule have not been escaped, which prematurely ends the double-quoted string starting with <code>"this.maxlength</code>, which really confuses the Javascript interpreter.
> It turns out that switching from double-quotes to single-quotes doesn't help, since there are also single-quoted strings within that double-quoted string, so basically it won't work no matter what you do (since backslash-escaping the quotes will cause the validwhen test itself to become invalid.
> I see two solutions: properly escape the variable values being dumped into Javascript, or avoid adding the "test" variable to the Javascript, since it will be ignored, anyway.
> I propose fixing the escaping, since there may be other validator "var" values with this same problem.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org