Bug 11501 - REGRESSION: \u no longer escapes metacharacters in RegExps
Summary: REGRESSION: \u no longer escapes metacharacters in RegExps
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Major
Assignee: Nobody
URL: javascript:alert(/a\u007cb/.exec('a'))
Keywords: HasReduction, Regression
Depends on:
Blocks:
 
Reported: 2006-11-03 01:55 PST by Alexey Proskuryakov
Modified: 2006-11-20 12:34 PST (History)
2 users (show)

See Also:


Attachments
Patch to fix 11501 and 11502 (12.82 KB, patch)
2006-11-05 23:17 PST, W. Andy Carrel
ggaren: review-
Details | Formatted Diff | Diff
Patch to fix 11501 et al. (13.17 KB, patch)
2006-11-06 18:45 PST, W. Andy Carrel
sam: review-
Details | Formatted Diff | Diff
Patch to fix 11501 et al. (13.11 KB, patch)
2006-11-10 16:12 PST, W. Andy Carrel
sam: review-
Details | Formatted Diff | Diff
Patch to fix 11501 et al. (13.58 KB, patch)
2006-11-13 13:27 PST, W. Andy Carrel
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2006-11-03 01:55:57 PST
The result of the test in bug URL should be null. See bug 7445 comment 24 and further discussion.
Comment 1 W. Andy Carrel 2006-11-05 23:17:31 PST
Created attachment 11392 [details]
Patch to fix 11501 and 11502

This patch backs out the fix in bug 7445 in favor of a solution that better matches the behavior of other browsers. There is also fix for a problem with UString::append(UChar &) that caused it to, under certain circumstances, not actually append the character.
Comment 2 Geoffrey Garen 2006-11-06 17:40:58 PST
Comment on attachment 11392 [details]
Patch to fix 11501 and 11502

We talked about this on IRC. Andy's going to submit a new patch.
Comment 3 W. Andy Carrel 2006-11-06 18:45:49 PST
Created attachment 11409 [details]
Patch to fix 11501 et al.

Now with better readability, among other things, per ggaren's feedback
Comment 4 Sam Weinig 2006-11-08 06:44:34 PST
Comment on attachment 11409 [details]
Patch to fix 11501 et al.

Marking r- as a few style issues remain.  

Please put spaces around infix operators.
 
+    next3 = (pos + 3 < length) ? code[pos+3].uc : -1;

the spaces inside the parenthesis are wrong even though they match the surrounding style.

+        if ( !lastWasEscape ) {

The * should be next to the type.

+    _regex = pcre_compile(reinterpret_cast<const uint16_t *>(cleanedPattern.data()), options, &errorMessage, &errorOffset, NULL);

Also, please regenerate the ChangeLog as the names of the new functions have changed.
Comment 5 W. Andy Carrel 2006-11-10 16:12:56 PST
Created attachment 11477 [details]
Patch to fix 11501 et al.

Now with the additional requested style fixes. Most of this batch came from reverting code. It might not hurt to make a linting program public if one exists for identifying these sorts of issues.
Comment 6 Sam Weinig 2006-11-13 11:12:08 PST
Comment on attachment 11477 [details]
Patch to fix 11501 et al.

r- for a few more issues.

No spaces before *'s

+    _regex = pcre_compile(reinterpret_cast<const uint16_t *>(cleanedPattern.data()), options, &errorMessage, &errorOffset, NULL);


Add a comment for:

+  while (pos != -1 + 2) {

Please make the new functions static. And add the URL for this bug to the changelogs.
Comment 7 W. Andy Carrel 2006-11-13 13:27:03 PST
Created attachment 11508 [details]
Patch to fix 11501 et al.

Now with yet more exciting style and static fixes.
Comment 8 Darin Adler 2006-11-18 17:46:06 PST
Comment on attachment 11508 [details]
Patch to fix 11501 et al.

This will probably work. But I think the best fix for this is to put the \u handling inside PCRE itself. For one thing, it's bound to be higher performance than this approach.
Comment 9 Darin Adler 2006-11-18 17:47:05 PST
Comment on attachment 11508 [details]
Patch to fix 11501 et al.

However, this patch does make things better than before, so it's probably fine to land this -- we can put the support into PCRE later as a further enhancement.
Comment 10 Maciej Stachowiak 2006-11-20 00:39:40 PST
Comment on attachment 11508 [details]
Patch to fix 11501 et al.

r=me
Comment 11 Alexey Proskuryakov 2006-11-20 12:34:41 PST
Committed revision 17862.