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

feat(android): add WebViewAssetLoader proxy handler for cdvfile #513

Merged
merged 10 commits into from
Mar 15, 2022

Conversation

erisu
Copy link
Member

@erisu erisu commented Feb 9, 2022

Motivation and Context

Fetching files from the persistent and temporary file systen while using custom scheme+hostname.

Description

This PR adds a proxy handler for the WebViewAssetLoader.

Since cdvfile://localhost no longer works, this PR implements a new URL pattern that works with custom scheme+hostname. It will fetch and return a WebResponse of the file located in the native file system.

E.g.:

Old cdvfile URL:

cdvfile://localhost/persistent/image.png

will become:

https://localhost/__cdvfile_persistent__/image.png

and continue to map to the native file:

file:///data/user/0/<APP ID>/files/files/image.png

If you configure the app to use http or with a different hostname, other then localhost, it will change as such.

Resolves:

Need confirmation:

Depending on use case and app configuration, it appears that many the file url was being used when the app is served though the custom scheme+hostname setup. Only with AndroidInsecureFileModeEnabled will the file path work. This PR will introduce changes which will make it possible to load files with the custom scheme+hostname setup but with a different URL structure.

Usage:

Use toURL, NOT toInternalURL.

The toURL method will call the toInternalURL when necessary.

  • IF AndroidInsecureFileModeEnabled is set to false or not defined, the toURL method will call and return the results from toInternalURL.

    Example result: https://localhost/__cdvfile_persistent__/image.png.

    The scheme and hostname may vary depending on its configurations.

  • IF AndroidInsecureFileModeEnabled is set to true, the toURL method will return the native URL.

    Example Result: file:///data/user/0/<APP ID>/files/files/image.png.

What happens if toInternalURL is called directly?

  • IF AndroidInsecureFileModeEnabled is set to false or not defined, calling the toInternalURL method will not have any negative effect. The correct URL pattern will be returned.
  • IF AndroidInsecureFileModeEnabled is set to true, calling the toInternalURL method will return a URL pattern that does not exist and is reachable. This will cause issues in fetching and loading of resources.

Testing

  • npm t
  • `cordova build

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

@NiklasMerz NiklasMerz left a comment

Choose a reason for hiding this comment

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

Code LGTM

plugin.xml Outdated Show resolved Hide resolved
@JustTestCode
Copy link

helpful

plugin.xml Outdated Show resolved Hide resolved
@skmbr
Copy link

skmbr commented Feb 25, 2022

Sorry if this is the wrong place to ask, but as the current latest version is from 2019, once this gets merged how long is it likely to be for a new release to be available?

Or will I need to build the plugin myself somehow rather than using cordova plugin add... ?

@erisu
Copy link
Member Author

erisu commented Feb 26, 2022

Sorry if this is the wrong place to ask, but as the current latest version is from 2019, once this gets merged how long is it likely to be for a new release to be available?

Or will I need to build the plugin myself somehow rather than using cordova plugin add... ?

@skmbr I can not give you an exact answer, but hoping to prepare a release shortly after.

This PR fixes a lot of the issues but I am trying to investigate one alternative solution. (This explains why I have the PR in draft.)

I am hoping with the alternative solution I will not need to introduce new front-end JS code (E.g. getCdvURL()). I am hoping to reuse the toInternalURL() method instead.

My vision of the alternative solution's use cases is:

  1. If AndroidInsecureFileModeEnabled = true, toInternalURL()/toURL() will continue to return the file:// URL.
  2. If AndroidInsecureFileModeEnabled is not defined (false), toInternalURL() will return the scheme + hostname combination URL. This URL is created from the native side.

Because toURL already calls toInternalURL, this alternative change would mean that app developers do not need to update or change their front-end code. Simply updating the plugin would be all that is needed.

As a side note, the alternative solution would still use all of the native changes I made in this PR.

@skmbr
Copy link

skmbr commented Feb 26, 2022

I can not give you an exact answer, but hoping to prepare a release shortly after.

@erisu That's good to hear! Thank you!

That alternative solution sounds great too.

If AndroidInsecureFileModeEnabled = true, toInternalURL()/toURL() will continue to return the file:// URL.

One thing I will say is that toInternalURL was returning a cdvfile url for me after enabling AndroidInsecureFileModeEnabled so I had to switch to toURL to get a file:// url.

Looking forward to this being solved so I can do a proper release of my app again. Luckily I have codepush so have still been able to get fixes and small updates out, but getting to the point where I need a full release targeting > SDK 30 to keep up with new features in the iOS version.

Thanks again for all your work on this!

@xtof974
Copy link

xtof974 commented Mar 2, 2022

Facing the same issue. Any updates about this PR ?
Thx for your work !

@LongTimeNoTea
Copy link

Thanks for your work! I'm waiting for this to update my app!

@erisu
Copy link
Member Author

erisu commented Mar 7, 2022

I updated the PR description to reflect the latest changes. Read the sections Usage: and What happens if toInternalURL is called directly? to understand about the use case and changes.

@NiklasMerz
Copy link
Member

We could add the Usage part to the README of this plugin.

Copy link
Contributor

@breautek breautek left a comment

Choose a reason for hiding this comment

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

Tested via using camera plugin to snap a picture and then passing the provided URL to the file plugin to resolve to a FileEntry, then use .toURL() to display in the DOM.

@erisu erisu marked this pull request as ready for review March 7, 2022 16:04
@erisu erisu merged commit 3366ea6 into apache:master Mar 15, 2022
@erisu erisu deleted the feat/android-webassetloader-cdvfile branch March 15, 2022 07:21
tmrkk added a commit to tinymobilerobots/cordova-plugin-file that referenced this pull request Mar 25, 2022
Abaddon-88 added a commit to webfactor/cordova-plugin-file that referenced this pull request Mar 28, 2022
@skmbr
Copy link

skmbr commented Apr 2, 2022

@erisu Great that this is merged in now. Thanks for all your hard work!

Any news on when there might be a release? Or is the only option to install it via the github url?

@kalandher
Copy link

kalandher commented Apr 26, 2022

> Task :app:compileDebugJavaWithJavac FAILED platforms\android\app\src\main\java\com\silkimen\cordovahttp\CordovaHttpDownload.java:12: error: package org.apache.cordova.file does not exist import org.apache.cordova.file.FileUtils; ^ platforms\android\app\src\main\java\com\silkimen\cordovahttp\CordovaHttpDownload.java:33: error: cannot find symbol JSONObject fileEntry = FileUtils.getFilePlugin().getEntryForFile(file); ^ symbol: variable FileUtils location: class CordovaHttpDownload Note: Some input files use or override a deprecated API. Note: Recompile with -Xlint:deprecation for details. Note: Some input files use unchecked or unsafe operations. Note: Recompile with -Xlint:unchecked for details. 2 errors FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':app:compileDebugJavaWithJavac'. > Compilation failed; see the compiler error output for details. * Try: Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights. * Get more help at https://help.gradle.org Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0. Use '--warning-mode all' to show the individual deprecation warnings. See https://docs.gradle.org/6.5/userguide/command_line_interface.html#sec:command_line_warnings

@erisu Its working fine when i'm installing latest plugin for the the first time, when i uninstall and try to reinstall its throwing me error in 'cordova-plugin-advanced-http' code

pmcquay pushed a commit to BetterImpact/cordova-plugin-file that referenced this pull request Sep 2, 2022
…he#513)

* feat: add WebAssetLoader proxy handler for cdvfile
* fix: update the fileTarget replace string
* chore: make androidx.webkit:webkit configurable & default to 1.4.0
* feat: toURL to return file or custom scheme based on window location
* chore: remove unused variable
* chore: add other file systems to check
* chore: remove comment
* feat: bump cordova-android requirement to >=10.0.0 for AndroidX usage
* doc: updated readme to include the Android changes
@terryjiang2020
Copy link

Hi there, this one is creating the URL that my Android app is not able to recognize. The URL constantly returns "not found" on the device, although it exists when I check file system. Can anyone help on this?

@mirko77
Copy link

mirko77 commented Mar 16, 2023

So while this URL was resolved correctly with version 6.0.2

content://com.android.providers.downloads.documents/document/raw%3A%2Fstorage%2Femulated%2F0%2FDownload%2F00052123047880195188.pdf

on 7.0.0 I get error 1 (not found)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment