Start the crawl where the browser login landed

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<String> hosts = new LinkedHashSet<String>();
@@ -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;

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<String> addressesToTry(final String landedUrl, final String openedUrl) {
+    final List<String> urls = new ArrayList<String>();
+    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

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));
   }