309840 - Regular expression /[/]/ gives "unterminated character class" error

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b5) Gecko/20050923
Firefox/1.4

Firefox gives an "unterminated character class" error on the regular expression
/[/]/.  Safari and Opera 8.5 accept it.

I noticed this because a regular expression containing [/] appears in 
http://msdn.microsoft.com/workshop/code/browdata.js, which was mentioned in 
bug 309695 comment 9.

Comment 1

[Bob Clary [:bc] (inactive)]

->all/all.

This is technically invalid. <http://bclary.com/2004/11/07/ecma-262#a-7.8.5>. I
have some raw data on top sites I can mine for how common this is in the wild.

Comment 2

[Brendan Eich [:brendan]]

This is entirely contrary to ECMA-262 Edition 3, and to its regexp progenitor,
Perl.  However, mrbkap expressed sympathy, however fleeting under my withering
counterattack ;-). He wins this no-prize.

I'm quite interested in bclary's top site spidering results.

/be

Comment 3

[Bob Clary [:bc] (inactive)]

I've been running scans of the top 69 sites 1 level deep (usually ~100 pages per
site) and haven't seen an instance of this error. This doesn't imply that the
error is rare. It has been a reoccuring issue over the years, especially on MS
sites. I could run a 2 level scan if you think it would be worth it, but it
would take a few days.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

I think if we want this, we want it for Firefox 1.5.

Comment 5

[Jesse Ruderman]

I searched for bugs with comments containing "unterminated character class" and
found these:

Open tech evang bugs: bug 235508, bug 224988 (Microsoft), bug 212822

Bugs with discussion: bug 89586, bug 173067, bug 212822

Comment 6

[Mike Connor [:mconnor]]

Need a fix this week if this is going to make it.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Allow / in char classes

Comment 8

[Mike Shaver (:shaver emeritus)]

Comment on attachment 197590 [details] [diff] [review]
Allow / in char classes

sr=shaver

Comment 9

[Asa Dotzler [:asa]]

we're not going to hold the release for this but if it comes to us with full
reviews and it's low risk, we can talk.

Comment 10

[Brendan Eich [:brendan]]

Comment on attachment 197590 [details] [diff] [review]
Allow / in char classes

Yeah, this is safe.  The only way to close a character class is with an
unescaped ], and you can't nest character classes, so a boolean suffices.

/be

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk.

Comment 12

[Brendan Eich [:brendan]]

Comment on attachment 197590 [details] [diff] [review]
Allow / in char classes

This fixes a highly dup'ed IE compatibility bug.  It's an extension to ECMA
syntax	that's allowed by Section 16.  I'm approving it so that we can get it
into 1.8b5 / Firefox 1.5b2.

/be

Comment 13

[Brendan Eich [:brendan]]

Don't forget to set fixed1.8 when this lands on the branch tonight.

/be

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

Checked into MOZILLA_1_8_BRANCH.

Comment 15

[Jesse Ruderman]

*** Bug 235508 has been marked as a duplicate of this bug. ***

Comment 16

[Jesse Ruderman]

*** Bug 212822 has been marked as a duplicate of this bug. ***

Comment 17

[Olivier Cahagne]

*** Bug 224988 has been marked as a duplicate of this bug. ***

Comment 18

[Bob Clary [:bc] (inactive)]

Checking in regress-309840.js;
/cvsroot/mozilla/js/tests/ecma_3/RegExp/regress-309840.js,v  <--  regress-309840.js
initial revision: 1.1
done

obsoletes js1_5/Regress/regress-173067.js

Comment 19

[:Gavin Sharp [email: [e-mail]]]

*** Bug 318303 has been marked as a duplicate of this bug. ***

Comment 20

[Bob Clary [:bc] (inactive)]

verified fixed 200604-1 1.8.0.2, 1.8, 1.9a1 win/mac/linux

Comment 21

[Bob Clary [:bc] (inactive)]

note to self: ecma_3/RegExp/regress-309840.js timed out on peach 1.8 debug browser, but not on prune. Could not reproduce manually.