Last Comment Bug 429241 - \x... and \u... in RegExp parsed wrong if not followed by enough hex digits
: \x... and \u... in RegExp parsed wrong if not followed by enough hex digits
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
-- minor (vote)
: mozilla12
Assigned To: Chris Leary [:cdleary] (not checking bugmail)
: Jason Orendorff [:jorendorff]
Depends on:
  Show dependency treegraph
Reported: 2008-04-15 18:59 PDT by x0
Modified: 2011-12-21 04:29 PST (History)
3 users (show)
highmind63: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (3.67 KB, patch)
2008-04-16 14:56 PDT, x0
crowderbt: review+
Details | Diff | Splinter Review
testcase for testsuite (4.38 KB, application/x-javascript)
2008-04-16 14:59 PDT, x0
no flags Details

Description User image x0 2008-04-15 18:59:41 PDT
\x and \u must always be followed by exactly 2 resp. 4 hexadecimal digits to form a Hex- or UnicodeEscapeSequence. If they are not, then they should be an IdentityEscape that resolves to plain x resp. u.

ParseTerm, CalculateBitmapSize and ProcessCharSet do that wrong and treat them as \0 instead (or the partly calculated character number if followed by too few hexadecimal digits). In CalculateBitmapSize and ProcessCharSet the (src < end) test wouldn't be necessary, because it is known there that a ']' follows at the end.

CalculateBitmapSize and ProcessCharSet are also wrong if an uncomplete \x... or \u... isn't at the end, because they treat the backslash literally and ignore the x or u. Instead it should be treated as IdentityEscape and thus the backslash be ignored. ParseTerm does that correctly.
Comment 1 User image x0 2008-04-16 14:56:19 PDT
Created attachment 316105 [details] [diff] [review]
Comment 2 User image x0 2008-04-16 14:59:43 PDT
Created attachment 316108 [details]
testcase for testsuite
Comment 3 User image Brian Crowder 2008-04-23 09:19:29 PDT
Comment on attachment 316105 [details] [diff] [review]

This should probably wait to land in a dot-release, though.
Comment 4 User image Nochum Sossonko [:Natch] 2009-07-08 11:32:11 PDT
x0: you should put the keyword 'checkin-needed' once the patch is indeed ready to be checked in.
Comment 5 User image Nochum Sossonko [:Natch] 2010-10-06 08:50:51 PDT
These bugs are all part of a search I made for js bugs that are getting lost in transit:

They all have a review+'ed, non-obsoleted patch and are not marked fixed-in-tracemonkey or checkin-needed but have not seen any activity in 300 days. Some of these got lost simply because the assignee/patch provider never requested a checkin, or just because they were forgotten about.
Comment 6 User image Nochum Sossonko [:Natch] 2011-02-18 09:30:41 PST
Is this still an issue with YARR?
Comment 7 User image Chris Leary [:cdleary] (not checking bugmail) 2011-02-18 15:04:47 PST
(In reply to comment #6)
> Is this still an issue with YARR?

Doesn't appear to be, as the testcase passes. I'm going to steal assignment so I remember to review and check in the test case post-FF4.
Comment 8 User image Tom Schuster [:evilpie] 2011-08-04 07:22:50 PDT
Should i do it?
Comment 9 User image Chris Leary [:cdleary] (not checking bugmail) 2011-12-20 16:57:39 PST
Comment 10 User image Ed Morley [:emorley] 2011-12-21 04:29:43 PST

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