[ 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
