Bug 172869

Summary: Prevent scheme handlers from handling all built-in URL schemes
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebKit2Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, commit-queue
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Brady Eidson 2017-06-02 12:57:46 PDT
Prevent scheme handlers from handling all built-in URL schemes

<rdar://problem/32404790>
Comment 1 Brady Eidson 2017-06-02 14:25:57 PDT
Created attachment 311860 [details]
Patch
Comment 2 Andy Estes 2017-06-02 14:47:37 PDT
Comment on attachment 311860 [details]
Patch

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

> Source/WebCore/platform/SchemeRegistry.cpp:63
> +        // Other misc schemes that the SchemeRegistry doesn't know about.
> +        schemes.get().add("webkit-fake-url");
> +#if PLATFORM(MAC)
> +        schemes.get().add("safari-extension");
> +#endif

There's also x-apple-ql-id (QLPreviewProtocol()) and x-apple-content-filter (ContentFilter::urlScheme()).
Comment 3 Andy Estes 2017-06-02 14:57:22 PDT
What happens if someone tries to register schemes used by system apps (tel:, mailto:, etc.)? Those were probably hijackable by NSURLProtocol, so maybe they should be hijackable here too. Just wondering if you'd thought about it.
Comment 4 Brady Eidson 2017-06-02 15:02:59 PDT
(In reply to Andy Estes from comment #3)
> What happens if someone tries to register schemes used by system apps (tel:,
> mailto:, etc.)? Those were probably hijackable by NSURLProtocol, so maybe
> they should be hijackable here too. Just wondering if you'd thought about it.

We have thought about it, and they definitely get to be hijackable. They are not URLs WebKit handles internally.
Comment 5 Brady Eidson 2017-06-02 15:11:19 PDT
Created attachment 311870 [details]
Patch
Comment 6 Andy Estes 2017-06-02 15:15:28 PDT
Comment on attachment 311870 [details]
Patch

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

> Source/WebCore/platform/SchemeRegistry.cpp:73
> +#if PLATFORM(IOS)
> +        schemes.get().add(QLPreviewProtocol());
> +#endif

This might break tvOS and watchOS builds. I'd use USE(QUICK_LOOK) instead.
Comment 7 Brady Eidson 2017-06-02 15:17:47 PDT
Created attachment 311874 [details]
Patch
Comment 8 WebKit Commit Bot 2017-06-02 16:07:12 PDT
Comment on attachment 311874 [details]
Patch

Clearing flags on attachment: 311874

Committed r217738: <http://trac.webkit.org/changeset/217738>
Comment 9 WebKit Commit Bot 2017-06-02 16:07:14 PDT
All reviewed patches have been landed.  Closing bug.