velocity-dev@jakarta.apache.org
[Top] [All Lists]

[jira] Commented: (VELTOOLS-52) ValidatorTool javascript generator can g

Subject: [jira] Commented: (VELTOOLS-52) ValidatorTool javascript generator can generate invalid Javascript
From: "Christopher Schultz (JIRA)"
Date: Mon, 17 Oct 2005 21:24:45 +0200 CEST
    [ 
http://issues.apache.org/jira/browse/VELTOOLS-52?page=comments#action_12332279 
] 

Christopher Schultz commented on VELTOOLS-52:
---------------------------------------------

I'm not sure about any Struts concerns about newlines. I'm a lot lss 
plugged-into the struts user groups then I was a year or so ago, unfortunately.

I think that using ValidatorUtils.replace multiple times is pretty foolish, 
since you're re-processing the String multiple times. It's not really even any 
easier to understand than some other ways to do it. I chose to write my own 
replacement function precicely for performance reasons. I originally "upgraded" 
the existing, StringTokenizer-based, escape() method by simply adding 
single-quotes, backslashes, and newlines to the mix.

I looked-up the javadoc for StringTokenizer just to make sure I remembered what 
I was doing (sine I rarely use that class). The StringTokenizer javadoc 
basically says "don't use me anymore", so I thought I'd switch to regular 
expressions, instead. I wasn't sure about the JDK version requirements for 
validator-utils, but I did know that Jakarta-ORO is already required. So, I 
wrote 4 tests for performance:

1. My own home-brewed solution (the one in the patch)
2. One that uses JDK 1.4 regular expressions
3. One that uses Jakarta-ORO regular expressions
4. The original StringTokenizer-based solution, with additional code to support 
the additional characters to escape

Here are my performance benchmarks based upon a set of ~50-character strings 
with 0,1,2,3, and many replacements within that string. The "0 (Long String)" 
test was a string of ~250 characters with no replacements. Each test was run 
100,000 times with each replacement method, and the figures are in milliseconds.

Replacements    Home-brew:      JDK:    ORO:    StringTokenizer
0       125     402     688     161
1       244     732     1020    621
2       273     865     1202    2718
3       303     1023    1435    758
Many    903     4937    6417    2318
0 (Long String) 1645    4245    7228    3434

The results are somewhat predictable: knowing exactly what I was replacing has 
allowed me to beat the other implementations pretty easily. What was suprising 
is that the JDK regular expression implementation is pretty much the next-best 
thing. I would have bet on the StringTokenizer.

At any rate... it's simpler to modify the StringTokenizer or 
regular-expression-based escapers than the one that I wrote, so we might want 
to do with something other than my hand-rolled escaper. I just didn't want 
anyone complaining about performance ;)


> 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@xxxxxxxxxxxxxxxxxx
For additional commands, e-mail: velocity-dev-help@xxxxxxxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>