WebPageProxy should use the right path for sandbox extension
<rdar://problem/50772810>
Created attachment 372216 [details] Patch
Comment on attachment 372216 [details] Patch Attachment 372216 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/12488522 Number of test failures exceeded the failure limit.
Created attachment 372218 [details] Archive of layout-test-results from ews104 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 372216 [details] Patch Attachment 372216 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/12488608 New failing tests: http/tests/resourceLoadStatistics/website-data-removal-for-site-navigated-to-with-link-decoration.html
Created attachment 372219 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.14.5
How will be subresources loaded after going through this path?
Created attachment 372275 [details] Patch
Created attachment 372340 [details] Patch
Created attachment 372350 [details] Patch
Comment on attachment 372350 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372350&action=review > Source/WebKit/ChangeLog:10 > + When doing process swapping, a file based loading may end up being loaded through loadRequestWithNavigationShared. > + In that case, the sandbox extension path is set to '/' which might not be readable by the UIProcess. Why is it that PSON (and only PSON) forgets the appropriate sandbox extension path?
(The reason I ask is that it's not clear that requesting access to the URL parent folder is the right policy in all cases. Our API allows our client to specify any folder for read access. If our client specified some folder that is not the URL parent folder, we need to honor that.)
> > Source/WebKit/ChangeLog:10 > > + When doing process swapping, a file based loading may end up being loaded through loadRequestWithNavigationShared. > > + In that case, the sandbox extension path is set to '/' which might not be readable by the UIProcess. > > Why is it that PSON (and only PSON) forgets the appropriate sandbox > extension path? PSON makes the issue more visible but this is probably a pre-existing issue. For instance in case of web process crash, we will not go through loadFile:, so will not provide the expected sandbox extension. A full fix should be to remember the appropriate sandbox extension path. > (The reason I ask is that it's not clear that requesting access to the URL > parent folder is the right policy in all cases. Our API allows our client to > specify any folder for read access. If our client specified some folder that > is not the URL parent folder, we need to honor that.) Agreed, the current patch seems good enough to me for the common case. I can have a go at a full fix if useful.
> Agreed, the current patch seems good enough to me for the common case. > I can have a go at a full fix if useful. Let's try the full fix. I recently learned about all the special rules Safari uses to compute the "allowingReadAccessToURL" URL, and they were super surprising to me. And the fact that this is a regression even though it's been our behavior for months is also surprising. It would be nice to land on a design that avoids future surprises, and honors the stated meaning of the API.
Created attachment 372513 [details] Patch
Created attachment 372556 [details] Patch
Created attachment 372574 [details] Patch
Attachment 372574 [details] did not pass style-queue: ERROR: Source/WebKit/UIProcess/WebPageProxy.h:1901: The parameter name "url" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Patch ready for review. This patch does not tackle the following things: Resource directory could be saved/restored when saving/restoring state. The selection of a resource directory when adding a history item from WebProcess is racy.
Comment on attachment 372574 [details] Patch r=me
Comment on attachment 372574 [details] Patch Clearing flags on attachment: 372574 Committed r246694: <https://trac.webkit.org/changeset/246694>
All reviewed patches have been landed. Closing bug.