From 6f7eac8a262efa67cdc1aaf7b734994e6a4d94d6 Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Fri, 17 Jul 2026 14:29:28 +0200 Subject: [PATCH 1/4] app: write mirrors to the app-specific external directory 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 /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) Signed-off-by: Xavier Roche --- app/src/main/AndroidManifest.xml | 3 - .../com/httrack/android/HTTrackActivity.java | 119 +++++++----------- app/src/main/res/xml/provider_paths.xml | 8 +- 3 files changed, 47 insertions(+), 83 deletions(-) diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 53a8be9..966ceba 100755 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -2,9 +2,6 @@ - - - diff --git a/app/src/main/java/com/httrack/android/HTTrackActivity.java b/app/src/main/java/com/httrack/android/HTTrackActivity.java index a9e5c56..21388a5 100755 --- a/app/src/main/java/com/httrack/android/HTTrackActivity.java +++ b/app/src/main/java/com/httrack/android/HTTrackActivity.java @@ -218,49 +218,36 @@ protected static synchronized void clearRunningInstance(final File profile) { } /* Get the SDCard directory, or @c null upon error (ie. no sdcard). */ - private File getSDCardStorage() { - final String state = Environment.getExternalStorageState(); - if (Environment.MEDIA_MOUNTED.equals(state)) { - return Environment.getExternalStorageDirectory(); - } else { - return null; - } + /* + * The only place mirrors may live under scoped storage: our own external directory, which + * needs no permission at any target. The engine takes a POSIX path either way. + */ + private File getDefaultHTTrackPath() { + final File external = getExternalFilesDir(null); + // Null while the volume is unmounted; internal storage keeps the engine writable. + return new File(external != null ? external : getFilesDir(), "Websites"); } - /* Get the root storage: SDCard, ExternalStoragePublicDirectory, ExternalStorageDirectory, or fallback to FilesDir */ - private File getExternalStorage() { - final File f = getSDCardStorage(); - if (f != null) { - return f; - } else { - if (android.os.Build.VERSION.SDK_INT >= VERSION_CODES.FROYO) { - return Environment - .getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS); - } else { - // Fallback. - return getFilesDir(); + /* + * Guards the persisted BasePath, which may name a public directory from an older build: + * from API 30 on, mkdirs() there merely fails without saying why. + */ + private boolean isWritableProjectPath(final File path) { + final File external = getExternalFilesDir(null); + for (final File root : new File[] { external, getFilesDir() }) { + if (root == null) { + continue; + } + try { + final String prefix = root.getCanonicalPath() + File.separator; + if ((path.getCanonicalPath() + File.separator).startsWith(prefix)) { + return true; + } + } catch (final IOException e) { + Log.w(getClass().getSimpleName(), "could not canonicalize " + path, e); } } - } - - private File getHTTrackDirectoryFromBaseDirectory(final File rootPath) { - // Naming - final File httrackBasePath = new File(rootPath, "HTTrack"); - final File httrackPath = new File(httrackBasePath, "Websites"); - - return httrackPath; - } - - /* Get the default root external storage. */ - private File getDefaultHTTrackPath() { - return getHTTrackDirectoryFromBaseDirectory(getExternalStorage()); - } - - /* Get the alternate root external storage. */ - private File getSDCardHTTrackPath() { - final File rootPath = getSDCardStorage(); - - return rootPath != null ? getHTTrackDirectoryFromBaseDirectory(rootPath) : null; + return false; } /* @@ -269,7 +256,15 @@ private File getSDCardHTTrackPath() { private void computeStorageTarget() { final SharedPreferences settings = getSharedPreferences(PREFS_NAME, 0); final String base = settings.getString(BASE_NAME, null); - final File baseFile = base != null ? new File(base) : null; + File baseFile = base != null ? new File(base) : null; + + // Drop a base path we may no longer write to; the mirrors it names stay on disk, out of + // reach until imported. + if (baseFile != null && !isWritableProjectPath(baseFile)) { + Log.i(getClass().getSimpleName(), "dropping unusable base path " + base); + settings.edit().remove(BASE_NAME).apply(); + baseFile = null; + } if (baseFile != null && !baseFile.exists() && !baseFile.mkdirs()) { showNotification(getString(R.string.directory_does_not_exist) + ": " + base); @@ -298,6 +293,10 @@ private void computeStorageTarget() { * Set the base path. */ private void setBasePath(final String path) { + if (!isWritableProjectPath(new File(path))) { + showNotification(getString(R.string.directory_does_not_exist) + ": " + path); + return; + } final SharedPreferences settings = getSharedPreferences(PREFS_NAME, 0); final SharedPreferences.Editor editor = settings.edit(); editor.putString(BASE_NAME, path); @@ -405,43 +404,15 @@ protected void ensureInternetIsAvailable() { } } - /* - * Ensure "hard" permissions are requested properly from user. - * This is a new Android 6 required step, sheesh. - * See - * and especially szedjani's insightful reply. - */ - private static final int REQUEST_WRITE_STORAGE = 1; private static final int REQUEST_INTERNET = 2; - protected void ensureMediaIsAllowedHardPermissions() { - final boolean hasPermissionStorage = (ContextCompat.checkSelfPermission(this, - Manifest.permission.WRITE_EXTERNAL_STORAGE) == PackageManager.PERMISSION_GRANTED); - if (!hasPermissionStorage) { - Log.d("httrack", "Requesting access to storage"); - ActivityCompat.requestPermissions(this, - new String[]{Manifest.permission.WRITE_EXTERNAL_STORAGE}, - REQUEST_WRITE_STORAGE); - } else { - Log.d("httrack", "Access to storage is allowed"); - } - } - /* - * Callback to receive ensureMediaIsAllowedHardPermissions() permission feedback. + * Callback to receive permission feedback. */ @Override public void onRequestPermissionsResult(int requestCode, String[] permissions, int[] grantResults) { super.onRequestPermissionsResult(requestCode, permissions, grantResults); switch (requestCode) { - case REQUEST_WRITE_STORAGE: { - if (grantResults.length == 0 || grantResults[0] != PackageManager.PERMISSION_GRANTED) { - Toast.makeText(this, "The app was not allowed to write to your storage. Hence, it cannot function properly. Please consider granting it this permission", Toast.LENGTH_LONG).show(); - } else { - Log.d("httrack", "Permission to storage is granted"); - } - } - break; case REQUEST_INTERNET: { if (grantResults.length == 0 || grantResults[0] != PackageManager.PERMISSION_GRANTED) { Toast.makeText(this, "The app was not allowed to access the Internet. Hence, it cannot function properly. Please consider granting it this permission", Toast.LENGTH_LONG).show(); @@ -462,7 +433,6 @@ protected boolean ensureExternalStorage() { Log.d("httrack", "called ensureExternalStorage"); computeStorageTarget(); - ensureMediaIsAllowedHardPermissions(); ensureInternetIsAvailable(); final File root = getProjectRootFile(); @@ -2208,15 +2178,10 @@ public void onClickBasePath(final View view) { final Intent intent = new Intent(this, FileChooserActivity.class); fillExtra(intent); - final File defaultPath = getDefaultHTTrackPath(); + // Only our own directory is left to browse, so there is no second root to offer. intent.putExtra("com.httrack.android.defaultHTTrackPath", - defaultPath); + getDefaultHTTrackPath()); - final File f = getSDCardHTTrackPath(); - if (f != null && !f.equals(defaultPath)) { - intent.putExtra("com.httrack.android.sdcardHTTrackPath", - f); - } startActivityForResult(intent, ACTIVITY_FILE_CHOOSER); } diff --git a/app/src/main/res/xml/provider_paths.xml b/app/src/main/res/xml/provider_paths.xml index 0708c9a..dd72ff9 100644 --- a/app/src/main/res/xml/provider_paths.xml +++ b/app/src/main/res/xml/provider_paths.xml @@ -1,6 +1,8 @@ - + - - \ No newline at end of file + + + From ac06df956117e73c951ab16a163c0f2291f2b9a7 Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Fri, 17 Jul 2026 14:43:28 +0200 Subject: [PATCH 2/4] app: separate the containment math out, and cover it with tests 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) Signed-off-by: Xavier Roche --- .github/workflows/android.yml | 10 +- app/build.gradle | 2 + .../httrack/android/FileChooserActivity.java | 3 - .../com/httrack/android/HTTrackActivity.java | 43 ++++----- .../com/httrack/android/StoragePaths.java | 43 +++++++++ app/src/main/res/xml/provider_paths.xml | 2 + .../com/httrack/android/StoragePathsTest.java | 94 +++++++++++++++++++ 7 files changed, 167 insertions(+), 30 deletions(-) create mode 100644 app/src/main/java/com/httrack/android/StoragePaths.java create mode 100644 app/src/test/java/com/httrack/android/StoragePathsTest.java diff --git a/.github/workflows/android.yml b/.github/workflows/android.yml index 5b32ef7..21f60b8 100644 --- a/.github/workflows/android.yml +++ b/.github/workflows/android.yml @@ -82,8 +82,14 @@ jobs: run: tools/ci/vendor-libiconv.sh - name: Stage OpenSSL statics into prebuild/ run: tools/ci/fetch-openssl-statics.sh # picks up $OPENSSL_ANDROID_ROOT from the image - - name: Gradle assemble + lint - run: ./gradlew --no-daemon assembleDebug assembleRelease lint + - name: Gradle assemble + test + lint + run: ./gradlew --no-daemon assembleDebug assembleRelease testDebugUnitTest lint + - uses: actions/upload-artifact@v4 + if: always() # the report is what says which case broke + with: + name: test-results + path: app/build/reports/tests/** + if-no-files-found: warn - uses: actions/upload-artifact@v4 with: name: apk diff --git a/app/build.gradle b/app/build.gradle index 8b33630..1e6adf2 100755 --- a/app/build.gradle +++ b/app/build.gradle @@ -58,4 +58,6 @@ dependencies { implementation 'androidx.core:core:1.13.1' implementation 'androidx.fragment:fragment:1.8.2' + + testImplementation 'junit:junit:4.13.2' } diff --git a/app/src/main/java/com/httrack/android/FileChooserActivity.java b/app/src/main/java/com/httrack/android/FileChooserActivity.java index a1bad4e..2583289 100755 --- a/app/src/main/java/com/httrack/android/FileChooserActivity.java +++ b/app/src/main/java/com/httrack/android/FileChooserActivity.java @@ -27,7 +27,6 @@ public class FileChooserActivity extends Activity implements protected File projectRootFile; protected File defaultRootFile; - protected File sdcardRootFile; protected final List> files = new ArrayList>(); @@ -144,8 +143,6 @@ protected void onCreate(final Bundle savedInstanceState) { .get("com.httrack.android.rootFile")); defaultRootFile = File.class.cast(extras .get("com.httrack.android.defaultHTTrackPath")); - sdcardRootFile = File.class.cast(extras - .get("com.httrack.android.sdcardHTTrackPath")); if (projectRootFile == null || defaultRootFile == null) { throw new RuntimeException("internal error"); } diff --git a/app/src/main/java/com/httrack/android/HTTrackActivity.java b/app/src/main/java/com/httrack/android/HTTrackActivity.java index 21388a5..9f65a85 100755 --- a/app/src/main/java/com/httrack/android/HTTrackActivity.java +++ b/app/src/main/java/com/httrack/android/HTTrackActivity.java @@ -217,7 +217,6 @@ protected static synchronized void clearRunningInstance(final File profile) { } } - /* Get the SDCard directory, or @c null upon error (ie. no sdcard). */ /* * The only place mirrors may live under scoped storage: our own external directory, which * needs no permission at any target. The engine takes a POSIX path either way. @@ -230,24 +229,11 @@ private File getDefaultHTTrackPath() { /* * Guards the persisted BasePath, which may name a public directory from an older build: - * from API 30 on, mkdirs() there merely fails without saying why. + * from API 30 on, mkdirs() there merely fails without saying why. Null means undecided, + * never refused; see StoragePaths.isWritable. */ - private boolean isWritableProjectPath(final File path) { - final File external = getExternalFilesDir(null); - for (final File root : new File[] { external, getFilesDir() }) { - if (root == null) { - continue; - } - try { - final String prefix = root.getCanonicalPath() + File.separator; - if ((path.getCanonicalPath() + File.separator).startsWith(prefix)) { - return true; - } - } catch (final IOException e) { - Log.w(getClass().getSimpleName(), "could not canonicalize " + path, e); - } - } - return false; + private Boolean isWritableProjectPath(final File path) { + return StoragePaths.isWritable(path, getExternalFilesDir(null), getFilesDir()); } /* @@ -258,12 +244,18 @@ private void computeStorageTarget() { final String base = settings.getString(BASE_NAME, null); File baseFile = base != null ? new File(base) : null; - // Drop a base path we may no longer write to; the mirrors it names stay on disk, out of - // reach until imported. - if (baseFile != null && !isWritableProjectPath(baseFile)) { - Log.i(getClass().getSimpleName(), "dropping unusable base path " + base); - settings.edit().remove(BASE_NAME).apply(); - baseFile = null; + if (baseFile != null) { + final Boolean writable = isWritableProjectPath(baseFile); + if (Boolean.FALSE.equals(writable)) { + // Known foreign: the mirrors it names stay on disk, out of reach until imported. + Log.i(getClass().getSimpleName(), "dropping unusable base path " + base); + settings.edit().remove(BASE_NAME).apply(); + baseFile = null; + } else if (writable == null) { + // Undecided: fall back for this run, but keep the setting for when the volume returns. + Log.i(getClass().getSimpleName(), "cannot vet base path yet: " + base); + baseFile = null; + } } if (baseFile != null && !baseFile.exists() && !baseFile.mkdirs()) { @@ -293,7 +285,8 @@ private void computeStorageTarget() { * Set the base path. */ private void setBasePath(final String path) { - if (!isWritableProjectPath(new File(path))) { + // Only a decided yes: an unvettable path must not be persisted. + if (!Boolean.TRUE.equals(isWritableProjectPath(new File(path)))) { showNotification(getString(R.string.directory_does_not_exist) + ": " + path); return; } diff --git a/app/src/main/java/com/httrack/android/StoragePaths.java b/app/src/main/java/com/httrack/android/StoragePaths.java new file mode 100644 index 0000000..fc24f5a --- /dev/null +++ b/app/src/main/java/com/httrack/android/StoragePaths.java @@ -0,0 +1,43 @@ +package com.httrack.android; + +import java.io.File; +import java.io.IOException; + +/** + * Where mirrors may be written. Pure path math, kept apart from the Context lookups so that it + * can be exercised off-device: this is the boundary that decides whether a persisted setting is + * honoured or destroyed. + */ +final class StoragePaths { + private StoragePaths() { + } + + /** + * Whether {@code path} lies inside one of our own storage roots, either of which may be null. + * + * @param path + * the candidate, need not exist + * @param external + * getExternalFilesDir(null), null while the volume is unmounted + * @param internal + * getFilesDir(), the fallback root + * @return true inside a root, false provably outside, and null when the question cannot be + * answered: with the external root unresolved there is nothing to compare against, and + * a caller that reads that as a refusal would discard a setting it cannot vet. + */ + static Boolean isWritable(final File path, final File external, final File internal) { + try { + // The separator on both sides is what keeps a sibling like "files2" from matching "files". + final String target = path.getCanonicalPath() + File.separator; + for (final File root : new File[] { external, internal }) { + if (root != null + && target.startsWith(root.getCanonicalPath() + File.separator)) { + return true; + } + } + } catch (final IOException e) { + return null; + } + return external != null ? Boolean.FALSE : null; + } +} diff --git a/app/src/main/res/xml/provider_paths.xml b/app/src/main/res/xml/provider_paths.xml index dd72ff9..30f751f 100644 --- a/app/src/main/res/xml/provider_paths.xml +++ b/app/src/main/res/xml/provider_paths.xml @@ -2,6 +2,8 @@ + + diff --git a/app/src/test/java/com/httrack/android/StoragePathsTest.java b/app/src/test/java/com/httrack/android/StoragePathsTest.java new file mode 100644 index 0000000..3bedbd7 --- /dev/null +++ b/app/src/test/java/com/httrack/android/StoragePathsTest.java @@ -0,0 +1,94 @@ +package com.httrack.android; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.nio.file.Files; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +/** Attacks StoragePaths.isWritable: what it must accept, what it must refuse, what it cannot tell. */ +public class StoragePathsTest { + @Rule + public final TemporaryFolder tmp = new TemporaryFolder(); + + private File external; + private File internal; + + @Before + public void setUp() throws Exception { + external = tmp.newFolder("ext", "Android", "data", "com.httrack.android", "files"); + internal = tmp.newFolder("int", "data", "com.httrack.android", "files"); + } + + @Test + public void acceptsOurOwnRoots() { + assertEquals(Boolean.TRUE, StoragePaths.isWritable(new File(external, "Websites"), external, internal)); + assertEquals(Boolean.TRUE, StoragePaths.isWritable(new File(internal, "Websites"), external, internal)); + } + + @Test + public void acceptsTheRootItself() { + assertEquals(Boolean.TRUE, StoragePaths.isWritable(external, external, internal)); + } + + @Test + public void refusesTheOldPublicRoot() { + final File old = new File(tmp.getRoot(), "ext/HTTrack/Websites"); + assertEquals(Boolean.FALSE, StoragePaths.isWritable(old, external, internal)); + assertEquals(Boolean.FALSE, StoragePaths.isWritable(new File("/"), external, internal)); + } + + /** "files2" shares the string prefix of "files" without being inside it. */ + @Test + public void refusesASiblingSharingThePrefix() throws Exception { + final File sibling = tmp.newFolder("ext", "Android", "data", "com.httrack.android", "files2"); + assertEquals(Boolean.FALSE, StoragePaths.isWritable(new File(sibling, "Websites"), external, internal)); + } + + @Test + public void refusesANeighbourPackage() throws Exception { + final File other = tmp.newFolder("ext", "Android", "data", "com.httrack.android.evil", "files"); + assertEquals(Boolean.FALSE, StoragePaths.isWritable(other, external, internal)); + } + + @Test + public void refusesTraversalOutOfTheRoot() { + assertEquals(Boolean.FALSE, + StoragePaths.isWritable(new File(external, "../../../../HTTrack"), external, internal)); + } + + @Test + public void refusesASymlinkLeavingTheRoot() throws Exception { + final File outside = tmp.newFolder("outside"); + final File link = new File(external, "escape"); + Files.createSymbolicLink(link.toPath(), outside.toPath()); + assertEquals(Boolean.FALSE, StoragePaths.isWritable(link, external, internal)); + } + + /** + * The volume is gone, so an external path cannot be vetted. Answering "false" here would make + * the caller erase a good setting that a remount would have restored. + */ + @Test + public void cannotDecideWithoutTheExternalRoot() { + assertNull(StoragePaths.isWritable(new File(external, "Websites"), null, internal)); + } + + @Test + public void stillDecidesInternalPathsWithoutTheExternalRoot() { + assertEquals(Boolean.TRUE, StoragePaths.isWritable(new File(internal, "Websites"), null, internal)); + } + + /** Whatever the mount state, the default root the app picks must satisfy its own check. */ + @Test + public void theDefaultRootAlwaysPassesItsOwnCheck() { + assertTrue(StoragePaths.isWritable(new File(external, "Websites"), external, internal)); + assertTrue(StoragePaths.isWritable(new File(internal, "Websites"), null, internal)); + } +} From 4b410896a675bb44d20bc9c39b6db3f4f5691bce Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Sat, 18 Jul 2026 07:34:38 +0200 Subject: [PATCH 3/4] app: import mirrors from a legacy folder through the storage framework The move to app-specific storage put the old public HTTrack/Websites tree out of reach, so this lets the user hand it back: a menu entry and a one-time first-launch offer open the storage-access picker, and the chosen folder is copied in. It needs no permission, which is why the framework picker is the route rather than direct file access. The copy never touches the source, so an interrupted or partial run loses nothing and can be repeated; existing destination files are skipped, and each file lands via a .part rename so a half-written one is never mistaken for complete. A picked "HTTrack" folder is descended into its "Websites" child so project directories keep their depth. The copy walk is written against a small Source interface with a DocumentFile adapter, so its fidelity, idempotency, and per-file error tolerance are unit tested off-device (the first tests this repo has for this path). The picker, the threading, and the dialogs still want a device to confirm end to end. Stacked on the app-specific-storage change; the offer is what makes shipping that safe for users with existing mirrors. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Xavier Roche --- app/build.gradle | 1 + .../com/httrack/android/HTTrackActivity.java | 107 ++++++++++ .../httrack/android/LegacyMirrorImport.java | 184 ++++++++++++++++++ app/src/main/res/menu/menu.xml | 4 + app/src/main/res/values/strings.xml | 10 + .../android/LegacyMirrorImportTest.java | 181 +++++++++++++++++ 6 files changed, 487 insertions(+) create mode 100644 app/src/main/java/com/httrack/android/LegacyMirrorImport.java create mode 100644 app/src/test/java/com/httrack/android/LegacyMirrorImportTest.java diff --git a/app/build.gradle b/app/build.gradle index 1e6adf2..df91b42 100755 --- a/app/build.gradle +++ b/app/build.gradle @@ -58,6 +58,7 @@ dependencies { implementation 'androidx.core:core:1.13.1' implementation 'androidx.fragment:fragment:1.8.2' + implementation 'androidx.documentfile:documentfile:1.0.1' testImplementation 'junit:junit:4.13.2' } diff --git a/app/src/main/java/com/httrack/android/HTTrackActivity.java b/app/src/main/java/com/httrack/android/HTTrackActivity.java index d5b51bc..270a1ec 100755 --- a/app/src/main/java/com/httrack/android/HTTrackActivity.java +++ b/app/src/main/java/com/httrack/android/HTTrackActivity.java @@ -53,6 +53,7 @@ import android.app.NotificationManager; import android.app.PendingIntent; import android.content.ActivityNotFoundException; +import android.content.ContentResolver; import android.content.Context; import android.content.Intent; import android.content.SharedPreferences; @@ -90,6 +91,7 @@ import android.widget.Toast; import androidx.core.app.ActivityCompat; +import androidx.documentfile.provider.DocumentFile; import androidx.core.app.NotificationChannelCompat; import androidx.core.app.NotificationCompat; import androidx.core.app.NotificationManagerCompat; @@ -123,6 +125,8 @@ public class HTTrackActivity extends FragmentActivity { // Whether POST_NOTIFICATIONS was ever asked for. Has to outlive the activity: pane_id is // restored on rotation, and a second refusal is the one that sticks for good. protected static final String NOTIFY_ASKED_NAME = "NotificationPermissionAsked"; + // Whether the one-time "import your old mirrors" offer has been shown and dismissed for good. + protected static final String IMPORT_OFFERED_NAME = "LegacyImportOffered"; //
Pattern protected static final Pattern brHtmlPattern = Pattern.compile(Pattern @@ -160,6 +164,7 @@ protected static class VERSION_CODES { protected static final int ACTIVITY_FILE_CHOOSER = 1; protected static final int ACTIVITY_PROJECT_NAME_CHOOSER = 2; protected static final int ACTIVITY_CLEANUP = 3; + protected static final int ACTIVITY_IMPORT_TREE = 4; // Process unique session ID for the fragment identifier protected String sessionID = "runner_task" + "_" @@ -548,6 +553,11 @@ protected void onCreate(final Bundle savedInstanceState) { if (extras != null) { restoreInstanceState(extras); } + + // First launch only, so a rotation does not bring the offer back. + if (savedInstanceState == null) { + offerLegacyMirrorImportOnce(); + } } /* Install/Update time. */ @@ -2211,6 +2221,95 @@ public void onClickPrevious(final View view) { } } + /** + * Let the user pick a legacy mirror folder to copy in. The storage-access framework is the + * only way left to reach the old public location once we no longer hold storage permissions. + */ + private void startLegacyMirrorImport() { + final Intent intent = new Intent(Intent.ACTION_OPEN_DOCUMENT_TREE); + intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION); + try { + showNotification(getString(R.string.import_mirrors_prompt)); + startActivityForResult(intent, ACTIVITY_IMPORT_TREE); + } catch (final ActivityNotFoundException e) { + Log.w(getClass().getSimpleName(), "no document-tree picker", e); + showNotification(getString(R.string.import_mirrors_none)); + } + } + + /** + * Copy the picked tree into our Websites directory, off the UI thread. If the pick holds a + * "Websites" subfolder (the user chose the HTTrack folder), descend into it so project + * directories do not end up one level too deep. The source is only read, never changed. + */ + private void importMirrorsFrom(final Uri treeUri) { + final ContentResolver resolver = getContentResolver(); + resolver.takePersistableUriPermission(treeUri, Intent.FLAG_GRANT_READ_URI_PERMISSION); + DocumentFile root = DocumentFile.fromTreeUri(this, treeUri); + if (root == null) { + showNotification(getString(R.string.import_mirrors_none)); + return; + } + final DocumentFile websites = root.findFile("Websites"); + if (websites != null && websites.isDirectory()) { + root = websites; + } + final LegacyMirrorImport.Source source = + new LegacyMirrorImport.DocumentFileSource(resolver, root); + final File dest = getProjectRootFile(); + showNotification(getString(R.string.import_mirrors_running)); + new Thread(new Runnable() { + @Override + public void run() { + final LegacyMirrorImport.Result result = LegacyMirrorImport.copyTree(source, dest); + runOnUiThread(new Runnable() { + @Override + public void run() { + onImportFinished(result); + } + }); + } + }, "legacy-import").start(); + } + + /** Report the outcome and surface any freshly imported projects. **/ + private void onImportFinished(final LegacyMirrorImport.Result result) { + if (result.firstError() != null) { + Log.w(getClass().getSimpleName(), "import: " + result.firstError()); + } + refreshprojectNameSuggests(); + final String message; + if (!result.isComplete()) { + message = getString(R.string.import_mirrors_partial, result.copied, result.failed); + } else if (result.copied == 0 && result.skipped == 0) { + message = getString(R.string.import_mirrors_none); + } else { + message = getString(R.string.import_mirrors_done, result.copied, result.skipped); + } + showNotification(message); + } + + /** + * Once per install, point users at the import. No storage permission remains to detect the + * old folder, so this offers rather than asserts; "Not now" leaves it to ask again. + */ + private void offerLegacyMirrorImportOnce() { + final SharedPreferences settings = getSharedPreferences(PREFS_NAME, 0); + if (settings.getBoolean(IMPORT_OFFERED_NAME, false)) { + return; + } + new AlertDialog.Builder(this) + .setMessage(R.string.import_mirrors_offer) + .setPositiveButton(R.string.import_mirrors_offer_yes, (dialog, which) -> { + settings.edit().putBoolean(IMPORT_OFFERED_NAME, true).apply(); + startLegacyMirrorImport(); + }) + .setNegativeButton(R.string.import_mirrors_offer_later, null) + .setNeutralButton(R.string.import_mirrors_offer_never, (dialog, which) -> + settings.edit().putBoolean(IMPORT_OFFERED_NAME, true).apply()) + .show(); + } + /** * Change base path */ @@ -2307,6 +2406,11 @@ protected void onActivityResult(final int requestCode, final int resultCode, setBasePath(path); } break; + case ACTIVITY_IMPORT_TREE: + if (resultCode == Activity.RESULT_OK && data != null && data.getData() != null) { + importMirrorsFrom(data.getData()); + } + break; case ACTIVITY_PROJECT_NAME_CHOOSER: if (resultCode == Activity.RESULT_OK) { final String projectName = data @@ -2566,6 +2670,9 @@ public boolean onOptionsItemSelected(final MenuItem item) { case R.id.action_help: browse(new File(new File(getResourceFile(), "html"), "index.html")); break; + case R.id.action_import_mirrors: + startLegacyMirrorImport(); + break; default: return super.onOptionsItemSelected(item); } diff --git a/app/src/main/java/com/httrack/android/LegacyMirrorImport.java b/app/src/main/java/com/httrack/android/LegacyMirrorImport.java new file mode 100644 index 0000000..fc9c683 --- /dev/null +++ b/app/src/main/java/com/httrack/android/LegacyMirrorImport.java @@ -0,0 +1,184 @@ +package com.httrack.android; + +import android.content.ContentResolver; +import android.net.Uri; + +import androidx.documentfile.provider.DocumentFile; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.ArrayList; +import java.util.List; + +/** + * One-time copy of mirrors from a user-picked legacy location into the app-specific directory. + * + * Scoped storage put the old public HTTrack/Websites tree out of reach, so the user grants it + * once through the storage-access framework and its contents are copied here. The copy never + * touches the source, so a failed or interrupted run loses nothing and can simply be re-run: + * existing destination files are skipped, which also makes it idempotent. + */ +final class LegacyMirrorImport { + private static final int BUFFER = 64 * 1024; + + private LegacyMirrorImport() { + } + + /** A node of the tree being copied, so the walk can be tested without a real DocumentFile. */ + interface Source { + String getName(); + + boolean isDirectory(); + + long length(); + + List children(); + + InputStream openStream() throws IOException; + } + + // Cap the retained messages: a broken tree could otherwise produce one per file. + private static final int MAX_ERRORS = 20; + + /** Tally of a copy run; a per-file failure is recorded, not thrown, so the rest still copies. */ + static final class Result { + int copied; + int skipped; + int failed; + final List errors = new ArrayList(); + + boolean isComplete() { + return failed == 0; + } + + String firstError() { + return errors.isEmpty() ? null : errors.get(0); + } + } + + /** + * Copy {@code src}'s contents into {@code destDir}, recursively. Files already present at the + * destination are left as they are. Never modifies the source. + * + * @return the tally; {@link Result#isComplete()} is false if any file could not be copied + */ + static Result copyTree(final Source src, final File destDir) { + final Result result = new Result(); + copyInto(src, destDir, result); + return result; + } + + private static void copyInto(final Source dir, final File destDir, final Result result) { + if (!destDir.exists() && !destDir.mkdirs()) { + record(result, "could not create " + destDir); + return; + } + for (final Source child : dir.children()) { + final String name = child.getName(); + if (name == null || name.isEmpty() || name.equals(".") || name.equals("..") + || name.indexOf('/') >= 0) { + record(result, "unsafe entry name '" + name + "'"); + continue; + } + final File dest = new File(destDir, name); + if (child.isDirectory()) { + copyInto(child, dest, result); + } else if (dest.exists()) { + result.skipped++; + } else { + copyFile(child, dest, result); + } + } + } + + private static void copyFile(final Source src, final File dest, final Result result) { + // A partial file from an interrupted run must not look complete on the next pass; write to a + // temp sibling and rename only once whole, so only finished files ever satisfy dest.exists(). + final File tmp = new File(dest.getParentFile(), dest.getName() + ".part"); + try (final InputStream in = src.openStream(); + final OutputStream out = new FileOutputStream(tmp)) { + final byte[] buffer = new byte[BUFFER]; + int n; + while ((n = in.read(buffer)) != -1) { + out.write(buffer, 0, n); + } + } catch (final IOException e) { + tmp.delete(); + record(result, "could not copy " + src.getName() + ": " + e.getMessage()); + return; + } + if (tmp.renameTo(dest)) { + result.copied++; + } else { + tmp.delete(); + record(result, "could not finalize " + dest); + } + } + + private static void record(final Result result, final String message) { + result.failed++; + if (result.errors.size() < MAX_ERRORS) { + result.errors.add(message); + } + } + + /** Bytes the tree holds, for a free-space check before starting. Skipped files still count. */ + static long totalSize(final Source src) { + if (!src.isDirectory()) { + return src.length(); + } + long total = 0; + for (final Source child : src.children()) { + total += totalSize(child); + } + return total; + } + + /** A DocumentFile-backed source; the adapter the app uses over a picked tree. */ + static final class DocumentFileSource implements Source { + private final ContentResolver resolver; + private final DocumentFile file; + + DocumentFileSource(final ContentResolver resolver, final DocumentFile file) { + this.resolver = resolver; + this.file = file; + } + + @Override + public String getName() { + return file.getName(); + } + + @Override + public boolean isDirectory() { + return file.isDirectory(); + } + + @Override + public long length() { + return file.length(); + } + + @Override + public List children() { + final List out = new ArrayList(); + for (final DocumentFile child : file.listFiles()) { + out.add(new DocumentFileSource(resolver, child)); + } + return out; + } + + @Override + public InputStream openStream() throws IOException { + final Uri uri = file.getUri(); + final InputStream in = resolver.openInputStream(uri); + if (in == null) { + throw new IOException("no stream for " + uri); + } + return in; + } + } +} diff --git a/app/src/main/res/menu/menu.xml b/app/src/main/res/menu/menu.xml index f5a7ca6..593d0e0 100644 --- a/app/src/main/res/menu/menu.xml +++ b/app/src/main/res/menu/menu.xml @@ -4,6 +4,10 @@ android:id="@+id/action_help" android:orderInCategory="100" android:title="@string/action_help"/> + Help View Documentation About HTTrack + Import existing mirrors + Pick the folder that holds your older HTTrack mirrors (for example HTTrack on the internal storage). Their contents will be copied into this app\'s storage; the originals are left untouched. + Importing mirrors… + Imported %1$d files (%2$d already present). + Imported %1$d files; %2$d could not be copied. You can run the import again. + Nothing to import from that folder. + Mirrors from an older version can no longer be reached directly. Import them into this app now? + Import + Not now + Don\'t ask again Go To HTTrack Website Go To HTTrack Forum View License diff --git a/app/src/test/java/com/httrack/android/LegacyMirrorImportTest.java b/app/src/test/java/com/httrack/android/LegacyMirrorImportTest.java new file mode 100644 index 0000000..24217f0 --- /dev/null +++ b/app/src/test/java/com/httrack/android/LegacyMirrorImportTest.java @@ -0,0 +1,181 @@ +package com.httrack.android; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.List; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +/** Exercises the copy walk over an in-memory tree: fidelity, idempotency, and error tolerance. */ +public class LegacyMirrorImportTest { + @Rule + public final TemporaryFolder tmp = new TemporaryFolder(); + + /** In-memory Source: a directory holds children; a file holds bytes, or throws if poisoned. */ + private static final class FakeNode implements LegacyMirrorImport.Source { + final String name; + final boolean dir; + final byte[] data; + final boolean poisoned; + final List kids = new ArrayList(); + + static FakeNode dir(final String name) { + return new FakeNode(name, true, null, false); + } + + static FakeNode file(final String name, final String body) { + return new FakeNode(name, false, body.getBytes(StandardCharsets.UTF_8), false); + } + + static FakeNode poison(final String name) { + return new FakeNode(name, false, null, true); + } + + private FakeNode(final String name, final boolean dir, final byte[] data, final boolean poisoned) { + this.name = name; + this.dir = dir; + this.data = data; + this.poisoned = poisoned; + } + + FakeNode with(final LegacyMirrorImport.Source... children) { + for (final LegacyMirrorImport.Source c : children) { + kids.add(c); + } + return this; + } + + @Override + public String getName() { + return name; + } + + @Override + public boolean isDirectory() { + return dir; + } + + @Override + public long length() { + return data != null ? data.length : 0; + } + + @Override + public List children() { + return kids; + } + + @Override + public InputStream openStream() throws IOException { + if (poisoned) { + throw new IOException("poisoned " + name); + } + return new ByteArrayInputStream(data); + } + } + + private String read(final File f) throws IOException { + return new String(Files.readAllBytes(f.toPath()), StandardCharsets.UTF_8); + } + + @Test + public void copiesTheWholeTreeFaithfully() throws Exception { + final FakeNode root = FakeNode.dir("HTTrack").with( + FakeNode.file("index.html", "top"), + FakeNode.dir("site").with( + FakeNode.file("page.html", "body"), + FakeNode.dir("img").with(FakeNode.file("a.png", "PNGDATA")))); + final File dest = tmp.newFolder("dest"); + + final LegacyMirrorImport.Result r = LegacyMirrorImport.copyTree(root, dest); + + assertTrue(r.isComplete()); + assertEquals(3, r.copied); + assertEquals(0, r.skipped); + assertEquals("top", read(new File(dest, "index.html"))); + assertEquals("body", read(new File(dest, "site/page.html"))); + assertEquals("PNGDATA", read(new File(dest, "site/img/a.png"))); + } + + @Test + public void reRunSkipsWhatIsAlreadyThere() throws Exception { + final FakeNode root = FakeNode.dir("HTTrack").with(FakeNode.file("index.html", "first")); + final File dest = tmp.newFolder("dest"); + + LegacyMirrorImport.copyTree(root, dest); + final LegacyMirrorImport.Result second = LegacyMirrorImport.copyTree(root, dest); + + assertEquals(0, second.copied); + assertEquals(1, second.skipped); + assertEquals("first", read(new File(dest, "index.html"))); + } + + @Test + public void oneUnreadableFileDoesNotAbortTheRest() throws Exception { + final FakeNode root = FakeNode.dir("HTTrack").with( + FakeNode.file("good1.html", "ok"), + FakeNode.poison("bad.html"), + FakeNode.file("good2.html", "ok")); + final File dest = tmp.newFolder("dest"); + + final LegacyMirrorImport.Result r = LegacyMirrorImport.copyTree(root, dest); + + assertFalse(r.isComplete()); + assertEquals(2, r.copied); + assertEquals(1, r.failed); + assertTrue(new File(dest, "good1.html").exists()); + assertTrue(new File(dest, "good2.html").exists()); + // The poisoned file leaves nothing half-written that a re-run would mistake for complete. + assertFalse(new File(dest, "bad.html").exists()); + assertFalse(new File(dest, "bad.html.part").exists()); + } + + @Test + public void refusesAnEntryThatWouldEscapeTheDestination() throws Exception { + final FakeNode root = FakeNode.dir("HTTrack").with( + FakeNode.file("../escape.html", "evil"), + FakeNode.file("ok.html", "ok")); + final File dest = tmp.newFolder("dest"); + + final LegacyMirrorImport.Result r = LegacyMirrorImport.copyTree(root, dest); + + assertEquals(1, r.copied); + assertEquals(1, r.failed); + assertFalse(new File(dest.getParentFile(), "escape.html").exists()); + } + + @Test + public void totalSizeSumsEveryFile() { + final FakeNode root = FakeNode.dir("HTTrack").with( + FakeNode.file("a", "12345"), + FakeNode.dir("d").with(FakeNode.file("b", "123"))); + assertEquals(8, LegacyMirrorImport.totalSize(root)); + } + + @Test + public void aByteForByteCopyMatchesBinaryContent() throws Exception { + final byte[] raw = new byte[1000]; + for (int i = 0; i < raw.length; i++) { + raw[i] = (byte) (i * 7); + } + final FakeNode root = FakeNode.dir("HTTrack").with( + new FakeNode("blob.bin", false, raw, false)); + final File dest = tmp.newFolder("dest"); + + LegacyMirrorImport.copyTree(root, dest); + + assertArrayEquals(raw, Files.readAllBytes(new File(dest, "blob.bin").toPath())); + } +} From c3babb6634ce4b1d3f009d2a5604a67c1a63db0c Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Sat, 18 Jul 2026 07:50:48 +0200 Subject: [PATCH 4/4] app: harden the mirror import after review Adversarial review found real gaps, and two of my own tests that could not fail. Addressed: - A second import while one runs started a second thread onto the same .part files, racing to corrupt them. An in-flight guard turns the re-entry into a no-op with the "importing" notice. - fromTreeUri and the findFile("Websites") walk ran on the UI thread, one IPC per entry: an ANR waiting to happen on a large tree. All of it moves to the worker now. - The result toast was posted to the activity that launched the pick, so a rotation during a long copy swallowed it. It now posts through a main-thread handler with the application context, and only refreshes the project list when an activity is still there to hold it. - The free-space check the helper carried totalSize() for was never wired; it now runs before the copy, so a tree that would not fit is refused up front rather than filling storage. - A source file whose name already exists as a destination directory was counted as "skipped", silently dropping it; it is now reported. - The first-launch offer no longer stacks on top of the native-load failure dialog. Tests: the .part atomicity and the entry-name guard were both covered by tests that passed whether or not the mechanism was present. Replaced with ones that fail under the corresponding mutation -- a copy that writes straight to the final name is caught by observing the file mid-write, and a guard shrunk to only reject slashes is caught by a null entry name (which DocumentFile.getName may return) crashing the walk. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Xavier Roche --- .../com/httrack/android/HTTrackActivity.java | 96 ++++++---- .../httrack/android/LegacyMirrorImport.java | 7 +- app/src/main/res/values/strings.xml | 1 + .../android/LegacyMirrorImportTest.java | 177 ++++++++++++++++++ 4 files changed, 247 insertions(+), 34 deletions(-) diff --git a/app/src/main/java/com/httrack/android/HTTrackActivity.java b/app/src/main/java/com/httrack/android/HTTrackActivity.java index 270a1ec..7663274 100755 --- a/app/src/main/java/com/httrack/android/HTTrackActivity.java +++ b/app/src/main/java/com/httrack/android/HTTrackActivity.java @@ -67,6 +67,7 @@ import android.os.Bundle; import android.os.Environment; import android.os.Handler; +import android.os.Looper; import android.os.Parcelable; import android.os.StrictMode; import android.text.Editable; @@ -554,8 +555,9 @@ protected void onCreate(final Bundle savedInstanceState) { restoreInstanceState(extras); } - // First launch only, so a rotation does not bring the offer back. - if (savedInstanceState == null) { + // First launch only, so a rotation does not bring the offer back; and not stacked on top + // of the native-load failure dialog. + if (savedInstanceState == null && HTTrackLib.loadedSuccessfully()) { offerLegacyMirrorImportOnce(); } } @@ -2237,56 +2239,84 @@ private void startLegacyMirrorImport() { } } + private volatile boolean importInProgress; + /** - * Copy the picked tree into our Websites directory, off the UI thread. If the pick holds a - * "Websites" subfolder (the user chose the HTTrack folder), descend into it so project - * directories do not end up one level too deep. The source is only read, never changed. + * Copy the picked tree into our Websites directory, entirely off the UI thread: even walking + * the tree to find "Websites" is one IPC per entry, too much for the main thread. The source + * is only read, never changed. Guarded against a second run while one is going, whose two + * threads would race on the same temp files. */ private void importMirrorsFrom(final Uri treeUri) { - final ContentResolver resolver = getContentResolver(); - resolver.takePersistableUriPermission(treeUri, Intent.FLAG_GRANT_READ_URI_PERMISSION); - DocumentFile root = DocumentFile.fromTreeUri(this, treeUri); - if (root == null) { - showNotification(getString(R.string.import_mirrors_none)); + if (importInProgress) { + showNotification(getString(R.string.import_mirrors_running)); return; } - final DocumentFile websites = root.findFile("Websites"); - if (websites != null && websites.isDirectory()) { - root = websites; - } - final LegacyMirrorImport.Source source = - new LegacyMirrorImport.DocumentFileSource(resolver, root); + final ContentResolver resolver = getContentResolver(); + resolver.takePersistableUriPermission(treeUri, Intent.FLAG_GRANT_READ_URI_PERMISSION); + final Context appContext = getApplicationContext(); final File dest = getProjectRootFile(); + importInProgress = true; showNotification(getString(R.string.import_mirrors_running)); new Thread(new Runnable() { @Override public void run() { - final LegacyMirrorImport.Result result = LegacyMirrorImport.copyTree(source, dest); - runOnUiThread(new Runnable() { - @Override - public void run() { - onImportFinished(result); - } - }); + String outcome; + try { + outcome = runImport(appContext, resolver, treeUri, dest); + } finally { + importInProgress = false; + } + deliverImportOutcome(appContext, outcome); } }, "legacy-import").start(); } - /** Report the outcome and surface any freshly imported projects. **/ - private void onImportFinished(final LegacyMirrorImport.Result result) { + /** The copy itself, on the worker thread; returns the message to show. **/ + private String runImport(final Context context, final ContentResolver resolver, + final Uri treeUri, final File dest) { + DocumentFile root = DocumentFile.fromTreeUri(context, treeUri); + if (root == null) { + return getString(R.string.import_mirrors_none); + } + // A picked "HTTrack" folder holds "Websites"; descend so project dirs keep their depth. + final DocumentFile websites = root.findFile("Websites"); + if (websites != null && websites.isDirectory()) { + root = websites; + } + final LegacyMirrorImport.Source source = + new LegacyMirrorImport.DocumentFileSource(resolver, root); + if (LegacyMirrorImport.totalSize(source) > dest.getUsableSpace()) { + return getString(R.string.import_mirrors_no_space); + } + final LegacyMirrorImport.Result result = LegacyMirrorImport.copyTree(source, dest); if (result.firstError() != null) { Log.w(getClass().getSimpleName(), "import: " + result.firstError()); } - refreshprojectNameSuggests(); - final String message; if (!result.isComplete()) { - message = getString(R.string.import_mirrors_partial, result.copied, result.failed); - } else if (result.copied == 0 && result.skipped == 0) { - message = getString(R.string.import_mirrors_none); - } else { - message = getString(R.string.import_mirrors_done, result.copied, result.skipped); + return getString(R.string.import_mirrors_partial, result.copied, result.failed); } - showNotification(message); + if (result.copied == 0 && result.skipped == 0) { + return getString(R.string.import_mirrors_none); + } + return getString(R.string.import_mirrors_done, result.copied, result.skipped); + } + + /** + * Report the outcome on the main thread. Uses the application context and its own handler so + * the toast still shows if the activity was rotated away during a long copy; the project list + * is refreshed only when an activity is still around to hold it. + */ + private void deliverImportOutcome(final Context appContext, final String message) { + new Handler(Looper.getMainLooper()).post(new Runnable() { + @Override + public void run() { + Toast.makeText(appContext, message, Toast.LENGTH_LONG).show(); + if (!isFinishing() && !isDestroyed()) { + refreshprojectNameSuggests(); + } + } + }); } /** diff --git a/app/src/main/java/com/httrack/android/LegacyMirrorImport.java b/app/src/main/java/com/httrack/android/LegacyMirrorImport.java index fc9c683..5ef1f36 100644 --- a/app/src/main/java/com/httrack/android/LegacyMirrorImport.java +++ b/app/src/main/java/com/httrack/android/LegacyMirrorImport.java @@ -48,10 +48,12 @@ static final class Result { int copied; int skipped; int failed; + // Set by the caller when the tree does not fit; nothing is copied in that case. + boolean outOfSpace; final List errors = new ArrayList(); boolean isComplete() { - return failed == 0; + return failed == 0 && !outOfSpace; } String firstError() { @@ -86,6 +88,9 @@ private static void copyInto(final Source dir, final File destDir, final Result final File dest = new File(destDir, name); if (child.isDirectory()) { copyInto(child, dest, result); + } else if (dest.isDirectory()) { + // A directory already sits where this file would go; skipping would drop it silently. + record(result, "name clash at " + dest); } else if (dest.exists()) { result.skipped++; } else { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index b19d7cb..06eb7ef 100755 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -146,6 +146,7 @@ Imported %1$d files (%2$d already present). Imported %1$d files; %2$d could not be copied. You can run the import again. Nothing to import from that folder. + Not enough free space to import these mirrors. Mirrors from an older version can no longer be reached directly. Import them into this app now? Import Not now diff --git a/app/src/test/java/com/httrack/android/LegacyMirrorImportTest.java b/app/src/test/java/com/httrack/android/LegacyMirrorImportTest.java index 24217f0..5f105e1 100644 --- a/app/src/test/java/com/httrack/android/LegacyMirrorImportTest.java +++ b/app/src/test/java/com/httrack/android/LegacyMirrorImportTest.java @@ -156,6 +156,183 @@ public void refusesAnEntryThatWouldEscapeTheDestination() throws Exception { assertFalse(new File(dest.getParentFile(), "escape.html").exists()); } + /** A file whose stream yields some bytes and then throws, to model a copy killed mid-write. */ + private static final class HalfThenThrow implements LegacyMirrorImport.Source { + private final String name; + + HalfThenThrow(final String name) { + this.name = name; + } + + @Override + public String getName() { + return name; + } + + @Override + public boolean isDirectory() { + return false; + } + + @Override + public long length() { + return 100; + } + + @Override + public List children() { + return new ArrayList(); + } + + @Override + public InputStream openStream() { + return new InputStream() { + private int served; + + @Override + public int read() throws IOException { + if (served++ < 50) { + return 'x'; + } + throw new IOException("stream died mid-file"); + } + }; + } + } + + @Test + public void aMidWriteFailureLeavesNoFileThatLooksComplete() throws Exception { + final FakeNode root = FakeNode.dir("HTTrack").with(new HalfThenThrow("truncated.html")); + final File dest = tmp.newFolder("dest"); + + final LegacyMirrorImport.Result r = LegacyMirrorImport.copyTree(root, dest); + + assertEquals(1, r.failed); + assertEquals(0, r.copied); + // Neither the final name nor the temp may survive: a re-run must see it as missing, not done. + assertFalse(new File(dest, "truncated.html").exists()); + assertFalse(new File(dest, "truncated.html.part").exists()); + } + + /** + * The real point of the temp-then-rename: a file only ever appears under its final name once + * whole, so a run killed mid-copy cannot leave a truncated file that a re-run trusts. We cannot + * kill the process here, but we can observe that while bytes are flowing they are NOT at the + * final name. A direct-to-destination copy fails this. + */ + @Test + public void bytesAreNotVisibleUnderTheFinalNameUntilComplete() throws Exception { + final File dest = tmp.newFolder("dest"); + final boolean[] finalNameSeenMidWrite = { false }; + final LegacyMirrorImport.Source watcher = new LegacyMirrorImport.Source() { + @Override + public String getName() { + return "HTTrack"; + } + + @Override + public boolean isDirectory() { + return true; + } + + @Override + public long length() { + return 0; + } + + @Override + public List children() { + final List kids = new ArrayList(); + kids.add(new LegacyMirrorImport.Source() { + @Override + public String getName() { + return "page.html"; + } + + @Override + public boolean isDirectory() { + return false; + } + + @Override + public long length() { + return 10; + } + + @Override + public List children() { + return new ArrayList(); + } + + @Override + public InputStream openStream() { + return new InputStream() { + private int served; + + @Override + public int read() { + if (new File(dest, "page.html").exists()) { + finalNameSeenMidWrite[0] = true; + } + return served++ < 10 ? 'x' : -1; + } + }; + } + }); + return kids; + } + + @Override + public InputStream openStream() { + throw new UnsupportedOperationException(); + } + }; + + LegacyMirrorImport.copyTree(watcher, dest); + + assertFalse("bytes appeared under the final name before the copy finished", + finalNameSeenMidWrite[0]); + assertEquals("xxxxxxxxxx", read(new File(dest, "page.html"))); + } + + /** + * DocumentFile.getName() may return null, and "."/".." are meaningless as entries. The null is + * the load-bearing case: without the guard, new File(dir, null) throws and takes the whole + * import down mid-run. The good sibling must still arrive, and copyTree must not throw. + */ + @Test + public void aNullOrDotEntryIsRefusedWithoutDeraillingTheRest() throws Exception { + final FakeNode root = FakeNode.dir("HTTrack").with( + FakeNode.file(null, "no-name"), + FakeNode.file("..", "parent"), + FakeNode.file(".", "self"), + FakeNode.file("", "empty"), + FakeNode.file("keep.html", "ok")); + final File dest = tmp.newFolder("dest"); + + final LegacyMirrorImport.Result r = LegacyMirrorImport.copyTree(root, dest); + + assertEquals(1, r.copied); + assertEquals(4, r.failed); + assertTrue(new File(dest, "keep.html").exists()); + // Nothing escaped to the destination's parent via "..". + assertFalse(new File(dest.getParentFile(), "parent").exists()); + } + + @Test + public void doesNotDropAFileWhereADirectoryAlreadyStands() throws Exception { + final File dest = tmp.newFolder("dest"); + assertTrue(new File(dest, "clash").mkdir()); + final FakeNode root = FakeNode.dir("HTTrack").with(FakeNode.file("clash", "would-be-lost")); + + final LegacyMirrorImport.Result r = LegacyMirrorImport.copyTree(root, dest); + + // The name collision is reported, not silently counted as skipped. + assertEquals(1, r.failed); + assertEquals(0, r.skipped); + assertTrue(new File(dest, "clash").isDirectory()); + } + @Test public void totalSizeSumsEveryFile() { final FakeNode root = FakeNode.dir("HTTrack").with(