306727 - RegExp: different treatment of \011 depending on JSOPTION_STRICT setting

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Michael Daumling]

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]

Attachment: Proposed fix for this bug

Comment 2

[Bob Clary [:bc] (inactive)]

trunk as well.

Comment 3

[Brendan Eich [:brendan]]

mrbkap, want to take this one?

/be

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: 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) (inactive)]

Comment on attachment 194553 [details] [diff] [review]
Proposed fix for this bug

Cancelling review request on an obsolete attachment.

Comment 6

[Brendan Eich [:brendan]]

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

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: 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]]

Comment on attachment 196260 [details] [diff] [review]
fix some tweaks (-w)

Like, duh, and stuff!

/be

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk. I'll check this in on branch tomorrow.

Comment 10

[Brendan Eich [:brendan]]

Looks like this never hit the branch, so I just checked it in there.

/be

Comment 11

[Bob Clary [:bc] (inactive)]

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]

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]

(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]

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.