Last Comment Bug 306727 - RegExp: different treatment of \011 depending on JSOPTION_STRICT setting
: RegExp: different treatment of \011 depending on JSOPTION_STRICT setting
Status: RESOLVED FIXED
: fixed1.8
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap) (in and out until 7-14)
:
Mentors:
Depends on:
Blocks: 383574
  Show dependency treegraph
 
Reported: 2005-09-01 10:45 PDT by Michael Daumling
Modified: 2007-06-12 15:14 PDT (History)
4 users (show)
brendan: blocking1.8b5+
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed fix for this bug (668 bytes, patch)
2005-09-01 10:45 PDT, Michael Daumling
no flags Details | Diff | Review
Allow octal escapes in strict mode (2.43 KB, patch)
2005-09-01 16:48 PDT, Blake Kaplan (:mrbkap) (in and out until 7-14)
no flags Details | Diff | Review
fix some tweaks (-w) (2.04 KB, patch)
2005-09-15 16:55 PDT, Blake Kaplan (:mrbkap) (in and out until 7-14)
brendan: review+
brendan: approval1.8b5+
Details | Diff | Review

Description Michael Daumling 2005-09-01 10:45:02 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6

The RegExp /\011/ is parsed wrong if the STRICT option is active. It is parsed
as \0x00 + "11" instead of the correct interpretation of \011 as an octal value.
This is in the Mozilla 1.8 branch.

Reproducible: Always

Steps to Reproduce:
1. Start jsshell.exe
2. execute /.\011\.exec ('a'+String.fromCharCode(0)+'11')
3. see correct result: null
4. execute options ('strict') and rum (2) again


Actual Results:  
incorrect result 'a 11'

Expected Results:  
null as with option STRICT disabled
Comment 1 Michael Daumling 2005-09-01 10:45:56 PDT
Created attachment 194553 [details] [diff] [review]
Proposed fix for this bug
Comment 2 Bob Clary [:bc:] 2005-09-01 11:12:15 PDT
trunk as well.
Comment 3 Brendan Eich [:brendan] 2005-09-01 12:25:27 PDT
mrbkap, want to take this one?

/be
Comment 4 Blake Kaplan (:mrbkap) (in and out until 7-14) 2005-09-01 16:48:35 PDT
Created attachment 194604 [details] [diff] [review]
Allow octal escapes in strict mode

I decided to leave the overflow quirk below this patch as it was (if the strict
option is not on, then we try to treat the escape as octal, otherwise we use
0x10000).

Michael, I was unable to apply your diff to my tree, so I created my own. In
general CVS diffs are preferred.
Comment 5 Blake Kaplan (:mrbkap) (in and out until 7-14) 2005-09-01 16:49:08 PDT
Comment on attachment 194553 [details] [diff] [review]
Proposed fix for this bug

Cancelling review request on an obsolete attachment.
Comment 6 Brendan Eich [:brendan] 2005-09-15 16:44:57 PDT
*** Bug 308738 has been marked as a duplicate of this bug. ***
Comment 7 Blake Kaplan (:mrbkap) (in and out until 7-14) 2005-09-15 16:55:28 PDT
Created attachment 196260 [details] [diff] [review]
fix some tweaks (-w)

This is a diff -w, I can produced a regular diff if you don't trust my
indentation skills!
Comment 8 Brendan Eich [:brendan] 2005-09-15 17:05:12 PDT
Comment on attachment 196260 [details] [diff] [review]
fix some tweaks (-w)

Like, duh, and stuff!

/be
Comment 9 Blake Kaplan (:mrbkap) (in and out until 7-14) 2005-09-15 17:21:26 PDT
Fix checked into trunk. I'll check this in on branch tomorrow.
Comment 10 Brendan Eich [:brendan] 2005-09-21 19:47:10 PDT
Looks like this never hit the branch, so I just checked it in there.

/be
Comment 11 Bob Clary [:bc:] 2005-10-23 05:19:15 PDT
Checking in regress-306727.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-306727.js,v  <--  regress-306727.js
initial revision: 1.1
done
Comment 12 Brian Crowder 2007-06-08 14:49:41 PDT
I think based on my interpretation of the spec, we went the wrong way on this bug.  We should instead _always_ be treating \0 as 0x00 (regardless of STRICT), and subsequent digits as _not_ part of an octal escape sequence (or perhaps as an error, see below):

  from 15.10.2.11:
  Informative comments: If \ is followed by a decimal number n whose first digit is not 0, then the escape sequence is considered to be a backreference. It is an error if n is greater than the total number of left capturing parentheses in the entire regular expressi on. \0 represents the NUL character and cannot be followed by a decimal digit.

I don't necessarily agree with the way this is specified.  It almost seems to suggest that we should actually be issuing an error for a digit followed by \0 ("cannot be followed by a decimal digit").  I would -rather- have \0 introduce octal literals (or be a backreference?), but I think it must be too late now.

Added a dependency for the more recent bug this patch affects.
Comment 13 Brian Crowder 2007-06-08 16:11:26 PDT
(In reply to comment #12)

> It almost seems to suggest that we should actually be issuing an error for
> a digit followed by \0 ("cannot be followed by a decimal digit").

I meant "a \0 followed by a digit" here, of course.
Comment 14 Brian Crowder 2007-06-12 15:14:24 PDT
After talking with mrbkap on IRC about this some, we've decided to flagrantly disregard the spec and instead go with the behavior we've had for years.  Bug 383574 will instead be fixed by removal of the warning.

Note You need to log in before you can comment on or make changes to this bug.