Bug 202882

Summary: Regular expression hangs in Safari only
Product: WebKit Reporter: Radu Samson <radusamson>
Component: JavaScriptCoreAssignee: Christopher Reid <chris.reid>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, annulen, benjamin, cdumez, chris.reid, cmarcelo, commit-queue, dbates, ews-watchlist, fpizlo, gyuyoung.kim, msaboff, ross.kirsling, ryuan.choi, saam, sergio, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: Safari 13   
Hardware: Mac   
OS: macOS 10.15   
Attachments:
Description Flags
Compiled regular expression to match a legal act in multiple languages
none
Patch
none
Patch
none
Patch for landing none

Description Radu Samson 2019-10-12 13:14:14 PDT
Created attachment 380832 [details]
Compiled regular expression to match a legal act in multiple languages

The regular expression attached works in all browsers except Safari, probably due to character encoding representation on the underlying platform. Following input should match but crashes:

Directive 2001/83/EC
Comment 1 Radar WebKit Bug Importer 2019-10-13 20:05:42 PDT
<rdar://problem/56236654>
Comment 2 Christopher Reid 2019-10-15 06:46:33 PDT
Created attachment 380980 [details]
Patch
Comment 3 Alex Christensen 2019-10-16 10:39:13 PDT
You should add a test case that fails without this change and passes with this change.
Comment 4 Saam Barati 2019-10-16 18:01:15 PDT
Comment on attachment 380980 [details]
Patch

nice. Agree with Alex. Please add a test. It's super easy to do inside JSTests/stress. The test. can be as simple as a JavaScript program which doesn't throw.
Comment 5 Christopher Reid 2019-10-31 23:54:56 PDT
Created attachment 382563 [details]
Patch

Added BumpPointerAllocator API test and a stress test.
I wasn't able to get a smaller regex that hits this infinite loop so I used the full expression for a stress test.
Comment 6 Yusuke Suzuki 2019-11-01 00:15:11 PDT
Comment on attachment 382563 [details]
Patch

r=me
Comment 7 WebKit Commit Bot 2019-11-01 00:58:16 PDT
Comment on attachment 382563 [details]
Patch

Rejecting attachment 382563 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 382563, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as commit-queue@webkit.org...
Fetching: https://bugs.webkit.org/attachment.cgi?id=382563&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=202882&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Updating working directory
Processing patch 382563 from bug 202882.
Fetching: https://bugs.webkit.org/attachment.cgi?id=382563
Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	A	JSTests/stress/regress-202882.js
	A	Tools/TestWebKitAPI/Tests/WTF/BumpPointerAllocator.cpp
	M	JSTests/ChangeLog
	M	Source/WTF/ChangeLog
	M	Source/WTF/wtf/BumpPointerAllocator.h
	M	Tools/ChangeLog
	M	Tools/TestWebKitAPI/CMakeLists.txt
	M	Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

ERROR from SVN:
A repository hook failed: Commit blocked by pre-commit hook (exit code 1) with output:

    The following files contain tab characters:

        trunk/JSTests/stress/regress-202882.js

    Please use spaces instead to indent.
    If you must commit a file with tabs, use svn propset to set the "allow-tabs" property.
W: 4c033311588a0338e1d55e21edf8aad018f43b3a and refs/remotes/origin/master differ, using rebase:
:040000 040000 8725fbb9d31ae8497d4e803658928d288ed78320 598d5e740e3a09b1c1748b58121babbb1bed9b13 M	JSTests
:040000 040000 265775ad8d6861b254c338f19a222bcd2df408eb 52c195d9a341dc8f90723c81660ebf756fddf032 M	Source
:040000 040000 e67be50be89f0ceaf97394052f5f1eb0de3c5acc e37ce7bf8a5071dae2c4c720fe8e08b07cf89d7c M	Tools
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	A	JSTests/stress/regress-202882.js
	A	Tools/TestWebKitAPI/Tests/WTF/BumpPointerAllocator.cpp
	M	JSTests/ChangeLog
	M	Source/WTF/ChangeLog
	M	Source/WTF/wtf/BumpPointerAllocator.h
	M	Tools/ChangeLog
	M	Tools/TestWebKitAPI/CMakeLists.txt
	M	Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

ERROR from SVN:
A repository hook failed: Commit blocked by pre-commit hook (exit code 1) with output:

    The following files contain tab characters:

        trunk/JSTests/stress/regress-202882.js

    Please use spaces instead to indent.
    If you must commit a file with tabs, use svn propset to set the "allow-tabs" property.
W: 4c033311588a0338e1d55e21edf8aad018f43b3a and refs/remotes/origin/master differ, using rebase:
:040000 040000 8725fbb9d31ae8497d4e803658928d288ed78320 598d5e740e3a09b1c1748b58121babbb1bed9b13 M	JSTests
:040000 040000 265775ad8d6861b254c338f19a222bcd2df408eb 52c195d9a341dc8f90723c81660ebf756fddf032 M	Source
:040000 040000 e67be50be89f0ceaf97394052f5f1eb0de3c5acc e37ce7bf8a5071dae2c4c720fe8e08b07cf89d7c M	Tools
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
Current branch master is up to date.

Full output: https://webkit-queues.webkit.org/results/13200148
Comment 8 Yusuke Suzuki 2019-11-29 05:43:29 PST
Can we land this? Or is it landed?
Comment 9 Christopher Reid 2019-12-02 11:11:33 PST
(In reply to Yusuke Suzuki from comment #8)
> Can we land this? Or is it landed?

The tab characters can be removed in the test for landing and still reproduce the issue. I was just worried how the commit queue will handle the utf-8 encoding.

Will there be any problems using the commit-queue with those utf-8 characters in the js test or should it be landed manually?
Comment 10 Ross Kirsling 2019-12-02 13:30:05 PST
(In reply to Christopher Reid from comment #9)
> Will there be any problems using the commit-queue with those utf-8
> characters in the js test or should it be landed manually?

You shouldn't need to worry about that; the restriction only applies to tab characters.

For instance, here's a test file with raw UTF-8 but an empty `svn proplist`:
https://github.com/WebKit/webkit/blob/master/JSTests/test262/test/intl402/Collator/prototype/this-value-collator-prototype.js
Comment 11 Christopher Reid 2019-12-03 13:22:14 PST
(In reply to Ross Kirsling from comment #10)
> (In reply to Christopher Reid from comment #9)
> > Will there be any problems using the commit-queue with those utf-8
> > characters in the js test or should it be landed manually?
> 
> You shouldn't need to worry about that; the restriction only applies to tab
> characters.
> 
> For instance, here's a test file with raw UTF-8 but an empty `svn proplist`:
> https://github.com/WebKit/webkit/blob/master/JSTests/test262/test/intl402/
> Collator/prototype/this-value-collator-prototype.js

Sounds good, I'll try re-landing it using the commit-queue without the tab characters.
Comment 12 Christopher Reid 2019-12-03 13:24:04 PST
Created attachment 384747 [details]
Patch for landing
Comment 13 WebKit Commit Bot 2019-12-03 14:07:22 PST
Comment on attachment 384747 [details]
Patch for landing

Clearing flags on attachment: 384747

Committed r253061: <https://trac.webkit.org/changeset/253061>
Comment 14 WebKit Commit Bot 2019-12-03 14:07:24 PST
All reviewed patches have been landed.  Closing bug.