RegExp: different treatment of \011 depending on JSOPTION_STRICT setting

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: Michael Daumling, Assigned: mrbkap)

Tracking

({fixed1.8})

Trunk
x86
Windows XP
fixed1.8
Points:
---
Bug Flags:
blocking1.8b5 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

12 years ago
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
(Reporter)

Comment 1

12 years ago
Created attachment 194553 [details] [diff] [review]
Proposed fix for this bug
Attachment #194553 - Flags: review?(brendan)

Comment 2

12 years ago
trunk as well.
Status: UNCONFIRMED → NEW
Ever confirmed: true
mrbkap, want to take this one?

/be
(Assignee)

Comment 4

12 years ago
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.
Assignee: general → mrbkap
Attachment #194553 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #194604 - Flags: review?(brendan)
(Assignee)

Comment 5

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

Cancelling review request on an obsolete attachment.
Attachment #194553 - Flags: review?(brendan)
*** Bug 308738 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 7

12 years ago
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!
Attachment #194604 - Attachment is obsolete: true
Attachment #196260 - Flags: review?(brendan)
Comment on attachment 196260 [details] [diff] [review]
fix some tweaks (-w)

Like, duh, and stuff!

/be
Attachment #196260 - Flags: review?(brendan)
Attachment #196260 - Flags: review+
Attachment #196260 - Flags: approval1.8b5+
(Assignee)

Comment 9

12 years ago
Fix checked into trunk. I'll check this in on branch tomorrow.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Updated

12 years ago
Attachment #194604 - Flags: review?(brendan)
Looks like this never hit the branch, so I just checked it in there.

/be
Flags: blocking1.8b5+
Keywords: fixed1.8

Comment 11

12 years ago
Checking in regress-306727.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-306727.js,v  <--  regress-306727.js
initial revision: 1.1
done
Flags: testcase+

Comment 12

10 years ago
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.
Blocks: 383574
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 13

10 years ago
(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

10 years ago
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.
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.