WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
207917
Re-disable top-level data URL navigations
https://bugs.webkit.org/show_bug.cgi?id=207917
Summary
Re-disable top-level data URL navigations
Brent Fulgham
Reported
2020-02-18 15:14:11 PST
Calls to WKPage and WKWebView API used to load data and strings directly should be recognized by the loading code as being from client API calls. This brings these API behaviors into alignment with similar API for loading URLs and URLRequests directly. This change also allows us to enforce stricter handling of Data URLs, and to remove the need to explicitly permit top-level data URL navigation when client APIs are used.
Attachments
Patch
(30.46 KB, patch)
2020-02-18 15:22 PST
,
Brent Fulgham
no flags
Details
Formatted Diff
Diff
Patch
(30.56 KB, patch)
2020-02-18 15:29 PST
,
Brent Fulgham
no flags
Details
Formatted Diff
Diff
Patch
(6.32 KB, patch)
2020-02-18 16:01 PST
,
Brent Fulgham
no flags
Details
Formatted Diff
Diff
Patch
(4.34 KB, patch)
2020-02-18 18:51 PST
,
Brent Fulgham
no flags
Details
Formatted Diff
Diff
Patch for landing
(4.26 KB, patch)
2020-02-19 10:04 PST
,
Brent Fulgham
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2020-02-18 15:15:12 PST
<
rdar://problem/59568037
>
Brent Fulgham
Comment 2
2020-02-18 15:22:19 PST
Created
attachment 391103
[details]
Patch
Brent Fulgham
Comment 3
2020-02-18 15:25:49 PST
Comment on
attachment 391103
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=391103&action=review
> Source/WebKit/UIProcess/ProvisionalPageProxy.cpp:156 > + m_page.loadDataWithNavigationShared(m_process.copyRef(), m_webPageID, navigation, data, MIMEType, encoding, baseURL, userData, WebCore::ShouldTreatAsContinuingLoad::Yes, WebCore::ShouldTreatAsClientOrUserInput::No, WTFMove(websitePolicies), navigation.lastNavigationAction().shouldOpenExternalURLsPolicy);
@Youenn, @Chris: Should we ever treat ProvisionalPageProxy loads as if they were generated from client API? It doesn't look like we do for normal load requests, so I don't think we would for data loads, either.
Brent Fulgham
Comment 4
2020-02-18 15:29:27 PST
Created
attachment 391108
[details]
Patch
Chris Dumez
Comment 5
2020-02-18 15:36:20 PST
Comment on
attachment 391108
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=391108&action=review
> Source/WebKit/UIProcess/WebPageProxy.cpp:1385 > +void WebPageProxy::loadDataWithNavigationShared(Ref<WebProcessProxy>&& process, WebCore::PageIdentifier webPageID, API::Navigation& navigation, const IPC::DataReference& data, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData, ShouldTreatAsContinuingLoad shouldTreatAsContinuingLoad, ShouldTreatAsClientOrUserInput shouldTreatAsClientOrUserInput, Optional<WebsitePoliciesData>&& websitePolicies, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy)
This is always called from API so it seems odd to take a ShouldTreatAsClientOrUserInput parameter. When is it ever ShouldTreatAsClientOrUserInput::No?
Brent Fulgham
Comment 6
2020-02-18 15:49:02 PST
Note: We allowed top-level navigations to Data URLs in
r255961
. We can go back to our previous state with this change.
Brent Fulgham
Comment 7
2020-02-18 15:56:30 PST
(In reply to Chris Dumez from
comment #5
)
> Comment on
attachment 391108
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=391108&action=review
> > > Source/WebKit/UIProcess/WebPageProxy.cpp:1385 > > +void WebPageProxy::loadDataWithNavigationShared(Ref<WebProcessProxy>&& process, WebCore::PageIdentifier webPageID, API::Navigation& navigation, const IPC::DataReference& data, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData, ShouldTreatAsContinuingLoad shouldTreatAsContinuingLoad, ShouldTreatAsClientOrUserInput shouldTreatAsClientOrUserInput, Optional<WebsitePoliciesData>&& websitePolicies, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy) > > This is always called from API so it seems odd to take a > ShouldTreatAsClientOrUserInput parameter. When is it ever > ShouldTreatAsClientOrUserInput::No?
Excellent -- this can be a much simpler change, then.
Brent Fulgham
Comment 8
2020-02-18 16:01:25 PST
Created
attachment 391113
[details]
Patch
Brent Fulgham
Comment 9
2020-02-18 18:51:44 PST
Created
attachment 391125
[details]
Patch
Darin Adler
Comment 10
2020-02-18 19:57:28 PST
Comment on
attachment 391125
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=391125&action=review
> Source/WebCore/ChangeLog:3 > + Recognize WKPage and WKWebView calls as generated by client API use
Wrong title here.
Chris Dumez
Comment 11
2020-02-19 08:21:38 PST
Comment on
attachment 391125
[details]
Patch LGTM too.
Brent Fulgham
Comment 12
2020-02-19 10:04:32 PST
Created
attachment 391175
[details]
Patch for landing
WebKit Commit Bot
Comment 13
2020-02-19 10:36:49 PST
Comment on
attachment 391175
[details]
Patch for landing Clearing flags on attachment: 391175 Committed
r256925
: <
https://trac.webkit.org/changeset/256925
>
WebKit Commit Bot
Comment 14
2020-02-19 10:36:51 PST
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.
Top of Page
Format For Printing
XML
Clone This Bug