Bug 189602

Summary:

PSON: window.open() with 'noopener' should only process-swap cross-site, not cross-origin

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

WebKit2

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, commit-queue, dbates, ews-watchlist, ggaren, japhet, koivisto, rniwa, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
WIP Patch	none
Archive of layout-test-results from ews106 for mac-sierra-wk2	none
Archive of layout-test-results from ews126 for ios-simulator-wk2	none
Archive of layout-test-results from ews121 for ios-simulator-wk2	none
Patch	none
Patch	none

Chris Dumez

Reported 2018-09-13 13:57:42 PDT

window.open() with 'noopener' should only process-swap cross-site, not cross-origin when process swap on cross-site navigation is enabled.

Attachments
WIP Patch (24.47 KB, patch) 2018-09-13 16:56 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews106 for mac-sierra-wk2 (3.10 MB, application/zip) 2018-09-13 18:20 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (13.70 MB, application/zip) 2018-09-13 19:02 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (13.15 MB, application/zip) 2018-09-13 21:00 PDT, EWS Watchlist	no flags	Details
Patch (47.85 KB, patch) 2018-09-14 09:56 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (47.83 KB, patch) 2018-09-17 09:51 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2018-09-13 13:58:18 PDT

<rdar://problem/44430549>

Chris Dumez

Comment 2 2018-09-13 16:56:26 PDT

Created attachment 349715 [details] WIP Patch Still needs testing for <a target="_blank"> & <a target="_blank" rel="noopener">.

EWS Watchlist

Comment 3 2018-09-13 16:58:08 PDT

Attachment 349715 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:250: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:258: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:260: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:264: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:266: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:274: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] Total errors found: 6 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.

EWS Watchlist

Comment 4 2018-09-13 18:19:59 PDT

Comment on attachment 349715 [details] WIP Patch Attachment 349715 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/9208396 New failing tests: http/tests/security/xssAuditor/link-opens-new-window.html

EWS Watchlist

Comment 5 2018-09-13 18:20:00 PDT

Created attachment 349719 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6

EWS Watchlist

Comment 6 2018-09-13 19:02:51 PDT

Comment on attachment 349715 [details] WIP Patch Attachment 349715 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/9208455 New failing tests: http/tests/security/xssAuditor/link-opens-new-window.html

EWS Watchlist

Comment 7 2018-09-13 19:02:53 PDT

Created attachment 349723 [details] Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4

EWS Watchlist

Comment 8 2018-09-13 21:00:51 PDT

Comment on attachment 349715 [details] WIP Patch Attachment 349715 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/9209566 New failing tests: http/tests/security/xssAuditor/link-opens-new-window.html

EWS Watchlist

Comment 9 2018-09-13 21:00:53 PDT

Created attachment 349730 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4

Ryosuke Niwa

Comment 10 2018-09-13 23:02:45 PDT

Comment on attachment 349715 [details] WIP Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349715&action=review > Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:530 > +TEST(ProcessSwap, CrossSiteButSameOriginWindowOpenNoOpener) Did you mean to say CrossOriginButSameSiteWindowOpenNoOpener here??

Chris Dumez

Comment 11 2018-09-14 08:40:34 PDT

(In reply to Ryosuke Niwa from comment #10) > Comment on attachment 349715 [details] > WIP Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=349715&action=review > > > Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:530 > > +TEST(ProcessSwap, CrossSiteButSameOriginWindowOpenNoOpener) > > Did you mean to say CrossOriginButSameSiteWindowOpenNoOpener here?? Definitely :) Thanks.

Chris Dumez

Comment 12 2018-09-14 09:56:35 PDT

Created attachment 349769 [details] Patch

EWS Watchlist

Comment 13 2018-09-14 09:59:20 PDT

Attachment 349769 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:250: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:258: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:260: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:264: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:266: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:274: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:283: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:286: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:290: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:292: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:295: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:299: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:301: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:304: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:308: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] Total errors found: 15 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.

Chris Dumez

Comment 14 2018-09-17 09:31:56 PDT

Ping review?

Geoffrey Garen

Comment 15 2018-09-17 09:49:21 PDT

Comment on attachment 349769 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349769&action=review r=me > Source/WebKit/ChangeLog:18 > + - This forces the origin check to happens on WebContent process site instead of relying on the side

Chris Dumez

Comment 16 2018-09-17 09:51:23 PDT

Created attachment 349895 [details] Patch

EWS Watchlist

Comment 17 2018-09-17 09:54:08 PDT

Attachment 349895 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:250: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:258: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:260: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:264: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:266: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:274: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:283: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:286: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:290: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:292: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:295: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:299: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:301: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:304: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:308: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] Total errors found: 15 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.

WebKit Commit Bot

Comment 18 2018-09-17 10:15:14 PDT

Comment on attachment 349895 [details] Patch Clearing flags on attachment: 349895 Committed r236069: <https://trac.webkit.org/changeset/236069>

WebKit Commit Bot

Comment 19 2018-09-17 10:15:16 PDT

All reviewed patches have been landed. Closing bug.

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