Issue Details (XML | Word | Printable)

Key: STR-2997
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Unassigned
Reporter: Eric Haszlakiewicz
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
Struts 1

[validator] validwhen strings don't work right

Created: 14/Jan/07 02:55 AM   Updated: 07/Feb/07 10:10 PM
Return to search
Component/s: Core
Affects Version/s: 1.2.4, 1.2.9
Fix Version/s: 1.3.7

File Attachments:
  Size
Text File Licensed for inclusion in ASF works STR-2997-fix.patch 2007-02-02 12:59 AM Henri Yandell 0.5 kB
Text File Licensed for inclusion in ASF works STR-2997-test.patch 2007-02-02 12:54 AM Henri Yandell 0.7 kB


 Description  « Hide
I originally noticed this problem with struts 1.2.4, but upgrading to 1.2.9 didn't fix it. I can't upgrade to 1.3.5 right now so I don't know if it still exists there or not.

It seems that the content of string that is usable by the validwhen validator is oddly limited. It only appears to allow letters and a few other random characters. This makes the validator mostly useless.

I tried to define a field validation using the validwhen validator:

<field property="myfield" depends="validwhen">
     <arg0 key="prop.myfield"/>
      <var>
            <var-name>test</var-name>
            <var-value>(otherfield != "foo:bar")</var-value>
      </var>
</field>

I got an error that looked like this:

ValidWhen Error for field ' myfield' - line 1:20: expecting ''', found ':'

I tried it with a few other characters, but there didn't seem to be any pattern to what is allowed and what isn't.

not ok:
    :@#$%^&;,/+

ok:
    *().<>-_=

(not a comprehensive list, I didn't try everything)
I tried using both single and double quotes, but it didn't seem to make a difference.
This problem seems to have been mentioned on the mailing lists a few times, but I wasn't able to find any answers.

Looking at the definition of STRING_LITERAL in ValidWhenParser.g, it seems like any non-quote character should be allowed:
 
STRING_LITERAL : ('\'' (~'\'')+ '\'') | ('\"' (~'\"')+ '\"') ;


 All   Comments   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Henri Yandell added a comment - 01/Feb/07 11:25 PM
Looking at the valid chars, the pattern seems to be characters that are legal outside of a string literal. I'd bet that '[' and ']' are okay too. Also '!' is probably okay. '-' would be okay because negative numbers are supported.

So I'm guessing the problem is that the STRING_LITERAL is not being applied to the part inside the quotes; instead it is treating it as source. Looking at ValidWhenParser.g, I can't see any obvious reasons why that'd be so.

Henri Yandell added a comment - 01/Feb/07 11:32 PM
If I add it to the ValidWhen unit test, I get an error:

Feb 1, 2007 3:31:18 PM org.apache.struts.validator.TestValidWhen doParse
SEVERE: Parsing (*this* == "foo:bar") for property 'stringValue1'
line 1:16: expecting '"', found ':'
        at org.apache.struts.validator.validwhen.ValidWhenLexer.nextToken(ValidWhenLexer.java:245)

Henri Yandell added a comment - 02/Feb/07 12:36 AM
Digging into this more, it appears to be because the charVocabulary is not being set. Every char referenced in the antlr grammer is implicitly added to the charVocabulary, and when you say "~f", it means every char other than f __in the charVocabulary__. So the reason you don't see the other characters is that they've not been added to the charVocabulary yet.

I'll post in a bit with a recommended fix, once I figure out the correct way to generate the .g file into the .java file. I don't think the Struts trunk build is doing it automatically yet.

Henri Yandell added a comment - 02/Feb/07 12:54 AM
Unit test for this issue.

Henri Yandell added a comment - 02/Feb/07 12:59 AM
Fix for this issue. I've chosen to make all chars from the space char up to the ~ legal. The rest of the grammar then adds the necessary ctrl characters (9,10,13). It's a bit wasteful as the uppercase chars are never checked as things are caseSensitive = false, but I don't think that's a big deal.

It does mean that Extended ISO and Unicode are not permitted. The Antlr site points out that a ~ on a Unicode charVocabulary takes up 8k, so that seems like something to think carefully about getting into.

Henri Yandell added a comment - 07/Feb/07 10:10 PM
 svn ci -m "Applying unit test and fix from STR-2997. Antlr 2.7.2 used to regenerate the parser/lexer. "

Sending core/src/main/java/org/apache/struts/validator/validwhen/ValidWhenLexer.java
Sending core/src/main/java/org/apache/struts/validator/validwhen/ValidWhenParser.g
Sending core/src/main/java/org/apache/struts/validator/validwhen/ValidWhenParser.java
Sending core/src/main/java/org/apache/struts/validator/validwhen/ValidWhenParserTokenTypes.java
Sending core/src/main/java/org/apache/struts/validator/validwhen/ValidWhenParserTokenTypes.txt
Sending core/src/test/java/org/apache/struts/validator/TestValidWhen.java
Transmitting file data ......
Committed revision 504715.