diff --git a/app/src/main/java/com/httrack/android/BrowserLoginActivity.java b/app/src/main/java/com/httrack/android/BrowserLoginActivity.java index cb5f7db..7ab5f03 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. @@ -343,12 +343,16 @@ 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. - crawlUrl rather than web.getUrl(), because the crawl starts from the address this window - was opened for. See BrowserLoginPolicy#withBundleUrl. */ - final HTTrackLib.LoginBundle bundle = HTTrackLib.writeLoginBundle(result.text, - bundleDir.getAbsolutePath(), web.getSettings().getUserAgentString(), crawlUrl); + /* 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.addressesToTry(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 8f8cf07..cf1829c 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 - * 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. * * @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, 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 + * @param openedUrl + * the address the window was opened for, which carries no credentials + * @return the addresses to try in order, without repeats + */ + 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 (hostOf(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="; @@ -131,14 +155,16 @@ 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 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 * @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 +183,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 +192,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. 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 @@ -184,14 +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 boolean sameHost = + reached != null && reached.equals(withoutTrailingDots(hostOf(secureUrl(typed)))); - if (at == -1 || at < host || scheme == -1) { + 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 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("\\.+$", ""); + } + /** * 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 c083452..f41bdac 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 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 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. */ private static String named(final String field, final String bundleUrl) { @@ -132,65 +135,115 @@ 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. + * 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 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)); + // A bare token on the host the login stayed on still reaches the userinfo merge. + assertEquals(SAME_HOST, named("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)); } - /** - * 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 theTypedCredentialsSurviveTheNaming() { - assertEquals("https://user:pw@a.example/members/" + BrowserLoginPolicy.BUNDLE_SUFFIX - + "a.example", named("https://user:pw@a.example/members/", WRITTEN)); + public void theTypedCredentialsSurviveALoginOnTheirOwnHost() { + 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/members/", WRITTEN)); + + "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(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 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)); + 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.addressesToTry("https://forum.a.example/x", opened)); + // One address, offered once. + 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.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. */ + @Test + 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.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.addressesToTry(null, null).isEmpty()); + } + /** A field of nothing but whitespace holds no address, so the name takes the whole field. */ @Test public void aFieldOfWhitespaceComesBackAsTheNameAlone() { @@ -275,7 +328,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 +350,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..d22fc00 100644 --- a/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java +++ b/app/src/test/java/com/httrack/android/BrowserLoginWiringTest.java @@ -211,23 +211,23 @@ 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 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 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("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("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,