fix: CSV/JSON export via WebAppInterface + FileProvider (fixes #104)#186
Conversation
Greptile SummaryThis PR adds native Android handling for CSV and JSON exports from the WebView. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "fix: harden downloadFile against path tr..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
|
|
||
| private fun downloadFile(content: String, filename: String, mimetype: String) { | ||
| val file = File(mContext.getExternalFilesDir(null), filename) |
There was a problem hiding this comment.
filename comes from JavaScript and is used as a path without validation. If the web UI passes a blank name or a name containing path separators such as ../, this can write outside the configured FileProvider root or target a directory, and the export then crashes in writeText or getUriForFile instead of opening the file.
There was a problem hiding this comment.
Fixed in 47bea06: use File(filename).name to strip any path components from the JS-supplied name before creating the File. Empty names fall back to "export".
| val intent = Intent(Intent.ACTION_VIEW) | ||
| intent.setDataAndType(uri, mimetype) | ||
| intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION or Intent.FLAG_ACTIVITY_NO_HISTORY) | ||
| mContext.startActivity(intent) |
There was a problem hiding this comment.
startActivity can throw when no installed app handles text/csv or application/json. A valid CSV or JSON export on a device without a matching viewer can terminate the bridge call instead of showing a recoverable error.
There was a problem hiding this comment.
Fixed in 47bea06: wrapped startActivity in a try/catch (e: ActivityNotFoundException) so devices without a CSV/JSON viewer log the error and return cleanly instead of crashing the bridge.
| } | ||
|
|
||
| private fun downloadFile(content: String, filename: String, mimetype: String) { | ||
| val file = File(mContext.getExternalFilesDir(null), filename) |
There was a problem hiding this comment.
getExternalFilesDir(null) can return null, but this path passes it directly to File. On devices or profiles where external app storage is unavailable, starting an export throws before the CSV or JSON file is written.
There was a problem hiding this comment.
Fixed in 47bea06: null-check getExternalFilesDir(null) with an early return and a Log.e when external storage is unavailable.
|
|
||
| private fun downloadFile(content: String, filename: String, mimetype: String) { | ||
| val file = File(mContext.getExternalFilesDir(null), filename) | ||
| file.writeText(content) |
There was a problem hiding this comment.
writeText can throw for normal storage failures such as a full disk, denied write, invalid child path, or missing parent directory. Because the bridge does not catch that exception, a storage problem during export can crash the WebView bridge instead of reporting that the file could not be saved.
There was a problem hiding this comment.
Fixed in 47bea06: wrapped file.writeText(content) in a try/catch (e: Exception) that logs the error and returns early, so a full-disk or permission failure doesn't propagate up to the WebView bridge.
Adds a JavaScript bridge so the webui can trigger native file downloads on Android. The original download-as-blob approach crashed because the Android WebView cannot initiate system downloads the same way a browser does. Changes: - WebAppInterface: exposes downloadCSV() and downloadJSON() to window.Android - Uses FileProvider (required API 24+) instead of Uri.fromFile(), which throws FileUriExposedException on Android 7.0 and later - Adds res/xml/file_paths.xml for the provider's path config - Declares the FileProvider in AndroidManifest.xml Depends on ActivityWatch/aw-webui#532 (webui side of the bridge). Fixes ActivityWatch#104
…rrors, missing viewer
TimeToBuildBob
force-pushed
the
fix/csv-export-fileprovider
branch
from
d242bf9 to
47bea06
Compare
|
@greptileai review |
|
CI-green and mergeable (Greptile 5/5) — waiting only on a maintainer click. This PR is ready to merge, but the bot has pull-only access to this repo and can't self-merge — surfacing it here so it isn't lost. The monitoring loop will stop re-flagging it now that this note is posted. |
2 participants
Takes over from #109 (Erik asked @TimeToBuildBob to take over).
Problem
The original crash (#104) happens because the Android WebView can't trigger system downloads the way a desktop browser does. PR #109 addressed this with a
JavascriptInterfacebridge — the right approach — but usedUri.fromFile()which throwsFileUriExposedExceptionon Android 7.0+ (API 24+), causing a new crash.Fix
WebUIFragment.kt: AddsWebAppInterfaceexposingdownloadCSV()/downloadJSON()towindow.Android, wired into the WebView viaaddJavascriptInterface. UsesFileProvider.getUriForFile()instead ofUri.fromFile()withFLAG_GRANT_READ_URI_PERMISSIONso the viewer app can read the file.AndroidManifest.xml: Declares theFileProviderwith${applicationId}.providerauthority.res/xml/file_paths.xml: Provider path config scoped togetExternalFilesDir(null).Dependencies
Depends on ActivityWatch/aw-webui#532 (adds the
window.Android.downloadCSV()call on the webui side).Testing
Could not run the Android emulator in this environment. The pattern (FileProvider +
@JavascriptInterface) is the standard Android approach for WebView→native file sharing since API 24.