Write mirrors to the app-specific external directory (scoped storage)#11
Open
xroche wants to merge 2 commits into
Open
Write mirrors to the app-specific external directory (scoped storage)#11xroche wants to merge 2 commits into
xroche wants to merge 2 commits into
Conversation
Scoped storage leaves exactly one external location writable without a permission, and this moves the mirror root to it. From API 30 on, WRITE_EXTERNAL_STORAGE grants nothing for shared storage and requestLegacyExternalStorage is ignored, so the old <external>/HTTrack/Websites root simply stops working; there is no flag that brings it back. The permission and its request go with it, being a no-op at 30 and auto-denied at 33. The engine is unaffected. Every path it sees is injected from Java (initRootPath, -O, buildTopIndex), so this is a Java-side choice; the one hardcoded outside-app path in the JNI glue is a dead pre-KitKat fallback. A persisted BasePath naming a public directory is now dropped rather than carried: mkdirs() there would merely return false and fall through to the default, which reads as the app forgetting its setting. The same check rejects one arriving from the file chooser. This does NOT migrate anything. A user with mirrors under the old public root keeps them on disk but loses sight of them; no legal read path exists in this configuration, since MediaStore does not index HTML. The import flow is a separate change, and the all-files build for sideload and F-Droid is another. FileProvider gains external-cache-path, without which browsing the help or licence pages throws "Failed to find configured root" as soon as the file:// reflection hack stops working at targetSdk 29. external-path goes, mapping storage we can no longer read, and files-path was never reachable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
Review of the first commit found the getFilesDir() fallback half-wired: it was added as a root, taught to isWritableProjectPath, then denied a FileProvider entry, so browsing a mirror on the internal fallback threw "Failed to find configured root". files-path comes back. The same check also conflated "outside our storage" with "cannot tell". An ejected volume hides getExternalFilesDir(), which made a perfectly good BasePath look foreign and erased it for good, a remount not being enough to bring it back. It now answers null when it cannot decide, and only a decided no drops the setting; an undecided one falls back for the run and keeps it. Since the answer decides whether a user's setting survives, the path math moves to StoragePaths, away from the Context lookups, where it can be tested off-device: sibling directories sharing a prefix, a neighbour package, .. traversal, a symlink out of the root, and the unmounted case. Dropping the separator from either side of the comparison makes files2 match files, and the suite says so. These are the repo's first tests, so the assemble job gains testDebugUnitTest and uploads the report. Also gone: a stale comment left stranded above the new method, and the sdcardHTTrackPath extra the chooser still parsed after this stopped sending it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scoped storage leaves exactly one external location writable without a permission, and this moves the mirror root to it. From API 30 on,
WRITE_EXTERNAL_STORAGEgrants nothing for shared storage andrequestLegacyExternalStorageis ignored, so the old<external>/HTTrack/Websitesroot simply stops working and no flag brings it back. The permission and its request go with it, being a no-op at 30 and auto-denied at 33.The engine is unaffected: every path it sees is injected from Java (
initRootPath,-O,buildTopIndex), so this is a Java-side choice. The one hardcoded outside-app path in the JNI glue is a dead pre-KitKat fallback.This migrates nothing, deliberately. A user with mirrors under the old public root keeps them on disk but loses sight of them. There is no legal read path in this configuration, since MediaStore does not index HTML and assets, so the import has to be user-mediated and it comes as a separate PR (a one-time
ACTION_OPEN_DOCUMENT_TREEpick of the old folder, copied viaDocumentFile). The all-files build for sideload and F-Droid is a third PR. Merging this one alone would ship the data loss without the remedy.A persisted
BasePathnaming a public directory is dropped rather than carried, sincemkdirs()there would merely return false and fall through to the default, which reads as the app quietly forgetting its setting. But only a decided no drops it: with the volume ejected our own external directory is unresolvable, and answering "outside" there would erase a good setting that a remount should have restored. Undecided falls back for the run and keeps the setting. The same check vets anything arriving from the file chooser.Because that answer decides whether a user's setting survives, the containment math lives in
StoragePaths, apart from the Context lookups, where it can be exercised off-device. These are the repo's first tests, so the assemble job gainstestDebugUnitTestand uploads the report. They cover sibling directories sharing a prefix (files2vsfiles), a neighbour package,..traversal, a symlink leaving the root, and both mount states. The separator on each side of the comparison is what stopsfiles2matchingfiles; drop either and the suite fails, which is checked by mutation rather than assumed.FileProviderroots now match what the app actually hands it:external-files-pathfor mirrors,files-pathfor the internal fallback taken while the volume is unmounted, andexternal-cache-pathfor the unpacked help and licence pages, which had no root at all.external-pathgoes, mapping storage we can no longer read. Any of the three missing throws "Failed to find configured root" as soon as thefile://hack stops working at targetSdk 29.Worth knowing for the target bump: recursive offline browsing dies at targetSdk 29 regardless of this PR. It survives today only because
allowUriSchemes()reflects intoStrictMode.disableDeathOnFileUriExposure(), which the greylist blocks from 29 on, leaving the FileProvider path that resolves the first HTML page and nothing it links to. That is a real user-visible loss, and it is not caused here.Known and left alone:
warnIfExternalStorageUnsuitable()still gates onEnvironment.getExternalStorageState(), which no longer determines where mirrors live, so on the internal fallback it warns about an "SDCARD" that is irrelevant. Non-blocking, and the fix wants string changes the localisation pipeline currently cannot deliver.Verified: the built APK no longer requests
WRITE_EXTERNAL_STORAGE, no reference to the deleted helpers survives, and the tests run (10 executed, 0 skipped). The path move itself needs a device, which is not available yet, so it rests on the API contract.