You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by "Christopher Schultz (JIRA)" <ve...@apache.org> on 2005/10/16 17:00:45 UTC

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

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
 Assigned to: Nathan Bubna 
     Fix For: 1.2


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


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

Posted by "Christopher Schultz (JIRA)" <ve...@apache.org>.
     [ http://issues.apache.org/jira/browse/VELTOOLS-52?page=all ]

Christopher Schultz updated VELTOOLS-52:
----------------------------------------

    Attachment: ValidatorTool.diff

Patch to fix output escaping.

This replaces ValidatorToo.escapeQuotes with a method that escapes not only double quotes, but also single-quotes, backslashes, carriage-returns, and newlines. I also applied this method to more than just the error messages for each validation rule, but also to the variable values themselves.


> 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


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

Posted by "Nathan Bubna (JIRA)" <ve...@apache.org>.
    [ http://issues.apache.org/jira/browse/VELTOOLS-52?page=comments#action_12332293 ] 

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

I don't really have any contacts there.  I mostly just lurk on struts-dev.  If an opportunity comes up, i'll suggest it to them.

in the meantime, i applied the patch locally and did my usual "bikeshed painting" (as Henning calls it :) to make it fit our code formatting patterns.  While doing that, I found myself wondering if the following variation on your code would be as performant:

    protected String escapeJavascript(String str)
    {
        if (str == null)
        {
            return null;
        }
        int length = str.length();
        if (length == 0)
        {
            return str;
        }

        // guess at how many chars we'll be adding...
        StringBuffer out = new StringBuffer(length + 4);
        // run through the string escaping sensitive chars
        for (int i=0; i < length; i++)
        {
            char c = str.charAt(i);
            if (c == '"'  ||
                c == '\'' ||
                c == '\\' || 
                c == '\n' || 
                c == '\r')
            {
                out.append('\\');
            }
            out.append(c)
        }
        return out.toString();
    }

it would certainly be easier to understand and maintain (for me, at least :) as above.  but i don't want to step on your toes (and testing) if your original is/was much faster or better for some reason i'm missing.  What say ye?

> 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


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

Posted by "Christopher Schultz (JIRA)" <de...@velocity.apache.org>.
    [ https://issues.apache.org/jira/browse/VELTOOLS-52?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12467200 ] 

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

Since I'm the only one who has ever really noticed this, I don't think that blocking veltools-1.3 is necessary.


> ValidatorTool javascript generator can generate invalid Javascript
> ------------------------------------------------------------------
>
>                 Key: VELTOOLS-52
>                 URL: https://issues.apache.org/jira/browse/VELTOOLS-52
>             Project: Velocity Tools
>          Issue Type: Bug
>          Components: VelocityStruts
>    Affects Versions: 1.2
>         Environment: Using JDK1.4.2 / Linux 2.4 kernel / Tomcat 4.1
>            Reporter: Christopher Schultz
>         Assigned To: 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.
-
You can reply to this email to add a comment to the issue online.


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


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

Posted by "Niall Pemberton (JIRA)" <ve...@apache.org>.
    [ http://issues.apache.org/jira/browse/VELTOOLS-52?page=comments#action_12332292 ] 

Niall Pemberton commented on VELTOOLS-52:
-----------------------------------------

The newline issue has come up in struts-world before, although I can't find actually find a reference on any of the mailing lists or in bugzilla. Any how, we don't have an open bugzilla for this issue and there aren't lots of people clamouring for it to be corrected - so I guess there aren't too many people who are using the "validwhen" validator with client side validation. If it does get raised then the workaround is to have everything on one line. My interest is more in doing away with most of whats in Struts's JavascriptValidatorTag (and Velocity's ValidatorTool), rather than patching it:

    http://issues.apache.org/bugzilla/show_bug.cgi?id=32343

... but I haven't spent any time on it for a while.

Anyway, I've openned a bugzilla ticket in Struts for this - thanks.

   http://issues.apache.org/bugzilla/show_bug.cgi?id=37131


> 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


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

Posted by "Nathan Bubna (JIRA)" <ve...@apache.org>.
    [ http://issues.apache.org/jira/browse/VELTOOLS-52?page=comments#action_12332416 ] 

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

Ok, since you're ok with it, i'll go with the simpler implementation.  Either way, the method is faster and will work right.  And performance is second fiddle to maintainability in my book.

> 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


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

Posted by "Christopher Schultz (JIRA)" <de...@velocity.apache.org>.
    [ https://issues.apache.org/jira/browse/VELTOOLS-52?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12467120 ] 

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

I'm not sure if this is fixed, and I'm a little confused, actually.

I have a 'validwhen' test that looks like this in my validator config file:

( ( ( ( premature == null ) and ( *this* == null ) ) or ( premature == 'false' ) ) or ( ( premature == 'true' ) and ( *this* != null ) ) )

(it requires the current field if the the value of another radio button field is 'true')

When rendered in javascript, this is the code that gets emitted in myForm_integerValidations:

     this.a1 = new Array("prematurityWeeks", "The field premature weeks must be an integer.", new Function ("varName", "this.test='( ( ( ( premature == null ) and ( *this* == null ) ) or ( premature == \'false\' ) ) or ( ( premature == \'true\' ) and ( *this* != null ) ) )';  return this[varName];"));

If you look, you'll see that the second argument to "new Function" is a double-quoted string. That double-quoted string contains a single-quoted string which contains the test from my config file which has it's single quotes escaped. This looks right to me.

But, it fails in Mozilla Firefox (2.0.0.1). I can't see anything in MSIE 7... the javascript does not appear to run at all (all attempts to see what's going on fail).

Here is the error:

Error: missing ; before statement
Source File: http://[host]/[action].do
Line: 186, Column: 83
Source Code:
this.test='( ( ( ( premature == null ) and ( *this* == null ) ) or ( premature == 'false' ) ) or ( ( premature == 'true' ) and ( *this* != null ) ) )';  return this[varName];

Note that the escape characters are missing.

I'm not javascript expert, but I'm guessing that we're trying to create a new function with the second argument as the code of that function. That means that the code within that argument will be executed in order to set "this.test" on the validator object to the text that I have in my configuration file.

Somewhere along the way, the escaping is being removed.

Is it possible that those single-quotes need to be double-escaped due to a second round of string-processing done by the javascript processor?


> ValidatorTool javascript generator can generate invalid Javascript
> ------------------------------------------------------------------
>
>                 Key: VELTOOLS-52
>                 URL: https://issues.apache.org/jira/browse/VELTOOLS-52
>             Project: Velocity Tools
>          Issue Type: Bug
>          Components: VelocityStruts
>    Affects Versions: 1.2
>         Environment: Using JDK1.4.2 / Linux 2.4 kernel / Tomcat 4.1
>            Reporter: Christopher Schultz
>         Assigned To: 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.
-
You can reply to this email to add a comment to the issue online.


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


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

Posted by "Christopher Schultz (JIRA)" <ve...@apache.org>.
    [ http://issues.apache.org/jira/browse/VELTOOLS-52?page=comments#action_12332422 ] 

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

Nathan,
I totally agree about maintainability versus performance -- particularly when the performance difference in negligible.

Note that the method replaceChar is completely unused. I missed that my first time through.


> 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


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

Posted by "Niall Pemberton (JIRA)" <ve...@apache.org>.
    [ http://issues.apache.org/jira/browse/VELTOOLS-52?page=comments#action_12356957 ] 

Niall Pemberton commented on VELTOOLS-52:
-----------------------------------------

Thanks Christopher/Nathan, I just copied your escapeJavasript() method into the Struts tag:

            http://svn.apache.org/viewcvs?rev=331261&view=rev

> 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


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

Posted by "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


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

Posted by "Nathan Bubna (JIRA)" <de...@velocity.apache.org>.
    [ https://issues.apache.org/jira/browse/VELTOOLS-52?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12467161 ] 

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

Frankly, i have no idea.  Even when i was using VelocityStruts, i was barely using the Validator stuff.  I know precious little here.  If you can fix this without breaking anything else, then i'll be happy to accept a patch.

Of course, right now we're in the middle of a vote to do a release candidate for VelocityTools 1.3, so this isn't the most wonderful of timing.   :-/   Do you think this is a significant enough problem (if you are even sure this is a problem on our end) to postpone a release candidate until it is fixed?  Or is this some small corner case that can be patched later for a VelocityTools 1.3.1 release?

Niall?  Are you out there?  I'd love to hear your thoughts on this too.

> ValidatorTool javascript generator can generate invalid Javascript
> ------------------------------------------------------------------
>
>                 Key: VELTOOLS-52
>                 URL: https://issues.apache.org/jira/browse/VELTOOLS-52
>             Project: Velocity Tools
>          Issue Type: Bug
>          Components: VelocityStruts
>    Affects Versions: 1.2
>         Environment: Using JDK1.4.2 / Linux 2.4 kernel / Tomcat 4.1
>            Reporter: Christopher Schultz
>         Assigned To: 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.
-
You can reply to this email to add a comment to the issue online.


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


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

Posted by "Christopher Schultz (JIRA)" <ve...@apache.org>.
    [ http://issues.apache.org/jira/browse/VELTOOLS-52?page=comments#action_12332291 ] 

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

I would be happy to "offer" it to the commons-validator or Struts folks. I'm just not sure exactly how to do that, since I don't have a current problem that needs to be solved. I'm a little wary of recommending a change to those packages mostly because of the their complexity.

Velocity-tools was nice and simple to research and patch for this particular problem.

If you've got a contact on either of those teams, please feel free to pass-on my code to them.


> 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


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

Posted by "Christopher Schultz (JIRA)" <ve...@apache.org>.
    [ http://issues.apache.org/jira/browse/VELTOOLS-52?page=comments#action_12332300 ] 

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

Niall,
Wow, I didn't know that anyone had done anything like this.. your tool sounds pretty sexy, especially if it can perform 'validwhen' validations on the client side. Maybe I'll give it a try.

In the meantime, the existing validator unfortunately fails even though the 'validwhen' validation variable is only used on the server side... because the existing velocity-tools implementation dumps *all* variables out to the generated javascript, instead of magically knowing which ones to generate.

I would imagine that the taglib version in Struts does a similar thing, so thanks for raising the issue over there. Of course, you are welcome to steal whatever parts of my solution are useful to you.


> 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


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

Posted by "Nathan Bubna (JIRA)" <ve...@apache.org>.
    [ http://issues.apache.org/jira/browse/VELTOOLS-52?page=comments#action_12332285 ] 

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

Ok.  I don't really think your solution is that much harder to maintain that it's worth the performance loss of using the others.  Of course, there's also the facts that we're not ready to rely on Jdk regexps, ORO is on its way out, and apparently StringTokenizer is too.  so, of the four, i like your "home brew".   perhaps the commons-validator or Struts taglib folks would as well, if you offered it to them.

i'll give this another day or so, so that other folks have a chance to chime in if they have an opinion.  then i'll commit it.  

veltools 1.2 is getting very close... :)

> 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


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

Posted by "Nathan Bubna (JIRA)" <ve...@apache.org>.
     [ http://issues.apache.org/jira/browse/VELTOOLS-52?page=all ]
     
Nathan Bubna resolved VELTOOLS-52:
----------------------------------

    Resolution: Fixed

Done in http://svn.apache.org/viewcvs?rev=326291&view=rev

> 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


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

Posted by "Christopher Schultz (JIRA)" <ve...@apache.org>.
    [ http://issues.apache.org/jira/browse/VELTOOLS-52?page=comments#action_12332302 ] 

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

Nathan,
The major change between your implementation and mine is that yours unconditionally creates a StringBuffer and copies into it, even when no replacements are to be done, and mine avoids the object creation in the case where the string has nothing to replace. I figured that a lot of the Strings being escaped would /not/ need any replacements to be done, so I opted for a best-case scenerio where the only thing wasted was CPU cycles.

I added yours to my set of benchmark tests and got the following:
0, 272
1, 276
2, 291
3, 315
many 310
long 2601

The only unexpected result is that of the "many replacements" test where yours beat mine silly. I checked the source of StringBuffer and I can't figure it out.

Honestly, I don't really care which implementation you choose, as long as it meets the requirements of which characters to escape and is reasonably efficient. Both of our implementations meet that requirement.


> 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


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

Posted by "Christopher Schultz (JIRA)" <ve...@apache.org>.
    [ 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@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org


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

Posted by "Nathan Bubna (JIRA)" <de...@velocity.apache.org>.
    [ https://issues.apache.org/jira/browse/VELTOOLS-52?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12467204 ] 

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

Glad to hear you don't think we should hold off on 1.3.  

But so you don't feel too alone, know that i always assume that for each time a problem is reported in open source software, there are 5 to 10 more times that it is experienced but not reported.   Not everyone takes time to improve software by reporting problems.  Many just fix it themselves, workaround it, or ditch the library (depending on the severity of the issue of course).

Anyway, let me know if i can help you in figuring out where the bug is or especially in fixing it.  I don't have a ton of time (since i can't use paid-work time on something the company isn't using), but i'd like to help where i can.

> ValidatorTool javascript generator can generate invalid Javascript
> ------------------------------------------------------------------
>
>                 Key: VELTOOLS-52
>                 URL: https://issues.apache.org/jira/browse/VELTOOLS-52
>             Project: Velocity Tools
>          Issue Type: Bug
>          Components: VelocityStruts
>    Affects Versions: 1.2
>         Environment: Using JDK1.4.2 / Linux 2.4 kernel / Tomcat 4.1
>            Reporter: Christopher Schultz
>         Assigned To: 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.
-
You can reply to this email to add a comment to the issue online.


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