Closed Bug 413155 Opened 14 years ago Closed 10 years ago

Overlarge backreferences, e.g. /(a)\9/, are not SyntaxErrors

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: Waldo, Assigned: Waldo)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file)

 
Attached patch PatchSplinter Review
This is an intentional break from past behavior, as apparently IE interprets overlarge backrefs as octal numbers for all intents and purposes in practical use.  I don't believe Hixie knows about this yet -- I've CCed him and sent email so he knows about this; we'll see what to do once he responds (in a few days, he's afk for now).

What's FrontPage doing these days with respect to this?  Bug 141078 says it used to generate invalid backrefs and expect them to be interpreted as octal, but if recent versions have changed (and in my opinion possibly even if they haven't) I'd say remove the hack and do it per spec.  I can't believe anyone would intentionally write such a regular expression to get octal parsing; off-by-one intuitively feels *way* more common.
If ES4 is going to change this, let me know, and I'll remove that part of Acid3.
Why doesn't Acid3 test the de-facto standard? If ES4 changes, it will be because it is doing exactly that.

/be
I assume it's because nobody's really widely documented it as a deviation, and it sticks out fairly obviously in the spec as something that should but does not happen.

Some deviations I like or don't mind; this one makes me really sad even if it's the "right" thing to do.  :-\
be: I can't test things in Acid3 that aren't required by the specs, for various reasons. Anyway, this subtest is removed now.
No longer blocks: acid3
Target Milestone: mozilla1.9beta3 → ---
Blocks: test262
Well, this shit just got meta:

In YarrParser::parseEscape():

            // To match Firefox, we parse an invalid backreference in the range [1-7] as an octal escape.
            // First, try to parse this as backreference.
V8 follows suit. I say we close as WNF and shoot an email to ES-discuss to note the de-facto behavior.
Notwithstanding browser behavior, referring by backreference to a capturing group that doesn't exist is clear error.  It would be best if it were a syntax error, for the sake of mistyping/miscounting authors, who will not find consistent yet overly-permissive behavior helpful.

We should land a fix for this just after the train departure later this month.  With that in place, we can get solid data (which seems lacking thus far) on whether anyone depends on this.  If we get bug reports, we can revert in short order.  If we don't, perhaps we'll be able to remove a silent failure mode from implementations.  Either way we'll know that much more to confidently resolve this bug.
(In reply to Jeff Walden (remove +bmo to email) (gone August 18-September 4) from comment #8)
> We should land a fix for this just after the train departure later this
> month.  With that in place, we can get solid data (which seems lacking thus
> far) on whether anyone depends on this.  If we get bug reports, we can
> revert in short order.  If we don't, perhaps we'll be able to remove a
> silent failure mode from implementations.  Either way we'll know that much
> more to confidently resolve this bug.

This seems to make sense, but it probably makes even more sense to ask on ES5 is anyone has this data already.
I emailed es-discuss: https://mail.mozilla.org/pipermail/es-discuss/2011-August/016229.html


Dave Fugate tried this at MS and determined that they broke "real world sites".

Gavin Barraclough from Apple tried this, and it broke the web (gmail and old jquery, he thinks).

Test262 actually disabled these tests some weeks ago (a flaw in my tests missed this), so I think we should mark this as WONTFIX.
Yeah, sounds like a WONTFIX for now. Thanks for checking on it.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.