141078 - Should JS support octal escape sequences in regexps?

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Phil Schwartau]

This arose from the HTML in bug 137458.

The site there was generated by Microsoft FrontPage v5.0.
Apparently Microsoft still supports the use of octal escape sequences
such as \240 within regular expressions. That is, sequences of form \OOO,
where each 'O' is a octal digit. This was accepted in ECMA-262 Edition 2.


The bug 137458 site uses constructs such as this:

              "#FF1111".replace(/\240/g, " ")

i.e. replace the character given by octal escape sequence \240
with a single space. This is accepted in both NN4.7 and IE6,
as you can see by trying

        javascript: alert("#FF1111".replace(/\240/g, ' '));


In Mozilla, however, you get this error:

Error: back-reference exceeds number of capturing parentheses
Source File: javascript: alert("#FF1111".replace(/\240/g, ' '));
Line: 1



I spoke with Waldemar about this. The ECMA-262 Edition 3 standard
deprecated the use of octal escape sequences in regular expressions;
but we may want to revisit this for backward compatibility.

Filing this bug for a decision on that -

Comment 1

[rogerl (gone)]

I need an opinion on this, we can

- back out the ECMA deprecation 
- mark as WONTFIX
- reassign to Evangelism

Comment 2

[Brendan Eich [:brendan]]

Compatibility is king, perl set precedent.  I see why ECMA deprecated, but it
has a different purview (future specs and impls); meanwhile, in the real world,
the Mozilla implementation wants to gain market share.  Evangelizing may help a
bit, but with frontpage spewing octal regexp char codes, it seems to me it won't
win enough.  I say back out the ECMA deprecation.

This matters for js1.5, so for mozilla1.1 and mozilla1.0.1.

/be

Comment 3

[rogerl (gone)]

Attachment: Allow old style octal references.

I switched to emitting the error only if the '-strict' option is in effect,
does that seem like the right thing? Or should it be no more than a warning
even in that case?

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 91232 [details] [diff] [review]
Allow old style octal references.

In general, you shouldn't test JS_HAS_STRICT_OPTION -- instead, call 
js_ReportCompileErrorNumber(..., JSREPORT_STRICT | JSREPORT_WARNING...) and if
it returns false, someone set option.werror, so you should fail immediately
(propagate the error via false or null return code).  Otherwise, do the bad old
thing.

Here, I see why you want to preserve the if-else control flow, but I hope the
strict user gets a nice strict warning (not an error, as I thought users get
now given \377).  Where is the error report for overlarge backref?  Can it
change into a strict warning and be issued iff the else clause of the if
condition shown in this patch is taken?

/be

Comment 5

[rogerl (gone)]

Attachment: Revert to earlier octal handling, but with 'strict' imposing ECMA3 rules.

Here's another attempt. I think I need to go with the strict flag testing
because this is simply a parsing choice - either \007 is an octal value or a
NUL followed by '07', \12 is an octal value or a decimal back-reference.
Neither is an error - except that the back-reference may turn out to be so,
later, in which case it's a real error, not a warning.

Comment 6

[Phil Schwartau]

Testcase added to JS testsuite:

        mozilla/js/tests/ecma_3/RegExp/octal-001.js


I have filed bug 158159 against Rhino for this same issue -

Comment 7

[rogerl (gone)]

Attachment: Fixed tabs.

Fixed whitespace issues, previous patch was -wu, I'm not sure which is more
readable.

Comment 8

[Phil Schwartau]

Another testcase for this added to JS testsuite:

        mozilla/js/tests/ecma_3/RegExp/octal-002.js

Comment 9

[Brendan Eich [:brendan]]

Rogerl, how about an up-to-date diff -u and a diff -wu (the latter for reviewing
purposes)?  Also, let's target this at 1.2alpha (timeless prodding works) and
I'll help get it reviewed. Thanks.

/be

Comment 10

[rogerl (gone)]

Attachment: Updated patch

Comment 11

[rogerl (gone)]

Attachment: Updated patch with -wu

Comment 12

[Brendan Eich [:brendan]]

I'll review and checkin before the deadline tonight.

/be

Comment 13

[Brendan Eich [:brendan]]

I checked the fix into the trunk (I tweaked a multiline && expression in an if
condition to put the && consistently at the end of each line).

/be

Comment 14

[Phil Schwartau]

Marking Verified Fixed.

Both testcases above now pass, in the debug/optimized JS shell:

        mozilla/js/tests/ecma_3/RegExp/octal-001.js
        mozilla/js/tests/ecma_3/RegExp/octal-002.js