Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Please discourage custom schemes #484

Open
nfischer opened this issue Dec 2, 2019 · 17 comments
Open

Please discourage custom schemes #484

nfischer opened this issue Dec 2, 2019 · 17 comments

Comments

@nfischer
Copy link

nfischer commented Dec 2, 2019

I'm an engineer on the Android WebView team. Custom schemes are poorly defined in web platform standards, and Android WebView generally struggles to maintain backwards compatibility for these. #348 and https://issuetracker.google.com/issues/140057007 are just a couple examples of this fragile use case breaking.

I see https://github.com/ionic-team/cordova-plugin-ionic-webview#scheme documents support for changing the scheme of content. While we encourage changing the scheme to https:// (and would even encourage making this the default, for improved web security), we strongly discourage any other scheme (data:, file:///, blob:, custom scheme, etc.). I would advise your docs to show the same recommendation.

If the app wants to distinguish injected content from real web content, they should host it on a unique domain rather than a unique scheme. This is the intended use case for https://developer.android.com/reference/androidx/webkit/WebViewAssetLoader.Builder.html#setDomain(java.lang.String).

CC @HazemSamir (the author of WebViewAssetLoader).

@jcesarmobile
Copy link
Member

People using custom schemes on android do it for matching the iOS scheme. Sadly on iOS, Apple doesn’t allow to intercept or use http nor https for serving local files. We can add a note to encourage people, but I think the Android webview should fix the issue and set the proper origin

@nfischer
Copy link
Author

nfischer commented Dec 4, 2019

Ah, I see. I agree that's very inconvenient. Does iOS restrict any other schemes? We originally considered supporting a custom scheme for intercepted content, but ultimately decided the AssetLoader design would be simpler (and would have better device coverage).

The issue is that the notion of an origin is only well defined for http(s) URLs. As I understand, the spec actually requires we treat custom schemes as origin-less. So it's not that it's something for us to "fix," but rather changing behavior would violate the spec. My colleague's response on this bug probably explains things more clearly: https://bugs.chromium.org/p/chromium/issues/detail?id=1029487#c4

@jcesarmobile
Copy link
Member

It restricts to any scheme handled by the webview, don’t give specifics, but some of them are https, http, file and data.

@NiklasMerz
Copy link
Contributor

People using custom schemes on android do it for matching the iOS scheme. Sadly on iOS, Apple doesn’t allow to intercept or use http nor https for serving local files.

Exactly that's what I am trying to find a solution for. The "Access-Control-Allow-Origin" header is now set to ionic://localhost and we would like to use that for Android too to not change that again.

I must agree that setting the app to run on https://ourdomain.com would be best and most future-proof stable solution. But right now this is not possible on iOS.

As I already discussed in the Chromium bug, I still think WebView could change the behavior according to the spec. RFC6454 is kind of vague with custom schemes and in my interpretation would allow hostnames to be part of the origin on custom schemes the app implements with shouldInterceptRequest. Another option would be a way to tell the Android WebView which schemes are valid to allow it to set the origin properly.

@nfischer
Copy link
Author

nfischer commented Dec 6, 2019

@NiklasMerz it sounds like your complaint is about setting up the "Access-Control-Allow-Origin" header. But even if you hosted your Android app on ionic://localhost, you would still need the header. Is there a problem with hosting the iOS app on ionic://localhost and the Android app on https://ourdomain.com? I haven't developed cordova/ionic apps myself, so I'm interested to learn the consequences for using different schemes on each platform.

As I already discussed in the Chromium bug, I still think WebView could change the behavior according to the spec. RFC6454 is kind of vague with custom schemes and in my interpretation would allow hostnames to be part of the origin on custom schemes the app implements with shouldInterceptRequest. Another option would be a way to tell the Android WebView which schemes are valid to allow it to set the origin properly.

I posted a response on crbug/1029487, since this Github probably isn't the best spot to discuss Android WebView APIs.

@NiklasMerz
Copy link
Contributor

NiklasMerz commented Dec 6, 2019

@NiklasMerz it sounds like your complaint is about setting up the "Access-Control-Allow-Origin" header. But even if you hosted your Android app on ionic://localhost, you would still need the header. Is there a problem with hosting the iOS app on ionic://localhost and the Android app on https://ourdomain.com? I haven't developed cordova/ionic apps myself, so I'm interested to learn the consequences for using different schemes on each platform.

Running Android on https://ourdomain.com and iOS on ionic://localhost means that the server must allow both origins. We are developing an app for an on-premise solution. Our previous release was configured to allow ionic://localhost and we told our customers to configure their servers with an additional setting to allow that origin. We now need to run our app for the new version of ionic on Android on https://ourdomain.com. The new app versions are always built to be backwards compatible but that means we are loosing the compatibility. That's why I am looking for a common origin for both platforms and ionic://localhost would be ideal because it was already set for that in the past.

I posted a response on crbug/1029487, since this Github probably isn't the best spot to discuss Android WebView APIs.

Thank you for the reponse. This makes it clear why it is like that.

@nfischer
Copy link
Author

nfischer commented Dec 6, 2019

I thought https://ourdomain.com was a placeholder for whatever domain is serving the content. My understanding is you don't need special CORS headers for that case, because it wouldn't be considered "cross origin." The intention of AssetLoader (and the original WebViewLocalServer which cordova-plugin-ionic-webview has forked) is you would choose a domain which will give your content the same origin as your real web server.

If your app connects with servers on multiple different domains, would it help for cordova-plugin-ionic-webview to give an API to set the domain during startup (this is how the AssetLoader API is designed) rather than statically in config.xml? I haven't looked closely at the plugin's implementation, so I'm not sure if this is something it could support even if it migrated to AssetLoader. Just trying to understand at which layer we're hitting the problem.

@jcesarmobile
Copy link
Member

what difference does it make if you do it with an API or with a preference? In the end you can only have a domain and if you connect with servers on multiple domains the problem will still be there

@NiklasMerz
Copy link
Contributor

NiklasMerz commented Dec 6, 2019 via email

@nfischer
Copy link
Author

nfischer commented Dec 7, 2019

what difference does it make if you do it with an API or with a preference?

I think the difference is that @NiklasMerz's app only knows which server it will connect to at runtime, they won't know at compile time. The config.xml preference only works if you know the desired origin at compile time.

In the end you can only have a domain and if you connect with servers on multiple domains the problem will still be there

I'm not sure you actually need to load content on multiple origins for this use case (you only need the right origin), but we support registering an arbitrary number of AssetLoaders, each of which can load content on their own origin. Here's some example code.

Our app is designed to always do CORS requests.

I see. Our team intends AssetLoader as a way to avoid CORS requests. From the sound of it, AssetLoader itself should be sufficient for this in native apps (you would just register the AssetLoader when the user inputs their origin, which you must already know prior to loading content in the web page anyway), and the only issue is this does't integrate well with this plugin's static preferences.

If it's possible, it might be nice if cordova-plugin-ionic-webview supported dynamically configuring the origin, to avoid CORS in cases like this. But I would hope most apps only need to connect to a predetermined list of origins, so they could host their content on those origins (this is what we recommend for AssetLoader/WebViewLocalServer, and it might be good to pass along the same recommendation in this repo's docs for scheme/domain).

@jcesarmobile
Copy link
Member

Since cordova apps load the app content since the beginning the hostname have to be configured before loading the app, so can’t be done with an API since plugins won’t work until the app code is loaded. Unless the API reloads the code with webview.reload/webview.loadUrl after changing the hostname

@NiklasMerz
Copy link
Contributor

NiklasMerz commented Dec 7, 2019

That means I could extend the plugin to set the hostname dynamically, but that would reload the whole app app, correct? That would be feasible for us as a workaround.

I don't know much about the AssetLoader. How would this work? The customer configures customer.com in the app, the app would reload to now run on "https://customer.com". It would now fetch all local resources for the app from "https://customer.com", but how do I do calls to the customers server? Does the AssetLoader know which URLs to get from the app assets and what has to be called to the server?

If that's possible, I will try that out on Monday and report back.

@nfischer Another option we considered would be to intercept all requests and do them natively to avoid CORS checks. But the intercept method does not allow access to POST data. Do you see any chance for https://crbug.com/155250 to be fixed. It is untriaged for some time but was discussed extensively.

@nfischer
Copy link
Author

nfischer commented Dec 9, 2019

Since cordova apps load the app content since the beginning the hostname have to be configured before loading the app, so can’t be done with an API since plugins won’t work until the app code is loaded.

In that case, this sounds out of scope for cordova-plugin-ionic-webview. I suspect this is a very limited use case, at which point I think a native app is probably a better path forward.

@nfischer Another option we considered would be to intercept all requests and do them natively to avoid CORS checks.

Even if you shouldInterceptRequest these resources, I think they should still be subject to the same CORS checks in Android WebView (otherwise apps proxying to custom net stacks would lack the CORS security feature, which would be bad). I'm not sure what you're suggesting here.

If you're going that far, I think you should just register your own AssetLoader (hooked up to your WebView's shouldInterceptRequest) to load your content on the appropriate origin. This will let you avoid CORS problems, because you can pick the "right" origin for your content.

But the intercept method does not allow access to POST data. Do you see any chance for crbug.com/155250 to be fixed. It is untriaged for some time but was discussed extensively.

It's on our team's radar, but not currently prioritized.

@NiklasMerz
Copy link
Contributor

NiklasMerz commented Dec 9, 2019

Even if you shouldInterceptRequest these resources, I think they should still be subject to the same CORS checks in Android WebView (otherwise apps proxying to custom net stacks would lack the CORS security feature, which would be bad). I'm not sure what you're suggesting here.

Nevermind, I was just thinking out loud ways to move http requests to native code where CORS is not a concern. But now I think we will go the "same origin route" or require server side changes.

I can report, I found a way to switch the hostname at runtime. My first attempt here:
master...GEDYSIntraWare:android-origin

I am now able to let the app run on the same originn like the server let's say https://customerexample.com/index.html. Now I can add paths to WebViewLocalServer that should not be handled by the local server and go through shouldInterceptRequests unchanged. That means my requests are same origin with the connected server. I think that's the best solution for now, even though it needs to reload the app after setting the hostname. Let's see how further testing goes.

Example app for testing

Regarding the original title of this issue, we could add a note that custom schemes may not be the best option for apps requiring CORS requests. Others should not walk down a wrong path like I did :-). See also #485

@mlynch
Copy link
Contributor

mlynch commented Dec 9, 2019

@nfischer thanks for reaching out, we really appreciate you guys communicating w/ us.

Are there any deprecations or things we should keep in mind here? i.e. is there some urgency in us finding a different solution?

Feel free to email me directly as well max AT ionic DOT io

@nfischer
Copy link
Author

nfischer commented Dec 9, 2019

Regarding the original title of this issue, we could add a note that custom schemes may not be the best option for apps requiring CORS requests. Others should not walk down a wrong path like I did :-). See also #485

Yeah, I think that's a good way to frame it.

Are there any deprecations or things we should keep in mind here? i.e. is there some urgency in us finding a different solution?

We haven't finalized any deprecations yet, but non-standard schemes have historically been fragile, and we recognize there's latency from updating docs discouraging this until all the apps in the wild migrate off it.

Another chromium team is working on a major rewrite of the CORS implementation (I forget the exact timeline, but should release in the next few months). There's a decent risk it won't be 100% backwards compatible, and apps using non-standard schemes are at a greater risk of seeing any incompatibilities. We would almost certainly fix such regressions as we discover them, but these sorts of bugs are often only discovered after they're released into the wild, so those fixes might take up to 6 weeks to go out. Ideally we catch this in our prerelease channels, but stuff often sneaks through due to small user population in those channels.

@NiklasMerz
Copy link
Contributor

I created a PR for switching the origin at runtime #486 . Going same origin this way was way better than the custom scheme for me. I hope this is the most future proof solution as well.

Let me know in the PR if I could change it to get it merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants