fix: allow webview to load non-web URLs

[zcbenz]

By default Chromium does not allow webview to load non-web URLs like file://, this patch disables the behavior.

This is required by https://github.com/electron/electron/pull/13869.

[deepak1556]

This seems to bypass external protocol and web safe scheme check for other non-web schemes, can we just add a check for file alongside and preserve the existing behavior ? If electron custom protocol schemes are failing to load, we have an api which consumers should use to register them as web safe scheme https://github.com/electron/electron/blob/master/atom/browser/api/atom_api_protocol.cc#L42 , thoughts ?

Another solution I can think of from my experience with integrating PDF is, it had a similar problem because pdfs are embedded in a webui page using <embed> which are essentially browser plugins and they aren't allowed to load local/non-web schemes by default. But the way its done is by intercepting the initial request where we tell chromium that the resource will be handled as a stream and we make chromium believe it by changing the origin to chrome://pdf-viewer https://github.com/electron/electron/blob/master/atom/browser/atom_resource_dispatcher_host_delegate.cc#L116-L143 . Then the final resource is loaded by us a stream https://github.com/electron/electron/blob/master/atom/browser/ui/webui/pdf_viewer_ui.cc#L235-L249, that way local schemes can be loaded selectively. Maybe similar thing can be attempted for <webview> , which would allow us to preserve existing security policy of chromium ?

[zcbenz]

@deepak1556 Currently webview is supposed to be able to load arbitrary URL, requiring custom protocols to be web-safe would be a breaking change.

While the comment in the related Chromium code says "do not allow browser plugin guests to navigate to non-web URLs", it actually only affects OOPIF based guests. Our old browser plugin based webview did not hit this check, and this check only starts to affect us after migrating webview to OOPIF.

So this patch should not affect normal pages, and it only makes OOPIF based webview behave the same with browser plugin based webview.

I'm not sure if I can adapt the solution for PDF viewer for OOPIF webview. PDF is loaded as resource, while the non_web_url_in_guest check happens as part of page navigation, and it happens before any kind of loader.

[deepak1556]

requiring custom protocols to be web-safe would be a breaking change.

Hmm yeah you are right, can we consider this for https://github.com/electron/electron/blob/master/docs/api/breaking-changes.md in the future, seems to make this check a bit strict.

So this patch should not affect normal pages, and it only makes OOPIF based webview behave the same with browser plugin based webview.

👍

I'm not sure if I can adapt the solution for PDF viewer for OOPIF webview. PDF is loaded as resource, while the non_web_url_in_guest check happens as part of page navigation, and it happens before any kind of loader.

Yeah, makes sense. Maybe if we associate the <webview> with an internal mime type then we could get the loaders to kick in and intercept them. But yeah this can be done as an experiment separately, not in the scope of the current migration. Thanks for the clarification!

[zcbenz]

Hmm yeah you are right, can we consider this for https://github.com/electron/electron/blob/master/docs/api/breaking-changes.md in the future, seems to make this check a bit strict.

I'm 👍 matching Chromium's security rules on webview.