368019 - regular expression (regex) parses /[/]/ incorrectly

Status: RESOLVED FIXED

(Rhino Graveyard :: Compiler, defect)

Description

[Chris Watts]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1
Build Identifier: js-1.6R2

If you have an html page that contains javascript code of:

<script>
myregex = /[/]/;
</script>

then you get an error of expected ';'.

As far as I am aware [/] is valid for regular expressions in javascript (it works in IE6 & Firefox).


A work around is:
<script>
myregex = /\//;
</script>

or

<script>
myregex = new RegEx("[/]");
</script>

Reproducible: Always

Steps to Reproduce:
Used htmlunit (http://htmlunit.sf.net) to parse page containing the following script:
<script>
myregex = /[/]/;
</script>
Actual Results:  
ERROR com.gargoylesoftware.htmlunit.ScriptEngine  - error: message=[missing ; before statement] sourceName=[http://localhost:6060/sms/JavaScript/common.js] line=[1] lineSource=[var testRegex = /[/]/;] lineOffset=[20]

Expected Results:  
No error?

Comment 1

[Brian Crowder]

"/" is the opening and closing character of the regular expression literal, to use it within an expression you must escape it with a backslash.  This is not a work-around, but expected behavior.  This is specified by ECMA-262 in section 7.8.5 (the syntax productions)

Comment 2

[Brendan Eich [:brendan]]

We support the IE quirk that allows unescaped backslash in character class, contra ECMA-262 Edition 3. The following works for me in Firefox 2 and the js shell built from the 1.8 branch, 1.8.0 branch, and trunk:

myregex = /[/]/;

Reporter, can you please attach a testcase and give the exact user agent in which it fails for you?

/be

Comment 3

[Brad Clarke]

The source of the bug report (and therefore the user agent) is HtmlUnit:

https://sourceforge.net/tracker/?func=detail&atid=448266&aid=1640568&group_id=47038

Just curious: Is this a difference between spidermonkey and rhino or does firefox pre-process the quirk?

Comment 4

[Ahmed Ashour]

May I post this test case using Rhino 1.6R5:

----------------------------------------------------------
        final Context cx = Context.enter();
        final Scriptable scope = cx.initStandardObjects();
        cx.evaluateString( scope, "myregex = /[/]/;", "source1", 1, null);
        Context.exit();
----------------------------------------------------------

Comment 5

[Sudhan Moghe]

Problem is occurring because of following while loop in readRegExp() method of org.mozilla.javascript.TokenStream.java class.

while ((c = getChar()) != '/') {
    if (c == '\n' || c == EOF_CHAR) {
        ungetChar(c);
        throw parser.reportError("msg.unterminated.re.lit");
    }
    if (c == '\\') {
        addToString(c);
        c = getChar();
    }

    addToString(c);
}


I am not an expert in Regular Expression so can't say whether it is a bug or not. But I am facing problem because of this. I am developing a testing tool using HtmlUnit for one of our clients and he is having following code in his JavaScripts.

if( str.match(/[*+-./]/) ) 

I have requested him to change the scripts. But that script is part of a framework and lot of applications depend on that framework. If the request is rejected then I will have to handle it in above mentioned code.

I am thinking of pushing '[' (and may be '(') on stack and consuming '/' if stack is not empty. Do I need to handle anything more than this? I know that it will work in my case but is it generic enough? If it is then I will post updated code here.

Thanks & Regards,
Sudhan

Comment 6

[Brian Crowder]

Sudhan Moghe:  I don't think your comment is correctly associated with the rest of this bug, you should probably file a new bug.

Thanks

Comment 7

[Sudhan Moghe]

Yes, it is.
What I have posted is the code from Rhino that is parsing the regex.
my case is 

if( str.match(/[*+-./]/) ) 

which is similar to 
/[/]/

as you can see from the rhino code, parsing of regex stops when it encounters second '/'. 
After this point parser expects ')' in my case and ';' in the case reported in the bug. But it encounters ']' and throws error.

Comment 8

[Brian Crowder]

You should escape your "/" with a "\".  This is not a bug.

Comment 9

[Brendan Eich [:brendan]]

(In reply to comment #8)
> You should escape your "/" with a "\".  This is not a bug.

Wait, we changed SpiderMonkey to match IE JScript and *not* require a backslash for this case. Why tell Rhino users to pound sand?

This is obviously a misfiled Rhino bug. ES4 matches the de-facto standard and I'm sure Rhino owners/peers will track that.

/be

Comment 10

[Sudhan Moghe]

(In reply to comment #8)
I am not saying that it is a bug but the problem is that the JavaScript is not under my control and I can not modify it to escape '/'.
I am developing a testing tool using HtmlUnit which internally uses Rhino. And my tool wont work unless this issue is fixed. I can handle it as I have suggested in Comment #5. I just wanted to know whether that is a proper handling or do I need to handle more cases? And what are the chances of that getting included in Rhino code.

Thanks & Regards,
Sudhan

Comment 11

[Marc]

If Rhino maintainers don't want to include this per default to respect Ecma spec, then what do you think of an additional feature option in Context to allow it?

Comment 12

[Brendan Eich [:brendan]]

Recommend that Rhino match SpiderMonkey and other browser engines, and anticipate JS2/ES4, by allowing unescaped / in character class.

/be

Comment 13

[Attila Szegedi]

I think that tracking open brackets would work as you suggested in Comment #5. If you have patched the code already and can post a patch, it'd be appreciated.

Comment 14

[Marc]

Attachment: Patch handling / with []

This patch fixes the problem for me and doesn't break any additional tests from the junit-all target (as already said, a good state of the tests would help to avoid regression and discover issues fixed as side effects)

Comment 15

[Norris Boyd]

Fixed.