Bug 184996 - Add initial support for 'Cross-Origin-Options' HTTP response header
Summary: Add initial support for 'Cross-Origin-Options' HTTP response header
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on: 184981
Blocks: 185520 185681
  Show dependency treegraph
 
Reported: 2018-04-25 13:30 PDT by Chris Dumez
Modified: 2018-05-17 09:05 PDT (History)
17 users (show)

See Also:


Attachments
WIP patch (27.36 KB, patch)
2018-04-25 13:48 PDT, Chris Dumez
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-sierra (2.59 MB, application/zip)
2018-04-25 14:57 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews106 for mac-sierra-wk2 (2.83 MB, application/zip)
2018-04-25 15:43 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (20.44 MB, application/zip)
2018-04-25 15:50 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews114 for mac-sierra (2.98 MB, application/zip)
2018-04-25 16:29 PDT, EWS Watchlist
no flags Details
WIP patch (27.17 KB, patch)
2018-04-25 16:51 PDT, Chris Dumez
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-sierra (2.76 MB, application/zip)
2018-04-25 18:06 PDT, EWS Watchlist
no flags Details
WIP patch (57.83 KB, patch)
2018-05-01 12:49 PDT, Chris Dumez
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-sierra (2.23 MB, application/zip)
2018-05-01 14:04 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews200 for win-future (12.91 MB, application/zip)
2018-05-01 14:40 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews117 for mac-sierra (3.00 MB, application/zip)
2018-05-01 15:02 PDT, EWS Watchlist
no flags Details
WIP patch (58.53 KB, patch)
2018-05-01 16:36 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP patch (56.94 KB, patch)
2018-05-08 16:43 PDT, Chris Dumez
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews117 for mac-sierra (3.16 MB, application/zip)
2018-05-08 19:45 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews206 for win-future (12.76 MB, application/zip)
2018-05-08 20:01 PDT, EWS Watchlist
no flags Details
WIP patch (60.81 KB, patch)
2018-05-09 09:54 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP patch (62.87 KB, patch)
2018-05-09 10:15 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP patch (63.11 KB, patch)
2018-05-09 10:28 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-sierra (2.80 MB, application/zip)
2018-05-09 11:41 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews106 for mac-sierra-wk2 (2.88 MB, application/zip)
2018-05-09 12:13 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (2.29 MB, application/zip)
2018-05-09 12:18 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews117 for mac-sierra (3.12 MB, application/zip)
2018-05-09 12:46 PDT, EWS Watchlist
no flags Details
Patch (74.37 KB, patch)
2018-05-09 12:53 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-sierra (2.89 MB, application/zip)
2018-05-09 14:10 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews104 for mac-sierra-wk2 (3.18 MB, application/zip)
2018-05-09 14:41 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (2.28 MB, application/zip)
2018-05-09 14:50 PDT, EWS Watchlist
no flags Details
Patch (78.68 KB, patch)
2018-05-09 15:10 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (78.70 KB, patch)
2018-05-09 15:21 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (83.92 KB, patch)
2018-05-09 15:31 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (84.33 KB, patch)
2018-05-09 16:09 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (84.37 KB, patch)
2018-05-09 16:12 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (84.37 KB, patch)
2018-05-09 16:33 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2018-04-25 13:30:00 PDT
Add initial support for 'X-Disown-Opener' HTTP response header which severs the link between the window and its opener upon loading.
Comment 1 Chris Dumez 2018-04-25 13:30:43 PDT
<rdar://problem/39664620>
Comment 2 Chris Dumez 2018-04-25 13:48:30 PDT
Created attachment 338788 [details]
WIP patch
Comment 3 EWS Watchlist 2018-04-25 14:57:47 PDT
Comment on attachment 338788 [details]
WIP patch

Attachment 338788 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/7458542

New failing tests:
http/tests/navigation/disown-opener-header-same-origin.html
http/tests/navigation/disown-opener-header-cross-origin.html
Comment 4 EWS Watchlist 2018-04-25 14:57:49 PDT
Created attachment 338803 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 5 EWS Watchlist 2018-04-25 15:43:35 PDT
Comment on attachment 338788 [details]
WIP patch

Attachment 338788 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/7459393

New failing tests:
http/tests/navigation/disown-opener-header-same-origin.html
http/tests/navigation/disown-opener-header-cross-origin.html
Comment 6 EWS Watchlist 2018-04-25 15:43:37 PDT
Created attachment 338811 [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
Comment 7 EWS Watchlist 2018-04-25 15:50:36 PDT
Comment on attachment 338788 [details]
WIP patch

Attachment 338788 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/7459032

New failing tests:
http/tests/navigation/disown-opener-header-same-origin.html
http/tests/navigation/disown-opener-header-cross-origin.html
Comment 8 EWS Watchlist 2018-04-25 15:50:38 PDT
Created attachment 338812 [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
Comment 9 EWS Watchlist 2018-04-25 16:29:03 PDT
Comment on attachment 338788 [details]
WIP patch

Attachment 338788 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/7460128

New failing tests:
http/tests/navigation/disown-opener-header-same-origin.html
http/tests/navigation/disown-opener-header-cross-origin.html
Comment 10 EWS Watchlist 2018-04-25 16:29:05 PDT
Created attachment 338820 [details]
Archive of layout-test-results from ews114 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 11 Chris Dumez 2018-04-25 16:51:44 PDT
Created attachment 338825 [details]
WIP patch
Comment 12 EWS Watchlist 2018-04-25 18:06:26 PDT
Comment on attachment 338825 [details]
WIP patch

Attachment 338825 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/7461914

New failing tests:
http/tests/navigation/disown-opener-header-same-origin.html
http/tests/navigation/disown-opener-header-cross-origin.html
Comment 13 EWS Watchlist 2018-04-25 18:06:27 PDT
Created attachment 338840 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 14 Chris Dumez 2018-05-01 12:49:28 PDT
Created attachment 339215 [details]
WIP patch
Comment 15 EWS Watchlist 2018-05-01 14:04:19 PDT
Comment on attachment 339215 [details]
WIP patch

Attachment 339215 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/7527805

New failing tests:
http/tests/navigation/x-frame-isolate-popup.html
Comment 16 EWS Watchlist 2018-05-01 14:04:21 PDT
Created attachment 339223 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 17 EWS Watchlist 2018-05-01 14:40:46 PDT
Comment on attachment 339215 [details]
WIP patch

Attachment 339215 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/7527947

New failing tests:
http/tests/navigation/x-frame-isolate-popup.html
Comment 18 EWS Watchlist 2018-05-01 14:40:57 PDT
Created attachment 339225 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 19 EWS Watchlist 2018-05-01 15:02:33 PDT
Comment on attachment 339215 [details]
WIP patch

Attachment 339215 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/7528257

New failing tests:
http/tests/navigation/x-frame-isolate-popup.html
Comment 20 EWS Watchlist 2018-05-01 15:02:35 PDT
Created attachment 339226 [details]
Archive of layout-test-results from ews117 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 21 Chris Dumez 2018-05-01 16:36:41 PDT
Created attachment 339242 [details]
WIP patch
Comment 22 Chris Dumez 2018-05-08 16:43:55 PDT
Created attachment 339897 [details]
WIP patch

Working on adding more testing.
Comment 23 EWS Watchlist 2018-05-08 16:47:19 PDT
Attachment 339897 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/network/HTTPParsers.h:102:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 1 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 24 EWS Watchlist 2018-05-08 19:45:03 PDT
Comment on attachment 339897 [details]
WIP patch

Attachment 339897 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/7618268

New failing tests:
http/wpt/cross-origin-options/cross-origin-options-header.html
Comment 25 EWS Watchlist 2018-05-08 19:45:05 PDT
Created attachment 339922 [details]
Archive of layout-test-results from ews117 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 26 EWS Watchlist 2018-05-08 20:01:18 PDT
Comment on attachment 339897 [details]
WIP patch

Attachment 339897 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/7618435

New failing tests:
http/tests/security/canvas-remote-read-remote-video-redirect.html
http/tests/security/canvas-remote-read-remote-video-localhost.html
http/tests/security/webaudio-render-remote-audio-allowed-crossorigin.html
Comment 27 EWS Watchlist 2018-05-08 20:01:30 PDT
Created attachment 339923 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 28 Chris Dumez 2018-05-09 09:54:01 PDT
Created attachment 339973 [details]
WIP patch
Comment 29 Chris Dumez 2018-05-09 10:15:18 PDT
Created attachment 339978 [details]
WIP patch
Comment 30 Chris Dumez 2018-05-09 10:28:46 PDT
Created attachment 339980 [details]
WIP patch
Comment 31 EWS Watchlist 2018-05-09 11:41:55 PDT
Comment on attachment 339980 [details]
WIP patch

Attachment 339980 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/7625565

New failing tests:
http/wpt/cross-origin-options/allow-postmessage.html
Comment 32 EWS Watchlist 2018-05-09 11:41:56 PDT
Created attachment 339995 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 33 EWS Watchlist 2018-05-09 12:13:06 PDT
Comment on attachment 339980 [details]
WIP patch

Attachment 339980 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/7625976

New failing tests:
http/wpt/cross-origin-options/allow-postmessage.html
Comment 34 EWS Watchlist 2018-05-09 12:13:08 PDT
Created attachment 339998 [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
Comment 35 EWS Watchlist 2018-05-09 12:18:53 PDT
Comment on attachment 339980 [details]
WIP patch

Attachment 339980 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/7625715

New failing tests:
http/wpt/cross-origin-options/allow-postmessage.html
Comment 36 EWS Watchlist 2018-05-09 12:18:54 PDT
Created attachment 340000 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 37 EWS Watchlist 2018-05-09 12:46:36 PDT
Comment on attachment 339980 [details]
WIP patch

Attachment 339980 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/7626237

New failing tests:
http/wpt/cross-origin-options/allow-postmessage.html
Comment 38 EWS Watchlist 2018-05-09 12:46:37 PDT
Created attachment 340007 [details]
Archive of layout-test-results from ews117 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 39 Chris Dumez 2018-05-09 12:53:11 PDT
Created attachment 340010 [details]
Patch
Comment 40 EWS Watchlist 2018-05-09 14:10:23 PDT
Comment on attachment 340010 [details]
Patch

Attachment 340010 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/7627488

New failing tests:
http/wpt/cross-origin-options/allow-postmessage-from-deny.html
http/wpt/cross-origin-options/allow-postmessage.html
http/wpt/cross-origin-options/cross-origin-options-header.html
Comment 41 EWS Watchlist 2018-05-09 14:10:25 PDT
Created attachment 340022 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 42 EWS Watchlist 2018-05-09 14:41:40 PDT
Comment on attachment 340010 [details]
Patch

Attachment 340010 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/7627972

New failing tests:
http/wpt/cross-origin-options/allow-postmessage.html
Comment 43 EWS Watchlist 2018-05-09 14:41:42 PDT
Created attachment 340031 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 44 EWS Watchlist 2018-05-09 14:50:33 PDT
Comment on attachment 340010 [details]
Patch

Attachment 340010 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/7627740

New failing tests:
http/wpt/cross-origin-options/allow-postmessage.html
Comment 45 EWS Watchlist 2018-05-09 14:50:34 PDT
Created attachment 340035 [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
Comment 46 Chris Dumez 2018-05-09 15:10:48 PDT
Created attachment 340039 [details]
Patch
Comment 47 Chris Dumez 2018-05-09 15:21:28 PDT
Created attachment 340041 [details]
Patch
Comment 48 Chris Dumez 2018-05-09 15:31:07 PDT
Created attachment 340042 [details]
Patch
Comment 49 Geoffrey Garen 2018-05-09 15:40:29 PDT
Comment on attachment 340042 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=340042&action=review

r=me if EWS is happy

> Source/WebCore/dom/Document.cpp:7815
> +void Document::setCrossOriginOptions(CrossOriginOptions value)
> +{
> +    m_crossOriginOptions = value;
> +    if (auto* window = domWindow())
> +        window->setCrossOriginOptions(value);
> +}

Do we need to propagate m_crossOriginOptions inside Document::createDOMWindow() (covering the case where domWindow() is null at this point)?
Comment 50 Chris Dumez 2018-05-09 15:44:27 PDT
Comment on attachment 340042 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=340042&action=review

>> Source/WebCore/dom/Document.cpp:7815
>> +}
> 
> Do we need to propagate m_crossOriginOptions inside Document::createDOMWindow() (covering the case where domWindow() is null at this point)?

No.

> Source/WebCore/page/DOMWindow.cpp:405
> +    : AbstractDOMWindow(GlobalWindowIdentifier { Process::identifier(), generateObjectIdentifier<WindowIdentifierType>() }, document.crossOriginOptions())

because we already take care of grabbing the options from the Document when constructing the Window.

> Source/WebCore/page/DOMWindow.cpp:416
> +    setCrossOriginOptions(document.crossOriginOptions());

And even when moving a DOMWindow from one document to another.
Comment 51 Brent Fulgham 2018-05-09 16:01:49 PDT
Comment on attachment 340042 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=340042&action=review

> Source/WebKitLegacy/win/Interfaces/IWebPreferencesPrivate.idl:229
>      HRESULT setMenuItemElementEnabled([in] BOOL enabled);

}

[uuid(<<Generate and fill in some kind of UUID here>>)]
interface IWebPreferencesPrivate7 : IWebPreferencesPrivate6
{

> Source/WebKitLegacy/win/Interfaces/IWebPreferencesPrivate.idl:231
> +    HRESULT setCrossOriginOptionsSupportEnabled([in] BOOL enabled);

You can't add these here without breaking existing WebKit clients on Windows (e.g., iTunes).

Instead, we have to create a new IWebPreferencesPrivate7 with these two new values.

> Source/WebKitLegacy/win/WebView.cpp:5152
>  

COMPtr<IWebPreferencesPrivate7> prefsPrivate { Query, preferences };

> Source/WebKitLegacy/win/WebView.cpp:5158
>      COMPtr<IWebPreferencesPrivate6> prefsPrivate { Query, preferences };

Then delete this one line ^^^
Comment 52 Brent Fulgham 2018-05-09 16:02:33 PDT
Comment on attachment 340042 [details]
Patch

r- because this will break Windows clients. I've tried to give you the code changes to make for this to work safely.
Comment 53 Chris Dumez 2018-05-09 16:09:32 PDT
Created attachment 340046 [details]
Patch
Comment 54 Brent Fulgham 2018-05-09 16:10:40 PDT
Comment on attachment 340046 [details]
Patch

Looks good to me with the latest changes.
Comment 55 Chris Dumez 2018-05-09 16:12:07 PDT
Created attachment 340047 [details]
Patch
Comment 56 Chris Dumez 2018-05-09 16:12:23 PDT
(In reply to Brent Fulgham from comment #54)
> Comment on attachment 340046 [details]
> Patch
> 
> Looks good to me with the latest changes.

Thanks.
Comment 57 Chris Dumez 2018-05-09 16:33:40 PDT
Created attachment 340049 [details]
Patch
Comment 58 Chris Dumez 2018-05-09 16:34:04 PDT
(In reply to Chris Dumez from comment #57)
> Created attachment 340049 [details]
> Patch

Turned on the feature by default as suggested by Geoff offline.
Comment 59 WebKit Commit Bot 2018-05-09 17:33:43 PDT
Comment on attachment 340049 [details]
Patch

Clearing flags on attachment: 340049

Committed r231622: <https://trac.webkit.org/changeset/231622>
Comment 60 WebKit Commit Bot 2018-05-09 17:33:46 PDT
All reviewed patches have been landed.  Closing bug.
Comment 61 Chris Dumez 2018-05-09 18:24:12 PDT
Windows build fix: <https://trac.webkit.org/changeset/231625>
Comment 62 Michael[tm] Smith 2018-05-09 20:36:44 PDT
Has a spec for Cross-Origin-Options been published somewhere other than just in the changelog? (https://trac.webkit.org/changeset/231622/webkit)
Comment 63 Fujii Hironori 2018-05-09 23:33:17 PDT
Windows port crashes soon. I'm going to fix it.

Bug 185505 – REGRESSION(r231622) [Win] Crashes for null dereference of prefsPrivate in WebView::notifyPreferencesChanged
Comment 64 Daniel Bates 2018-05-09 23:55:38 PDT
Comment on attachment 340049 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=340049&action=review

> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:248
> +    auto isCrossOriginAccess = [&] {

Not at a computer with a checkout at the moment. I take it the return value and error message can change after calling BindingSecurity::shouldAllowAccessToDOMWindow() more than once with the same arguments and hence we need to use a lambda and 2 local variable as opposed to just a 2 local variable to cache its result and error message?

> Source/WebCore/dom/Document.cpp:7812
> +    m_crossOriginOptions = value;

Is it necessary to store this state on the document given that we also store it on the window?

> Source/WebCore/loader/FrameLoader.cpp:748
> +            if (!crossOriginOptionsHeader.isEmpty())

This code said that and empty header is treated analogous to “allow”. For your consideration, I suggest that this logic be part of parseCrossOriginOptionsHeader() so as to centralize all the parsing and default semantics related to this header similar to how we parse X-Frame-Options and then update this code to only call the parse routine if crossOriginOptionsHeader is non-null (as the null string is an artifact of how ResourceResponse::httpHeaderField() works).

> Source/WebCore/platform/network/HTTPParsers.cpp:918
> +    auto strippedHeader = stripLeadingAndTrailingHTTPSpaces(header);

I would have stored the return value in header as I do not feel that “stripped” adds much value, header is copied by value, and we can re-use the stack-space of the parameter instead of stack-allocating space for a local.

> Source/WebCore/platform/network/HTTPParsers.cpp:919
> +    if (strippedHeader.isEmpty())

Look at this. You handle the empty string case. This conditional always evaluates to false because we never invoke this function when header is the empty string by line 748 of FrameLoader.cpp. And FrameLoader is the only caller of this function. See my remark in that file. If you have your heart set on keeping line 748 as-is then we should remove this line and line 920 and add an assertion that header is a non-empty string.

Even better, just remove this code and let us fall through to the default case on line 928.

> LayoutTests/http/wpt/cross-origin-options/cross-origin-options-header-expected.txt:3
> +PASS Cross-origin iframe with 'Cross-Origin-Options: deny' HTTP header 

We should add tests for mIXED caSe header values since you use equalLettersIgnoringASCIICasein the parsing code. Also, we should add tests for when multiple Cross-Origin-Options are given in the same HTTP response.
Comment 65 Chris Dumez 2018-05-10 09:01:49 PDT
Comment on attachment 340049 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=340049&action=review

>> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:248
>> +    auto isCrossOriginAccess = [&] {
> 
> Not at a computer with a checkout at the moment. I take it the return value and error message can change after calling BindingSecurity::shouldAllowAccessToDOMWindow() more than once with the same arguments and hence we need to use a lambda and 2 local variable as opposed to just a 2 local variable to cache its result and error message?

Not sure I understand your comment. The result can change when called several times, which is why I cannot cache the result in a static.
The reason I have a lambda is to lazily initialize the cache value because there is one (common) code path that does not require a cross-origin check (There is a frame with this index and crossOriginOptions is Allow). Using a lambda avoids duplicating this code in other code paths.

>> Source/WebCore/dom/Document.cpp:7812
>> +    m_crossOriginOptions = value;
> 
> Is it necessary to store this state on the document given that we also store it on the window?

It'd be nice indeed but it is a little tricky because you can have a Document without a Window (and vice-versa). I'll dig deeper into it to see if this is feasible and follow-up if I can.

>> Source/WebCore/loader/FrameLoader.cpp:748
>> +            if (!crossOriginOptionsHeader.isEmpty())
> 
> This code said that and empty header is treated analogous to “allow”. For your consideration, I suggest that this logic be part of parseCrossOriginOptionsHeader() so as to centralize all the parsing and default semantics related to this header similar to how we parse X-Frame-Options and then update this code to only call the parse routine if crossOriginOptionsHeader is non-null (as the null string is an artifact of how ResourceResponse::httpHeaderField() works).

parseCrossOriginOptionsHeader() already does the right thing when passed an empty string so it would be fine to use isNull() here. I would do an unnecessary function call but this is not hot code anyway.

>> LayoutTests/http/wpt/cross-origin-options/cross-origin-options-header-expected.txt:3
>> +PASS Cross-origin iframe with 'Cross-Origin-Options: deny' HTTP header 
> 
> We should add tests for mIXED caSe header values since you use equalLettersIgnoringASCIICasein the parsing code. Also, we should add tests for when multiple Cross-Origin-Options are given in the same HTTP response.

Good idea, will extend testing.