diff --git a/app/src/main/java/com/httrack/android/BrowserLoginActivity.java b/app/src/main/java/com/httrack/android/BrowserLoginActivity.java index c5d9d12..bfb26c4 100644 --- a/app/src/main/java/com/httrack/android/BrowserLoginActivity.java +++ b/app/src/main/java/com/httrack/android/BrowserLoginActivity.java @@ -265,8 +265,8 @@ public void onClickLoginDone(final View view) { /** * Write TEXT to FILE, readable by this app alone. * - * The file holds a live session, so it writes a .new beside it and renames, because a write - * that failed halfway would leave the previous login truncated and the crawl with nothing. + * The file holds a live session, so it stages beside FILE and renames, because a write that + * failed halfway would leave the previous login truncated and the crawl with nothing. * * @return true when FILE now holds TEXT */ @@ -275,7 +275,7 @@ static boolean writeOwnerOnly(final File file, final String text) { if (dir != null && !dir.isDirectory() && !dir.mkdirs()) { return false; } - final File staged = new File(file.getPath() + ".new"); + final File staged = StoragePaths.stagedLoginFile(file); try (final OutputStream out = new FileOutputStream(staged)) { out.write(text.getBytes(UTF8)); } catch (final IOException e) { diff --git a/app/src/main/java/com/httrack/android/StoragePaths.java b/app/src/main/java/com/httrack/android/StoragePaths.java index c38433f..9640e7a 100644 --- a/app/src/main/java/com/httrack/android/StoragePaths.java +++ b/app/src/main/java/com/httrack/android/StoragePaths.java @@ -276,14 +276,30 @@ static List savedLoginFiles(final File internal, final String projectName) return files; } + /** {@link #isLoginSibling} must accept the name {@link #stagedLoginFile} builds. */ + private static final String STAGED_SUFFIX = ".new"; + /** - * Whether FILENAME is LOGINNAME, or one of the files the engine's save leaves beside it. - * Those are its {@code .bak}, its {@code ..tmp}, and the temporary the {@code .bak} is - * staged through, because each one can hold a whole jar. No run deletes another's temporary. + * Whether LOGINFILENAME names CANDIDATE, or one of the files a save leaves beside it. The + * engine leaves its {@code .bak}, its {@code ..tmp}, and the temporary the {@code .bak} + * is staged through. Our own write leaves its staged copy when the app dies before the + * rename. Each one can hold a whole jar. No run deletes another's temporary. */ - static boolean isLoginSibling(final String ourName, final String candidate) { + static boolean isLoginSibling(final String loginFileName, final String candidate) { return candidate != null - && candidate.matches(Pattern.quote(ourName) + "(\\.bak)?(\\.[0-9]+\\.tmp)?"); + && candidate.matches(Pattern.quote(loginFileName) + "(" + Pattern.quote(STAGED_SUFFIX) + + "|(\\.bak)?(\\.[0-9]+\\.tmp)?)"); + } + + /** + * Where a login write stages before it renames onto FILE. + * + * @param file + * the login file being written + * @return the name to write first + */ + static File stagedLoginFile(final File file) { + return new File(file.getPath() + STAGED_SUFFIX); } /** diff --git a/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java b/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java index 76726be..4894866 100644 --- a/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java +++ b/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java @@ -143,6 +143,19 @@ public void deletingAProjectForgetsItsLogin() throws IOException { assertTrue(forget, forget.contains("StoragePaths.savedLoginFiles(")); } + /** The staged name must come from the shared helper, so the delete pattern can match it. */ + @Test + public void theStagedWriteBuildsItsNameFromTheSharedHelper() throws IOException { + final String source = TestSources + .withoutCommentsAndStrings(TestSources.javaSource("BrowserLoginActivity")); + final String body = TestSources.balancedBlock(source, + TestSources.indexOf(source, "static boolean writeOwnerOnly(")); + assertTrue(body, body.contains("StoragePaths.stagedLoginFile(file)")); + // On the staged file, because calling the helper and then writing elsewhere saves nothing. + assertTrue(body, body.contains("FileOutputStream(staged)")); + assertTrue(body, body.contains("staged.renameTo(file)")); + } + /** -%K is read only when the engine accepts cookies, so the hand-off has to tick the box. */ @Test public void theHandOverTurnsCookiesOn() throws IOException { diff --git a/app/src/test/java/com/httrack/android/StoragePathsTest.java b/app/src/test/java/com/httrack/android/StoragePathsTest.java index 70e6861..8660226 100644 --- a/app/src/test/java/com/httrack/android/StoragePathsTest.java +++ b/app/src/test/java/com/httrack/android/StoragePathsTest.java @@ -39,6 +39,16 @@ public void aDeletedLoginTakesWhatTheEngineLeftBesideIt() { assertTrue(StoragePaths.isLoginSibling("My Site.txt", "My Site.txt.bak.4321.tmp")); } + /** The matcher must take whatever our write stages, or a crash mid-save leaks the session. */ + @Test + public void aDeletedLoginTakesTheStagedWriteACrashLeftBehind() { + final File login = StoragePaths.loginCookiesFile(internal, "My Site"); + // Beside the login, because savedLoginFiles lists that one directory and nothing else. + assertEquals(new File(login.getPath() + ".new"), StoragePaths.stagedLoginFile(login)); + assertTrue(StoragePaths.isLoginSibling(login.getName(), + StoragePaths.stagedLoginFile(login).getName())); + } + @Test public void aDeletedLoginLeavesAnotherProjectAlone() { assertFalse(StoragePaths.isLoginSibling("A.txt", "B.txt")); @@ -49,6 +59,9 @@ public void aDeletedLoginLeavesAnotherProjectAlone() { assertFalse(StoragePaths.isLoginSibling("A.txt", "A.txt.bak2")); assertFalse(StoragePaths.isLoginSibling("A.txt", "A.txt.bak.tmp")); assertFalse(StoragePaths.isLoginSibling("A.txt", "A.txt.4321.bak")); + // The staged suffix stands alone, because nothing writes it onto a .bak or a temporary. + assertFalse(StoragePaths.isLoginSibling("A.txt", "A.txt.bak.new")); + assertFalse(StoragePaths.isLoginSibling("A.txt", "A.txt.new.new")); assertFalse(StoragePaths.isLoginSibling("A.txt", null)); /* isValidProjectName allows a regex metacharacter, so the name is quoted before it becomes a pattern. Unquoted, "A+" would match another project's "AA.txt" and delete its session, @@ -65,7 +78,7 @@ public void savedLoginFilesTakesOurOwnAndLeavesTheNeighbour() throws IOException final File logins = new File(internal, "logins"); assertTrue(logins.mkdirs()); for (final String name : new String[] { "P.txt", "P.txt.bak", "P.txt.4321.tmp", - "P.txt.bak.4321.tmp", "Q.txt", "Q.txt.bak", "notes.txt" }) { + "P.txt.bak.4321.tmp", "P.txt.new", "Q.txt", "Q.txt.bak", "Q.txt.new", "notes.txt" }) { assertTrue(new File(logins, name).createNewFile()); } final java.util.List ours = StoragePaths.savedLoginFiles(internal, "P"); @@ -74,7 +87,7 @@ public void savedLoginFilesTakesOurOwnAndLeavesTheNeighbour() throws IOException names.add(file.getName()); } assertEquals(new java.util.TreeSet(java.util.Arrays.asList("P.txt", "P.txt.bak", - "P.txt.4321.tmp", "P.txt.bak.4321.tmp")), names); + "P.txt.4321.tmp", "P.txt.bak.4321.tmp", "P.txt.new")), names); } @Test