From a18f642e22b9ef0b5b4075a2a6822b65d87cb4e3 Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Tue, 3 Nov 2026 14:57:04 +0100 Subject: [PATCH 1/4] Start the crawl where the browser login landed The login window is there to pass a form and stop on the page to be copied, so that page is the crawl's root. The address field now carries it in place of the address the window was opened for, which is only the way in. Credentials typed in the address field follow only when the login stayed on their own host. A login can end on a sign-on provider, and that provider has no business being sent them. This replaces part of #262, which named the opened address after #260 had added the landed one as a second crawl root. Naming the landed address in place of the typed token keeps the field to one root either way. WinHTTrack already names the landed address and overwrites the typed one, so the two front ends now agree. Signed-off-by: Xavier Roche Co-Authored-By: Claude Opus 5 (1M context) --- .../httrack/android/BrowserLoginActivity.java | 26 +++++-- .../httrack/android/BrowserLoginPolicy.java | 24 +++--- .../android/BrowserLoginPolicyTest.java | 77 ++++++++++--------- .../android/BrowserLoginWiringTest.java | 20 ++--- 4 files changed, 87 insertions(+), 60 deletions(-) diff --git a/app/src/main/java/com/httrack/android/BrowserLoginActivity.java b/app/src/main/java/com/httrack/android/BrowserLoginActivity.java index cb5f7db..1195553 100644 --- a/app/src/main/java/com/httrack/android/BrowserLoginActivity.java +++ b/app/src/main/java/com/httrack/android/BrowserLoginActivity.java @@ -81,8 +81,8 @@ public class BrowserLoginActivity extends FragmentActivity { private File bundleDir; - /** The address this window was opened for, which is the one the crawl will start from. */ - private String crawlUrl; + /** The address this window was opened for, which the crawl falls back to. */ + private String openedUrl; /** Hosts reached, in visit order, so the login's first host leads the file. */ private final Set hosts = new LinkedHashSet(); @@ -124,7 +124,7 @@ protected void onCreate(final Bundle savedInstanceState) { /* Without the credentials it may carry: the engine refuses such an address, and the crawl still authenticates from the field's own copy. */ - crawlUrl = BrowserLoginPolicy.withoutUserInfo(url); + openedUrl = BrowserLoginPolicy.withoutUserInfo(url); web = WebView.class.cast(findViewById(R.id.webLogin)); final WebSettings settings = web.getSettings(); // A login form usually needs scripts, and often its storage too. @@ -315,6 +315,20 @@ private void reportForgotten() { setResult(RESULT_OK, data); } + /** + * The address the crawl will start from, which is where the login landed. Passing a form is + * only the way in, and the page the user stops on is the one they want copied, so that + * address replaces the one this window was opened for. See BrowserLoginPolicy#withBundleUrl. + * + * @return the landed address without its credentials, or the opened one while no page has + * committed and getUrl therefore answers for none + */ + private String crawlUrl() { + final String landed = BrowserLoginPolicy.withoutUserInfo(web.getUrl()); + + return BrowserLoginPolicy.hostOf(landed) == null ? openedUrl : landed; + } + /** "Use this login" */ public void onClickLoginDone(final View view) { CookieManager.getInstance().flush(); @@ -344,11 +358,9 @@ public void onClickLoginDone(final View view) { return; } /* The engine writes the file: it owns the bundle format, the name, the scope, the caps and - the address rules, and it reads the result back through its own loader before keeping it. - crawlUrl rather than web.getUrl(), because the crawl starts from the address this window - was opened for. See BrowserLoginPolicy#withBundleUrl. */ + the address rules, and it reads the result back through its own loader before keeping it. */ final HTTrackLib.LoginBundle bundle = HTTrackLib.writeLoginBundle(result.text, - bundleDir.getAbsolutePath(), web.getSettings().getUserAgentString(), crawlUrl); + bundleDir.getAbsolutePath(), web.getSettings().getUserAgentString(), crawlUrl()); if (bundle == null) { Toast.makeText(this, getString(R.string.browser_login_no_bundle), Toast.LENGTH_LONG).show(); return; diff --git a/app/src/main/java/com/httrack/android/BrowserLoginPolicy.java b/app/src/main/java/com/httrack/android/BrowserLoginPolicy.java index 8f8cf07..ce68396 100644 --- a/app/src/main/java/com/httrack/android/BrowserLoginPolicy.java +++ b/app/src/main/java/com/httrack/android/BrowserLoginPolicy.java @@ -45,8 +45,8 @@ static String startUrl(final String urlField) { /** * The field's own text for the address {@link #startUrl} opens, which is the token the login - * names afterwards. Dropping a bundle name cannot change which token this picks, because a - * token that names no host without its name names none with it either. + * replaces afterwards. Dropping a bundle name cannot change which token this picks, because + * a token that names no host without its name names none with it either. * * @param urlField * the field's text @@ -131,14 +131,17 @@ private static int endOfAuthority(final String url, final int from) { } /** - * The address field with the bundle's name on the address the login was opened for, which is - * the one the crawl starts from. That one token is named, so the field gains no second crawl - * root. + * The address field with the login's own address in place of the one the window was opened + * for. The login stops on the page the user wants copied, so that address is the crawl's + * root and the one it was opened for is only the way in. It is replaced rather than kept, so + * the field gains no second crawl root. * * @param urlField * the field's text, one or more whitespace-separated addresses * @param bundleUrl * the start URL the engine wrote, or null when it wrote none + * @param openedToken + * the token the login was opened for, which {@link #openedToken} picked * @return the field's new text, with the user's own spacing and other addresses kept */ static String withBundleUrl(final String urlField, final String bundleUrl, @@ -157,7 +160,6 @@ static String withBundleUrl(final String urlField, final String bundleUrl, final String part = parts.group(); if (!named && part.equals(openedToken)) { - // The engine was handed no credentials, and the crawl needs the ones the user typed. kept.append(withUserInfoOf(bundleUrl, part)); named = true; } else { @@ -167,12 +169,14 @@ static String withBundleUrl(final String urlField, final String bundleUrl, if (named) { return kept.toString(); } - // The field no longer holds the address the login opened, so the name needs its own line. + // The field no longer holds the address the login opened, so the new one needs its own line. return kept.length() == 0 ? bundleUrl : kept + "\n" + bundleUrl; } /** - * URL carrying the user name and password of TYPED, which the engine was not given. + * URL carrying the user name and password of TYPED, which the engine was not given. Only + * where both name the same host, because a password typed for one site must not be sent to + * the next one the login happened to end on. * * @param url * the address the engine wrote, which holds no userinfo @@ -184,8 +188,10 @@ private static String withUserInfoOf(final String url, final String typed) { final int host = typed.indexOf("://"); final int at = host == -1 ? -1 : typed.lastIndexOf('@', endOfAuthority(typed, host + 3)); final int scheme = url.indexOf("://"); + final String reached = hostOf(url); - if (at == -1 || at < host || scheme == -1) { + if (at == -1 || at < host || scheme == -1 || reached == null + || !reached.equals(hostOf(secureUrl(typed)))) { return url; } return url.substring(0, scheme + 3) + typed.substring(host + 3, at + 1) diff --git a/app/src/test/java/com/httrack/android/BrowserLoginPolicyTest.java b/app/src/test/java/com/httrack/android/BrowserLoginPolicyTest.java index c083452..b455a3b 100644 --- a/app/src/test/java/com/httrack/android/BrowserLoginPolicyTest.java +++ b/app/src/test/java/com/httrack/android/BrowserLoginPolicyTest.java @@ -120,8 +120,11 @@ public void anExistingLogoutRuleIsLeftAsTheUserWroteIt() { assertEquals("-*signout* -*logout*", BrowserLoginPolicy.rules("-*signout*")); } - /** The start URL the engine hands back, which already names the bundle it wrote. */ - private static final String WRITTEN = "https://a.example/members/" + BrowserLoginPolicy.BUNDLE_SUFFIX + "a.example"; + /** The start URL the engine hands back, built on the address the login landed on. */ + private static final String WRITTEN = "https://forum.a.example/" + BrowserLoginPolicy.BUNDLE_SUFFIX + "forum.a.example"; + + /** The same, for a login that stayed on the host it was opened for. */ + private static final String SAME_HOST = "https://a.example/members/" + BrowserLoginPolicy.BUNDLE_SUFFIX + "a.example"; /** withBundleUrl as the activity calls it, names stripped first and the token read from that. */ private static String named(final String field, final String bundleUrl) { @@ -132,45 +135,39 @@ private static String named(final String field, final String bundleUrl) { } /** - * The name lands on the address the login was opened for, so the field keeps one crawl root. - * Appending the address the login ENDED on gave the crawl a second root, which is the bug - * this replaces. + * The login stops on the page the user wants copied, so its address replaces the one the + * window was opened for. Keeping both gave the crawl a root nobody asked for, and keeping + * only the opened one dropped the page the login was for. */ @Test - public void theNameLandsOnTheAddressTheLoginWasOpenedFor() { - assertEquals(WRITTEN, named("https://a.example/members/", WRITTEN)); + public void theLandedAddressReplacesTheOneTheLoginWasOpenedFor() { + assertEquals(WRITTEN, named("https://a.example/", WRITTEN)); // The raising startUrl applied before opening the window, so the typed form matches too. - assertEquals(WRITTEN, named("a.example/members/", WRITTEN)); - assertEquals(WRITTEN, named("http://a.example/members/", WRITTEN)); - // A second login names that address once, rather than again. + assertEquals(WRITTEN, named("a.example/", WRITTEN)); + assertEquals(WRITTEN, named("http://a.example/", WRITTEN)); + // A second login replaces the first one's address, rather than adding to it. assertEquals(WRITTEN, named(WRITTEN, WRITTEN)); - /* The same address twice is named once. Two names for one site would leave the engine with - two bundles whose cookie values clash, and it then refuses to start the crawl. */ - assertEquals(WRITTEN + " https://a.example/members/", - named("https://a.example/members/ https://a.example/members/", - WRITTEN)); + // A login that goes nowhere names the address it opened, and the field keeps that one. + assertEquals(SAME_HOST, named("https://a.example/members/", SAME_HOST)); } - /** The other addresses are the user's own text, and only one of them gains the name. */ + /** The other addresses are the user's own text, and only one of them is replaced. */ @Test public void everyOtherAddressIsLeftExactlyAsItWas() { /* The login opens the field's FIRST address, so that is the token the engine was given - and the one that gets named. The others are not touched. */ - final String first = "https://b.example/one" + BrowserLoginPolicy.BUNDLE_SUFFIX + "b.example"; - assertEquals(first + "\nhttps://a.example/members/\nhttps://c.example/two", - named( - "https://b.example/one\nhttps://a.example/members/\nhttps://c.example/two", first)); + and the one that is replaced. The others are not touched. */ + assertEquals(WRITTEN + "\nhttps://b.example/one\nhttps://c.example/two", + named("https://a.example/\nhttps://b.example/one\nhttps://c.example/two", WRITTEN)); // Any whitespace separates addresses, and the user's own spacing survives. assertEquals(WRITTEN + " \thttps://b.example/one", - named("https://a.example/members/ \thttps://b.example/one", - WRITTEN)); + named("https://a.example/ \thttps://b.example/one", WRITTEN)); // A token that is not an address is not touched, even one urlTokens would refuse. - assertEquals("-foo " + WRITTEN, - named("-foo https://a.example/members/", WRITTEN)); - // The name lands on the FIRST address, which is the one the login window opened. - assertEquals(WRITTEN + "\nhttps://a.example/other", - named("https://a.example/members/\nhttps://a.example/other", - WRITTEN)); + assertEquals("-foo " + WRITTEN, named("-foo https://a.example/", WRITTEN)); + /* The same address typed twice is replaced once. Two names for one site would leave the + engine with two bundles whose cookie values clash, and it then refuses to start the + crawl. */ + assertEquals(WRITTEN + " https://a.example/", + named("https://a.example/ https://a.example/", WRITTEN)); } /** @@ -178,19 +175,29 @@ public void everyOtherAddressIsLeftExactlyAsItWas() { * the ones the user typed, and the field's own token is what the crawl reads, so they go back. */ @Test - public void theTypedCredentialsSurviveTheNaming() { + public void theTypedCredentialsSurviveALoginOnTheirOwnHost() { assertEquals("https://user:pw@a.example/members/" + BrowserLoginPolicy.BUNDLE_SUFFIX - + "a.example", named("https://user:pw@a.example/members/", WRITTEN)); + + "a.example", named("https://user:pw@a.example/", SAME_HOST)); assertEquals("https://user@a.example/members/" + BrowserLoginPolicy.BUNDLE_SUFFIX - + "a.example", named("https://user@a.example/members/", WRITTEN)); + + "a.example", named("https://user@a.example/", SAME_HOST)); // Nothing is invented where the user typed none. - assertEquals(WRITTEN, named("https://a.example/members/", WRITTEN)); + assertEquals(SAME_HOST, named("https://a.example/", SAME_HOST)); // An '@' in the path is not userinfo, so nothing moves. assertEquals("https://a.example/u@x" + BrowserLoginPolicy.BUNDLE_SUFFIX + "a.example", named("https://a.example/u@x", "https://a.example/u@x" + BrowserLoginPolicy.BUNDLE_SUFFIX + "a.example")); } + /** + * A password typed for one site does not follow the login to the next host. The login can + * end on a sign-on provider, and that provider has no business being sent these. + */ + @Test + public void theTypedCredentialsDoNotTravelToAnotherHost() { + assertEquals(WRITTEN, named("https://user:pw@a.example/", WRITTEN)); + assertEquals(WRITTEN, named("https://user@a.example/", WRITTEN)); + } + /** A field of nothing but whitespace holds no address, so the name takes the whole field. */ @Test public void aFieldOfWhitespaceComesBackAsTheNameAlone() { @@ -275,7 +282,7 @@ public void anEngineThatWroteNothingChangesNothing() { @Test public void forgettingTheLoginTakesEveryBundleNameOutOfTheField() { assertEquals("https://a.example/members/", - BrowserLoginPolicy.withoutBundleNames(WRITTEN)); + BrowserLoginPolicy.withoutBundleNames(SAME_HOST)); // Every line, because a project can name one bundle per site it copies. assertEquals("https://a.example/\nhttps://b.example/", BrowserLoginPolicy.withoutBundleNames("https://a.example/" + BrowserLoginPolicy.BUNDLE_SUFFIX @@ -297,7 +304,7 @@ public void forgettingTheLoginTakesEveryBundleNameOutOfTheField() { /** The name is what the file is called, so the login can delete the bundles it replaces. */ @Test public void theStartUrlSaysWhichBundleItNames() { - assertEquals("a.example", BrowserLoginPolicy.bundleNameOf(WRITTEN)); + assertEquals("a.example", BrowserLoginPolicy.bundleNameOf(SAME_HOST)); assertNull(BrowserLoginPolicy.bundleNameOf("https://a.example/")); assertNull(BrowserLoginPolicy.bundleNameOf(null)); } diff --git a/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java b/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java index d727c39..df327ae 100644 --- a/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java +++ b/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java @@ -211,23 +211,25 @@ public void deletingAProjectForgetsItsLogin() throws IOException { } /** - * The engine is given the address the window was opened for, not the one the login ended on. - * That address is the crawl's own root, so naming it adds no second root, and a login ending - * on an identity provider cannot put that provider in the list. + * The engine is given the address the login landed on, because that is the page the user + * wants copied. It replaces the address the window was opened for, so the list keeps one + * crawl root. */ @Test - public void theBundleIsNamedForTheAddressTheCrawlStartsFrom() throws IOException { + public void theBundleIsNamedForTheAddressTheLoginLandedOn() throws IOException { final String source = TestSources .withoutCommentsAndStrings(TestSources.javaSource("BrowserLoginActivity")); final String done = TestSources.balancedBlock(source, TestSources.indexOf(source, "public void onClickLoginDone(")); - assertTrue(done, done.contains("getUserAgentString(), crawlUrl)")); - assertFalse("the address the login ended on is not what the crawl starts from", - done.contains("web.getUrl()")); - // And that address is the one the window was opened with, kept for the write. + assertTrue(done, done.contains("getUserAgentString(), crawlUrl())")); + final String landed = TestSources.balancedBlock(source, + TestSources.indexOf(source, "private String crawlUrl(")); + assertTrue(landed, landed.contains("withoutUserInfo(web.getUrl())")); + // getUrl answers for no page until one commits, which the opened address covers. + assertTrue(landed, landed.contains("hostOf(landed) == null ? openedUrl : landed")); final String create = TestSources.balancedBlock(source, TestSources.indexOf(source, "protected void onCreate(")); - assertTrue(create, create.contains("crawlUrl = BrowserLoginPolicy.withoutUserInfo(url)")); + assertTrue(create, create.contains("openedUrl = BrowserLoginPolicy.withoutUserInfo(url)")); final String caller = TestSources .withoutCommentsAndStrings(TestSources.javaSource("HTTrackActivity")); final String apply = TestSources.balancedBlock(caller, From f2971d2ad43ddd67bdb0079fe664007b55728438 Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Tue, 3 Nov 2026 15:12:51 +0100 Subject: [PATCH 2/4] Offer the opened address when the engine refuses the landed one A review found the landed address can be one the engine refuses. It caps the address length, and a sign-on callback carrying a long token goes past that cap. The login then saved nothing where it used to save, and the message named no cause. The window now offers the landed address first and the opened one after, so a refusal still leaves a login to save. The host match that keeps a typed password off another host now trims a trailing dot, because the engine trims one too. Without that, a field holding user:pw@a.example. lost the password. The decision moved into BrowserLoginPolicy, where a test can reach it. The old test read the activity's source text, so it passed a mutant that reverted the change and failed one that only reworded it. Signed-off-by: Xavier Roche Co-Authored-By: Claude Opus 5 (1M context) --- .../httrack/android/BrowserLoginActivity.java | 27 +++----- .../httrack/android/BrowserLoginPolicy.java | 51 +++++++++++--- .../android/BrowserLoginPolicyTest.java | 68 +++++++++++++++---- .../android/BrowserLoginWiringTest.java | 14 ++-- 4 files changed, 108 insertions(+), 52 deletions(-) diff --git a/app/src/main/java/com/httrack/android/BrowserLoginActivity.java b/app/src/main/java/com/httrack/android/BrowserLoginActivity.java index 1195553..79718fc 100644 --- a/app/src/main/java/com/httrack/android/BrowserLoginActivity.java +++ b/app/src/main/java/com/httrack/android/BrowserLoginActivity.java @@ -315,20 +315,6 @@ private void reportForgotten() { setResult(RESULT_OK, data); } - /** - * The address the crawl will start from, which is where the login landed. Passing a form is - * only the way in, and the page the user stops on is the one they want copied, so that - * address replaces the one this window was opened for. See BrowserLoginPolicy#withBundleUrl. - * - * @return the landed address without its credentials, or the opened one while no page has - * committed and getUrl therefore answers for none - */ - private String crawlUrl() { - final String landed = BrowserLoginPolicy.withoutUserInfo(web.getUrl()); - - return BrowserLoginPolicy.hostOf(landed) == null ? openedUrl : landed; - } - /** "Use this login" */ public void onClickLoginDone(final View view) { CookieManager.getInstance().flush(); @@ -358,9 +344,16 @@ public void onClickLoginDone(final View view) { return; } /* The engine writes the file: it owns the bundle format, the name, the scope, the caps and - the address rules, and it reads the result back through its own loader before keeping it. */ - final HTTrackLib.LoginBundle bundle = HTTrackLib.writeLoginBundle(result.text, - bundleDir.getAbsolutePath(), web.getSettings().getUserAgentString(), crawlUrl()); + the address rules, and it reads the result back through its own loader before keeping it. + Every address the policy offers, because the engine refuses some a WebView can reach. */ + HTTrackLib.LoginBundle bundle = null; + for (final String url : BrowserLoginPolicy.bundleUrls(web.getUrl(), openedUrl)) { + bundle = HTTrackLib.writeLoginBundle(result.text, bundleDir.getAbsolutePath(), + web.getSettings().getUserAgentString(), url); + if (bundle != null) { + break; + } + } if (bundle == null) { Toast.makeText(this, getString(R.string.browser_login_no_bundle), Toast.LENGTH_LONG).show(); return; diff --git a/app/src/main/java/com/httrack/android/BrowserLoginPolicy.java b/app/src/main/java/com/httrack/android/BrowserLoginPolicy.java index ce68396..9480678 100644 --- a/app/src/main/java/com/httrack/android/BrowserLoginPolicy.java +++ b/app/src/main/java/com/httrack/android/BrowserLoginPolicy.java @@ -45,8 +45,7 @@ static String startUrl(final String urlField) { /** * The field's own text for the address {@link #startUrl} opens, which is the token the login - * replaces afterwards. Dropping a bundle name cannot change which token this picks, because - * a token that names no host without its name names none with it either. + * replaces afterwards. * * @param urlField * the field's text @@ -97,6 +96,31 @@ static String hostOf(final String url) { return host.toLowerCase(Locale.ROOT); } + /** + * The addresses to offer the engine for this login, best first. The login stops on the page + * the user wants copied, so that address leads. The engine refuses an address it cannot + * name, such as a sign-on callback carrying a long token. The opened address then still + * saves the login. + * + * @param landedUrl + * the login window's own address, or null while no page has committed + * @param openedUrl + * the address the window was opened for, which carries no credentials + * @return the addresses to try in order, without repeats + */ + static List bundleUrls(final String landedUrl, final String openedUrl) { + final List urls = new ArrayList(); + final String landed = withoutUserInfo(landedUrl); + + if (hostOf(landed) != null) { + urls.add(landed); + } + if (openedUrl != null && !urls.contains(openedUrl)) { + urls.add(openedUrl); + } + return urls; + } + /** What a start URL ends with to name a login bundle, HTS_BUNDLE_SUFFIX in the engine. */ static final String BUNDLE_SUFFIX = "#httrack-bundle="; @@ -132,9 +156,8 @@ private static int endOfAuthority(final String url, final int from) { /** * The address field with the login's own address in place of the one the window was opened - * for. The login stops on the page the user wants copied, so that address is the crawl's - * root and the one it was opened for is only the way in. It is replaced rather than kept, so - * the field gains no second crawl root. + * for. The login stops on the page the user wants copied, so that page becomes the crawl's + * root. One token is replaced, so the field gains no second crawl root. * * @param urlField * the field's text, one or more whitespace-separated addresses @@ -174,9 +197,9 @@ static String withBundleUrl(final String urlField, final String bundleUrl, } /** - * URL carrying the user name and password of TYPED, which the engine was not given. Only - * where both name the same host, because a password typed for one site must not be sent to - * the next one the login happened to end on. + * URL carrying the user name and password of TYPED, which the engine was not given. The + * merge happens only when both name the same host, because a password typed for one site + * must not follow the login to another host. * * @param url * the address the engine wrote, which holds no userinfo @@ -188,16 +211,22 @@ private static String withUserInfoOf(final String url, final String typed) { final int host = typed.indexOf("://"); final int at = host == -1 ? -1 : typed.lastIndexOf('@', endOfAuthority(typed, host + 3)); final int scheme = url.indexOf("://"); - final String reached = hostOf(url); + final String reached = bareHost(hostOf(url)); + final boolean sameHost = + reached != null && reached.equals(bareHost(hostOf(secureUrl(typed)))); - if (at == -1 || at < host || scheme == -1 || reached == null - || !reached.equals(hostOf(secureUrl(typed)))) { + if (at == -1 || at < host || scheme == -1 || !sameHost) { return url; } return url.substring(0, scheme + 3) + typed.substring(host + 3, at + 1) + url.substring(scheme + 3); } + /** HOST without the trailing dots the engine trims, so both sides of a match agree. */ + private static String bareHost(final String host) { + return host == null ? null : host.replaceAll("\\.+$", ""); + } + /** * The address field with every bundle name dropped, which is what Forget leaves behind. A * start URL naming a bundle that is gone stops the crawl at startup rather than running it diff --git a/app/src/test/java/com/httrack/android/BrowserLoginPolicyTest.java b/app/src/test/java/com/httrack/android/BrowserLoginPolicyTest.java index b455a3b..67ee690 100644 --- a/app/src/test/java/com/httrack/android/BrowserLoginPolicyTest.java +++ b/app/src/test/java/com/httrack/android/BrowserLoginPolicyTest.java @@ -120,10 +120,10 @@ public void anExistingLogoutRuleIsLeftAsTheUserWroteIt() { assertEquals("-*signout* -*logout*", BrowserLoginPolicy.rules("-*signout*")); } - /** The start URL the engine hands back, built on the address the login landed on. */ + /** The engine hands this back for a login that landed on a host the field never named. */ private static final String WRITTEN = "https://forum.a.example/" + BrowserLoginPolicy.BUNDLE_SUFFIX + "forum.a.example"; - /** The same, for a login that stayed on the host it was opened for. */ + /** The engine hands this back when the login stayed on the host the field named. */ private static final String SAME_HOST = "https://a.example/members/" + BrowserLoginPolicy.BUNDLE_SUFFIX + "a.example"; /** withBundleUrl as the activity calls it, names stripped first and the token read from that. */ @@ -135,9 +135,8 @@ private static String named(final String field, final String bundleUrl) { } /** - * The login stops on the page the user wants copied, so its address replaces the one the - * window was opened for. Keeping both gave the crawl a root nobody asked for, and keeping - * only the opened one dropped the page the login was for. + * Keeping both gave the crawl a root nobody asked for, and keeping the opened one alone + * dropped the page the login was for. */ @Test public void theLandedAddressReplacesTheOneTheLoginWasOpenedFor() { @@ -149,6 +148,8 @@ public void theLandedAddressReplacesTheOneTheLoginWasOpenedFor() { assertEquals(WRITTEN, named(WRITTEN, WRITTEN)); // A login that goes nowhere names the address it opened, and the field keeps that one. assertEquals(SAME_HOST, named("https://a.example/members/", SAME_HOST)); + // A bare token on the host the login stayed on, which is where the userinfo merge runs. + assertEquals(SAME_HOST, named("a.example/members/", SAME_HOST)); } /** The other addresses are the user's own text, and only one of them is replaced. */ @@ -170,16 +171,19 @@ public void everyOtherAddressIsLeftExactlyAsItWas() { named("https://a.example/ https://a.example/", WRITTEN)); } - /** - * The engine is handed no credentials, so the address it returns has none. The crawl needs - * the ones the user typed, and the field's own token is what the crawl reads, so they go back. - */ + /** The engine is handed no credentials, so the crawl needs the ones the user typed. */ @Test public void theTypedCredentialsSurviveALoginOnTheirOwnHost() { - assertEquals("https://user:pw@a.example/members/" + BrowserLoginPolicy.BUNDLE_SUFFIX - + "a.example", named("https://user:pw@a.example/", SAME_HOST)); + final String merged = "https://user:pw@a.example/members/" + BrowserLoginPolicy.BUNDLE_SUFFIX + + "a.example"; + + assertEquals(merged, named("https://user:pw@a.example/", SAME_HOST)); assertEquals("https://user@a.example/members/" + BrowserLoginPolicy.BUNDLE_SUFFIX + "a.example", named("https://user@a.example/", SAME_HOST)); + // The engine trims a trailing host dot, so the two sides still name the same host. + assertEquals(merged, named("https://user:pw@a.example./", SAME_HOST)); + // The window raised the token to https, and the field still holds the http one. + assertEquals(merged, named("http://user:pw@a.example/", SAME_HOST)); // Nothing is invented where the user typed none. assertEquals(SAME_HOST, named("https://a.example/", SAME_HOST)); // An '@' in the path is not userinfo, so nothing moves. @@ -188,14 +192,48 @@ public void theTypedCredentialsSurviveALoginOnTheirOwnHost() { "https://a.example/u@x" + BrowserLoginPolicy.BUNDLE_SUFFIX + "a.example")); } - /** - * A password typed for one site does not follow the login to the next host. The login can - * end on a sign-on provider, and that provider has no business being sent these. - */ + /** A login can end on a sign-on provider, which has no business being sent these. */ @Test public void theTypedCredentialsDoNotTravelToAnotherHost() { assertEquals(WRITTEN, named("https://user:pw@a.example/", WRITTEN)); assertEquals(WRITTEN, named("https://user@a.example/", WRITTEN)); + // Nor back up to the apex from a subdomain the user typed them for. + assertEquals(SAME_HOST, named("https://user:pw@forum.a.example/", SAME_HOST)); + // No https address means no host to match, so there is nothing to merge them onto. + final String cleartext = "http://a.example/x" + BrowserLoginPolicy.BUNDLE_SUFFIX + "a.example"; + assertEquals(cleartext, named("https://user:pw@a.example/", cleartext)); + } + + /** + * The landed address leads, because that is the page to copy. The opened one follows, so a + * landed address the engine refuses still leaves a login to save. + */ + @Test + public void theLandedAddressIsOfferedBeforeTheOpenedOne() { + final String opened = "https://a.example/members/"; + + assertEquals(Arrays.asList("https://forum.a.example/x", opened), + BrowserLoginPolicy.bundleUrls("https://forum.a.example/x", opened)); + // One address, offered once. + assertEquals(Arrays.asList(opened), BrowserLoginPolicy.bundleUrls(opened, opened)); + // The engine refuses an address holding credentials, so they go before it sees one. + assertEquals(Arrays.asList("https://a.example/x", opened), + BrowserLoginPolicy.bundleUrls("https://user:pw@a.example/x", opened)); + } + + /** An address the window cannot have landed on is not offered, so the opened one stands. */ + @Test + public void onlyAnHttpsAddressNamingAHostIsOffered() { + final String opened = "https://a.example/members/"; + + // getUrl answers for no page until one commits. + assertEquals(Arrays.asList(opened), BrowserLoginPolicy.bundleUrls(null, opened)); + assertEquals(Arrays.asList(opened), BrowserLoginPolicy.bundleUrls("about:blank", opened)); + assertEquals(Arrays.asList(opened), BrowserLoginPolicy.bundleUrls("data:text/html,x", opened)); + assertEquals(Arrays.asList(opened), BrowserLoginPolicy.bundleUrls("http://a.example/x", opened)); + assertEquals(Arrays.asList(opened), BrowserLoginPolicy.bundleUrls("https:///x", opened)); + // Nothing to offer stops the write, which is what the window reports as a refusal. + assertTrue(BrowserLoginPolicy.bundleUrls(null, null).isEmpty()); } /** A field of nothing but whitespace holds no address, so the name takes the whole field. */ diff --git a/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java b/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java index df327ae..7ff016d 100644 --- a/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java +++ b/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java @@ -211,9 +211,8 @@ public void deletingAProjectForgetsItsLogin() throws IOException { } /** - * The engine is given the address the login landed on, because that is the page the user - * wants copied. It replaces the address the window was opened for, so the list keeps one - * crawl root. + * The engine is offered the address the login landed on, and the window keeps trying until + * one is accepted. BrowserLoginPolicyTest owns which addresses those are. */ @Test public void theBundleIsNamedForTheAddressTheLoginLandedOn() throws IOException { @@ -221,12 +220,9 @@ public void theBundleIsNamedForTheAddressTheLoginLandedOn() throws IOException { .withoutCommentsAndStrings(TestSources.javaSource("BrowserLoginActivity")); final String done = TestSources.balancedBlock(source, TestSources.indexOf(source, "public void onClickLoginDone(")); - assertTrue(done, done.contains("getUserAgentString(), crawlUrl())")); - final String landed = TestSources.balancedBlock(source, - TestSources.indexOf(source, "private String crawlUrl(")); - assertTrue(landed, landed.contains("withoutUserInfo(web.getUrl())")); - // getUrl answers for no page until one commits, which the opened address covers. - assertTrue(landed, landed.contains("hostOf(landed) == null ? openedUrl : landed")); + assertTrue(done, done.contains("BrowserLoginPolicy.bundleUrls(web.getUrl(), openedUrl)")); + // The first address the engine accepts, because it refuses some a WebView can reach. + assertTrue(done, done.contains("if (bundle != null)")); final String create = TestSources.balancedBlock(source, TestSources.indexOf(source, "protected void onCreate(")); assertTrue(create, create.contains("openedUrl = BrowserLoginPolicy.withoutUserInfo(url)")); From 3645ca21983b9adbbecf17e50f5aa50553d191b3 Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Tue, 3 Nov 2026 15:14:10 +0100 Subject: [PATCH 3/4] Trim the trailing dot only where one can appear The engine cuts a trailing host dot, so the address it writes never carries one and trimming that side changed nothing. A mutation that dropped it left every test green. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Xavier Roche --- app/src/main/java/com/httrack/android/BrowserLoginPolicy.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/httrack/android/BrowserLoginPolicy.java b/app/src/main/java/com/httrack/android/BrowserLoginPolicy.java index 9480678..5c1558b 100644 --- a/app/src/main/java/com/httrack/android/BrowserLoginPolicy.java +++ b/app/src/main/java/com/httrack/android/BrowserLoginPolicy.java @@ -211,7 +211,7 @@ private static String withUserInfoOf(final String url, final String typed) { final int host = typed.indexOf("://"); final int at = host == -1 ? -1 : typed.lastIndexOf('@', endOfAuthority(typed, host + 3)); final int scheme = url.indexOf("://"); - final String reached = bareHost(hostOf(url)); + final String reached = hostOf(url); final boolean sameHost = reached != null && reached.equals(bareHost(hostOf(secureUrl(typed)))); @@ -222,7 +222,7 @@ private static String withUserInfoOf(final String url, final String typed) { + url.substring(scheme + 3); } - /** HOST without the trailing dots the engine trims, so both sides of a match agree. */ + /** HOST without the trailing dots, which the engine cuts and the field's own token keeps. */ private static String bareHost(final String host) { return host == null ? null : host.replaceAll("\\.+$", ""); } From aeb19d42d5b0ef3d4061f09f14e6875cdd9e60be Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Tue, 3 Nov 2026 15:21:30 +0100 Subject: [PATCH 4/4] Say which addresses the login offers, not which one it wrote This file already used "bundle URL" for the start URL the engine WROTE, which carries the bundle name. The new helper meant the reverse, the addresses offered TO the engine, and the window used both senses eleven lines apart. Two review mutants reverted the change and kept every test green: one passed the opened address inside the loop, the other took the last accepted address instead of the first. The window test now pins both. The table of refused addresses gained the positive control it lacked, so it can no longer pass an implementation that never offers a landed address at all. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Xavier Roche --- .../httrack/android/BrowserLoginActivity.java | 7 ++--- .../httrack/android/BrowserLoginPolicy.java | 14 ++++----- .../android/BrowserLoginPolicyTest.java | 30 ++++++++++++------- .../android/BrowserLoginWiringTest.java | 8 +++-- 4 files changed, 34 insertions(+), 25 deletions(-) diff --git a/app/src/main/java/com/httrack/android/BrowserLoginActivity.java b/app/src/main/java/com/httrack/android/BrowserLoginActivity.java index 79718fc..7ab5f03 100644 --- a/app/src/main/java/com/httrack/android/BrowserLoginActivity.java +++ b/app/src/main/java/com/httrack/android/BrowserLoginActivity.java @@ -343,11 +343,10 @@ public void onClickLoginDone(final View view) { .show(); return; } - /* The engine writes the file: it owns the bundle format, the name, the scope, the caps and - the address rules, and it reads the result back through its own loader before keeping it. - Every address the policy offers, because the engine refuses some a WebView can reach. */ + /* The engine decides what it writes, so it gets every address the policy offers and keeps + trying until one is accepted. */ HTTrackLib.LoginBundle bundle = null; - for (final String url : BrowserLoginPolicy.bundleUrls(web.getUrl(), openedUrl)) { + for (final String url : BrowserLoginPolicy.addressesToTry(web.getUrl(), openedUrl)) { bundle = HTTrackLib.writeLoginBundle(result.text, bundleDir.getAbsolutePath(), web.getSettings().getUserAgentString(), url); if (bundle != null) { diff --git a/app/src/main/java/com/httrack/android/BrowserLoginPolicy.java b/app/src/main/java/com/httrack/android/BrowserLoginPolicy.java index 5c1558b..cf1829c 100644 --- a/app/src/main/java/com/httrack/android/BrowserLoginPolicy.java +++ b/app/src/main/java/com/httrack/android/BrowserLoginPolicy.java @@ -99,8 +99,8 @@ static String hostOf(final String url) { /** * The addresses to offer the engine for this login, best first. The login stops on the page * the user wants copied, so that address leads. The engine refuses an address it cannot - * name, such as a sign-on callback carrying a long token. The opened address then still - * saves the login. + * name, such as a sign-on callback carrying a long token, and the login is then saved under + * the opened address instead. Nothing is screened here that only the engine can judge. * * @param landedUrl * the login window's own address, or null while no page has committed @@ -108,14 +108,14 @@ static String hostOf(final String url) { * the address the window was opened for, which carries no credentials * @return the addresses to try in order, without repeats */ - static List bundleUrls(final String landedUrl, final String openedUrl) { + static List addressesToTry(final String landedUrl, final String openedUrl) { final List urls = new ArrayList(); final String landed = withoutUserInfo(landedUrl); if (hostOf(landed) != null) { urls.add(landed); } - if (openedUrl != null && !urls.contains(openedUrl)) { + if (hostOf(openedUrl) != null && !urls.contains(openedUrl)) { urls.add(openedUrl); } return urls; @@ -213,7 +213,7 @@ private static String withUserInfoOf(final String url, final String typed) { final int scheme = url.indexOf("://"); final String reached = hostOf(url); final boolean sameHost = - reached != null && reached.equals(bareHost(hostOf(secureUrl(typed)))); + reached != null && reached.equals(withoutTrailingDots(hostOf(secureUrl(typed)))); if (at == -1 || at < host || scheme == -1 || !sameHost) { return url; @@ -222,8 +222,8 @@ private static String withUserInfoOf(final String url, final String typed) { + url.substring(scheme + 3); } - /** HOST without the trailing dots, which the engine cuts and the field's own token keeps. */ - private static String bareHost(final String host) { + /** HOST loses the trailing dots the engine also cuts, so a match compares like with like. */ + private static String withoutTrailingDots(final String host) { return host == null ? null : host.replaceAll("\\.+$", ""); } diff --git a/app/src/test/java/com/httrack/android/BrowserLoginPolicyTest.java b/app/src/test/java/com/httrack/android/BrowserLoginPolicyTest.java index 67ee690..f41bdac 100644 --- a/app/src/test/java/com/httrack/android/BrowserLoginPolicyTest.java +++ b/app/src/test/java/com/httrack/android/BrowserLoginPolicyTest.java @@ -148,7 +148,7 @@ public void theLandedAddressReplacesTheOneTheLoginWasOpenedFor() { assertEquals(WRITTEN, named(WRITTEN, WRITTEN)); // A login that goes nowhere names the address it opened, and the field keeps that one. assertEquals(SAME_HOST, named("https://a.example/members/", SAME_HOST)); - // A bare token on the host the login stayed on, which is where the userinfo merge runs. + // A bare token on the host the login stayed on still reaches the userinfo merge. assertEquals(SAME_HOST, named("a.example/members/", SAME_HOST)); } @@ -192,7 +192,7 @@ public void theTypedCredentialsSurviveALoginOnTheirOwnHost() { "https://a.example/u@x" + BrowserLoginPolicy.BUNDLE_SUFFIX + "a.example")); } - /** A login can end on a sign-on provider, which has no business being sent these. */ + /** A login can end on a sign-on provider that must not receive typed credentials. */ @Test public void theTypedCredentialsDoNotTravelToAnotherHost() { assertEquals(WRITTEN, named("https://user:pw@a.example/", WRITTEN)); @@ -213,12 +213,16 @@ public void theLandedAddressIsOfferedBeforeTheOpenedOne() { final String opened = "https://a.example/members/"; assertEquals(Arrays.asList("https://forum.a.example/x", opened), - BrowserLoginPolicy.bundleUrls("https://forum.a.example/x", opened)); + BrowserLoginPolicy.addressesToTry("https://forum.a.example/x", opened)); // One address, offered once. - assertEquals(Arrays.asList(opened), BrowserLoginPolicy.bundleUrls(opened, opened)); + assertEquals(Arrays.asList(opened), BrowserLoginPolicy.addressesToTry(opened, opened)); // The engine refuses an address holding credentials, so they go before it sees one. assertEquals(Arrays.asList("https://a.example/x", opened), - BrowserLoginPolicy.bundleUrls("https://user:pw@a.example/x", opened)); + BrowserLoginPolicy.addressesToTry("https://user:pw@a.example/x", opened)); + /* Only the engine can judge an IPv6 literal, a public suffix or the length cap, which it + works out from the name. A screen here would answer differently from the engine. */ + assertEquals(Arrays.asList("https://[::1]/x", opened), + BrowserLoginPolicy.addressesToTry("https://[::1]/x", opened)); } /** An address the window cannot have landed on is not offered, so the opened one stands. */ @@ -226,14 +230,18 @@ public void theLandedAddressIsOfferedBeforeTheOpenedOne() { public void onlyAnHttpsAddressNamingAHostIsOffered() { final String opened = "https://a.example/members/"; + // The positive control, because every other row here expects the opened address alone. + assertEquals(Arrays.asList("https://a.example/x", opened), + BrowserLoginPolicy.addressesToTry("https://a.example/x", opened)); // getUrl answers for no page until one commits. - assertEquals(Arrays.asList(opened), BrowserLoginPolicy.bundleUrls(null, opened)); - assertEquals(Arrays.asList(opened), BrowserLoginPolicy.bundleUrls("about:blank", opened)); - assertEquals(Arrays.asList(opened), BrowserLoginPolicy.bundleUrls("data:text/html,x", opened)); - assertEquals(Arrays.asList(opened), BrowserLoginPolicy.bundleUrls("http://a.example/x", opened)); - assertEquals(Arrays.asList(opened), BrowserLoginPolicy.bundleUrls("https:///x", opened)); + assertEquals(Arrays.asList(opened), BrowserLoginPolicy.addressesToTry(null, opened)); + assertEquals(Arrays.asList(opened), BrowserLoginPolicy.addressesToTry("about:blank", opened)); + assertEquals(Arrays.asList(opened), + BrowserLoginPolicy.addressesToTry("data:text/html,x", opened)); + assertEquals(Arrays.asList(opened), + BrowserLoginPolicy.addressesToTry("http://a.example/x", opened)); // Nothing to offer stops the write, which is what the window reports as a refusal. - assertTrue(BrowserLoginPolicy.bundleUrls(null, null).isEmpty()); + assertTrue(BrowserLoginPolicy.addressesToTry(null, null).isEmpty()); } /** A field of nothing but whitespace holds no address, so the name takes the whole field. */ diff --git a/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java b/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java index 7ff016d..d22fc00 100644 --- a/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java +++ b/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java @@ -220,9 +220,11 @@ public void theBundleIsNamedForTheAddressTheLoginLandedOn() throws IOException { .withoutCommentsAndStrings(TestSources.javaSource("BrowserLoginActivity")); final String done = TestSources.balancedBlock(source, TestSources.indexOf(source, "public void onClickLoginDone(")); - assertTrue(done, done.contains("BrowserLoginPolicy.bundleUrls(web.getUrl(), openedUrl)")); - // The first address the engine accepts, because it refuses some a WebView can reach. - assertTrue(done, done.contains("if (bundle != null)")); + assertTrue(done, done.contains("BrowserLoginPolicy.addressesToTry(web.getUrl(), openedUrl)")); + // The loop's own address, or every attempt would name the one the window was opened for. + assertTrue(done, done.contains("getUserAgentString(), url);")); + // The loop stops at the first address the engine accepts, rather than at the last. + assertTrue(done, done.contains("if (bundle != null) {\n break;")); final String create = TestSources.balancedBlock(source, TestSources.indexOf(source, "protected void onCreate(")); assertTrue(create, create.contains("openedUrl = BrowserLoginPolicy.withoutUserInfo(url)"));