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

doc(android): document AndroidInsecureFileModeEnabled #1172

Merged
merged 4 commits into from
Aug 17, 2021

Conversation

NiklasMerz
Copy link
Member

Platforms affected

Android

Motivation and Context

Document apache/cordova-android#1222

Description

Document setting and add warning from https://developer.android.com/reference/android/webkit/WebSettings#setAllowUniversalAccessFromFileURLs(boolean)

Testing

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • I've updated the documentation if necessary

Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should probably say something about backwards compatibility and avoid data loss on existing apps

Also, just noticed there is another section in the file where we put the actual preference tag with an example, can you add it there too?

@NiklasMerz
Copy link
Member Author

I am not sure if this description of all config.xml preferences is the right place for that.

Maybe here on the general Android page? Of course we need it in the announce post.

@jcesarmobile
Copy link
Member

I've fixed the merge conflicts after merging my PR that removed the AndroidX preference.
I edited right before you commented, so you probably didn't see, can you add the preference to the examples section too?

If you don't want to go into detail about the data loss in this PR it's ok, but it should still be about loading from file instead of accessing file urls, because it's not the same thing.
If you load from file url, you obviously need to access file urls, but you might still want to load from https and access file urls (i.e. take a picture from the camera and assign it to a img tag) and can wrongly enable this preference to do so.

BTW, that example made me think, do we have some helper like on iOS to convert file urls like the one you get from camera to https urls? would the assets loader be able to load those urls somehow?

NiklasMerz and others added 2 commits May 6, 2021 12:22
Co-authored-by: jcesarmobile <jcesarmobile@gmail.com>
Describe origin change and data loss
Which means the app starts with the URL `file:///android_asset/www/index.html`. Loading `file:///` URLs is considered insecure
and [Android has deprecated support](https://developer.android.com/reference/android/webkit/WebSettings#setAllowUniversalAccessFromFileURLs(boolean)).
Cordova Android 10.0.0 now uses an Android API called `WebViewAssetLoader` to load web content via the HTTP(S) scheme (`https://localhost`) by default.
Therefore the app now starts with the URL `https://localhost/` instead of `file:///android_asset/www/index.html`. Because this is a new origin you might encouter data loss and you need to migrate your web data (Localstorage, IndexedDB etc).
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add more information around migration and data loss once we have a good suggestion?

@NiklasMerz
Copy link
Member Author

I've fixed the merge conflicts after merging my PR that removed the AndroidX preference.
I edited right before you commented, so you probably didn't see, can you add the preference to the examples section too?

Done

If you don't want to go into detail about the data loss in this PR it's ok, but it should still be about loading from file instead of accessing file urls, because it's not the same thing.

I changed the wording and found the right page to describe that. Maybe others can jump in and help to improve it.

If you load from file url, you obviously need to access file urls, but you might still want to load from https and access file urls (i.e. take a picture from the camera and assign it to a img tag) and can wrongly enable this preference to do so.

BTW, that example made me think, do we have some helper like on iOS to convert file urls like the one you get from camera to https urls? would the assets loader be able to load those urls somehow?

TBH I don't really what happens with camera and other files. Maybe it still works? I need to test it out.

@NiklasMerz NiklasMerz requested review from breautek and dpogue May 6, 2021 10:29
@erisu erisu merged commit 4d887aa into apache:master Aug 17, 2021
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

Successfully merging this pull request may close these issues.

None yet

3 participants