Bug 226678 – Move Timing-Allow-Origin checks to the network process

• Component: New Bugs
• Status: RESOLVED FIXED

[Alex Christensen]

Move Timing-Allow-Origin checks to the network process

[Alex Christensen]

Created attachment 430645 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Alex Christensen]

Created attachment 430785 [details]
Patch

[Alex Christensen]

Created attachment 430787 [details]
Patch

[Alex Christensen]

Created attachment 430868 [details]
Patch

[Alex Christensen]

Created attachment 430876 [details]
Patch

[Alex Christensen]

Created attachment 430893 [details]
Patch

[Alex Christensen]

Created attachment 430927 [details]
Patch

[Alex Christensen]

Created attachment 430974 [details]
Patch

[Alex Christensen]

I need to mark http/wpt/resource-timing/rt-revalidate-requests-2.html as failing on Windows, but otherwise this should be ready for review

[Chris Dumez]

Comment on attachment 430974 [details]
Patch

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

> Source/WebCore/platform/network/TimingAllowOrigin.cpp:42
> +    const String& timingAllowOriginString = response.httpHeaderField(HTTPHeaderName::TimingAllowOrigin);

auto& ?

> Source/WebCore/platform/network/TimingAllowOrigin.cpp:43
> +    const String& securityOrigin = initiatorSecurityOrigin.toString();

ditto.

> Source/WebCore/platform/network/TimingAllowOrigin.cpp:44
> +    for (auto& originWithSpace : timingAllowOriginString.split(',')) {

Wouldn't it be more efficient to iterate over StringView(timingAllowOriginString).split(',') ?

> Source/WebCore/platform/network/TimingAllowOrigin.cpp:45
> +        auto origin = stripLeadingAndTrailingHTTPSpaces(StringView(originWithSpace));

Since you want StringViews anyway?

> Source/WebCore/platform/network/TimingAllowOrigin.h:33
> +WEBCORE_EXPORT bool passesTimingAllowOriginCheck(const ResourceResponse&, const WebCore::SecurityOrigin& initiatorSecurityOrigin);

WebCore:: is unnecessary.

> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:525
> +void ResourceHandle::platformLoadResourceSynchronously(NetworkingContext* context, const ResourceRequest& request,  StoredCredentialsPolicy storedCredentialsPolicy, SecurityOrigin* sourceOrigin, ResourceError& error, ResourceResponse& response, Vector<uint8_t>& data)

extra space before StoredCredentialsPolicy

> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:540
> +    RefPtr<ResourceHandle> handle = adoptRef(new ResourceHandle(context, request, &client, defersLoading, shouldContentSniff, shouldContentEncodingSniff, sourceOrigin, false));

A comment by the 'false' to clarify what it means or an enum class would be nice

> LayoutTests/imported/w3c/web-platform-tests/resource-timing/buffer-full-inspect-buffer-during-callback-expected.txt:2
> +Harness Error (TIMEOUT), message = null

Please skip in TestExpectations.

> LayoutTests/imported/w3c/web-platform-tests/resource-timing/buffer-full-set-to-current-buffer-expected.txt:2
> +Harness Error (TIMEOUT), message = null

Please skip in TestExpectations.

> LayoutTests/imported/w3c/web-platform-tests/resource-timing/document-domain-no-impact-opener-expected.txt:2
> +Harness Error (TIMEOUT), message = null

Please skip test in TestExpectations to avoid slowing runs.

[Alex Christensen]

Created attachment 431092 [details]
Patch

[Alex Christensen]

Marked EWS failing tests as flaky and landed in r278738

[Radar WebKit Bug Importer]

<rdar://problem/79166791>

[Anne van Kesteren]

*** Bug 184627 has been marked as a duplicate of this bug. ***