Fix relative 'fetch' locations.

The Android manifest
(https://android.googlesource.com/platform/manifest/+/master/default.xml)
uses this.

Change-Id: I27720dedc5afef143a8bfc3c8f7daa9438792c46
diff --git a/src/main/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java b/src/main/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
index f6a45d4..447252a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
@@ -41,6 +41,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import org.apache.http.client.utils.URIBuilder;
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.api.errors.InvalidRemoteException;
 import org.eclipse.jgit.api.errors.RefNotFoundException;
@@ -70,7 +71,7 @@
   private static final String SECTION_NAME = "superproject";
 
   private final GitRepositoryManager repoManager;
-  private final String canonicalWebUrl;
+  private final URI canonicalWebUrl;
   private final PluginConfigFactory cfgFactory;
   private final String pluginName;
   private final AllProjectsName allProjectsName;
@@ -93,7 +94,12 @@
     this.serverIdent = serverIdent;
     this.allProjectsName = allProjectsName;
     this.repoManager = repoManager;
-    this.canonicalWebUrl = canonicalWebUrl;
+    try {
+      this.canonicalWebUrl = new URI(canonicalWebUrl);
+    } catch (URISyntaxException e) {
+      throw new IllegalArgumentException(e);
+    }
+
     this.cfgFactory = cfgFactory;
     this.projectCache = projectCache;
   }
@@ -112,6 +118,7 @@
   private static class ConfigEntry {
     Project.NameKey srcRepoKey;
     String srcRef;
+    URI srcRepoUrl;
     String xmlPath;
     Project.NameKey destRepoKey;
     boolean recordSubmoduleLabels;
@@ -179,23 +186,27 @@
     }
     for (String subsect : cfg.getSubsections(SECTION_NAME)) {
       try {
-        ConfigEntry e = newConfigEntry(cfg, subsect);
-        if (destinations.contains(e.srcRepoKey.get()) || sources.contains(e.destRepoKey.get())) {
+        ConfigEntry configEntry = newConfigEntry(cfg, subsect);
+        if (destinations.contains(configEntry.srcRepoKey.get())
+            || sources.contains(configEntry.destRepoKey.get())) {
           // Don't want cyclic dependencies.
           throw new ConfigInvalidException(
-              String.format("repo in entry %s cannot be both source and destination", e));
+              String.format("repo in entry %s cannot be both source and destination", configEntry));
         }
-        if (e.destBranch.equals("*")) {
-          if (wildcardDestinations.contains(e.destRepoKey.get())) {
+        if (configEntry.destBranch.equals("*")) {
+          if (wildcardDestinations.contains(configEntry.destRepoKey.get())) {
             throw new ConfigInvalidException(
-                String.format("repo %s already has a wildcard destination branch.", e.destRepoKey));
+                String.format(
+                    "repo %s already has a wildcard destination branch.", configEntry.destRepoKey));
           }
-          wildcardDestinations.add(e.destRepoKey.get());
+          wildcardDestinations.add(configEntry.destRepoKey.get());
         }
-        sources.add(e.srcRepoKey.get());
-        destinations.add(e.destRepoKey.get());
 
-        newConf.add(e);
+
+        sources.add(configEntry.srcRepoKey.get());
+        destinations.add(configEntry.destRepoKey.get());
+
+        newConf.add(configEntry);
 
       } catch (ConfigInvalidException e) {
         log.error("ConfigInvalidException: " + e.toString());
@@ -205,7 +216,7 @@
     return newConf;
   }
 
-  private static ConfigEntry newConfigEntry(Config cfg, String name) throws ConfigInvalidException {
+  private ConfigEntry newConfigEntry(Config cfg, String name) throws ConfigInvalidException {
     String[] parts = name.split(":");
     if (parts.length != 2) {
       throw new ConfigInvalidException(
@@ -262,6 +273,16 @@
     e.destBranch = destRef.substring(REFS_HEADS.length());
 
     e.recordSubmoduleLabels = cfg.getBoolean(SECTION_NAME, name, "recordSubmoduleLabels", false);
+
+
+    try {
+      String newPath = canonicalWebUrl.getPath() + "/" + e.srcRepoKey.toString();
+      e.srcRepoUrl =
+          new URIBuilder(canonicalWebUrl).setPath(newPath).build().normalize();
+    } catch (URISyntaxException exception) {
+      throw new ConfigInvalidException("could not build src URL", exception);
+    }
+
     return e;
   }
 
@@ -378,9 +399,7 @@
       cmd.setInputStream(manifestStream);
       cmd.setRecommendShallow(true);
       cmd.setRemoteReader(reader);
-
-      // Is this the right URL?
-      cmd.setURI(canonicalWebUrl);
+      cmd.setURI(c.srcRepoUrl.toString());
 
       // Must setup a included file reader; the default is to read the file from the filesystem
       // otherwise, which would leak data from the serving machine.
diff --git a/src/test/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestIT.java b/src/test/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestIT.java
index e4f0533..9a6cbe3 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestIT.java
@@ -15,6 +15,7 @@
 package com.googlesource.gerrit.plugins.supermanifest;
 
 import static com.google.common.truth.Truth.assertThat;
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.Assert.fail;
 
 import com.google.gerrit.acceptance.GitUtil;
@@ -25,8 +26,11 @@
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
+import java.net.URI;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.BlobBasedConfig;
+import org.eclipse.jgit.lib.Config;
 import org.junit.Test;
 
 @TestPlugin(
@@ -36,10 +40,10 @@
 public class SuperManifestIT extends LightweightPluginDaemonTest {
   Project.NameKey[] testRepoKeys;
 
-  void setupTestRepos() throws Exception {
+  void setupTestRepos(String prefix) throws Exception {
     testRepoKeys = new Project.NameKey[2];
     for (int i = 0; i < 2; i++) {
-      testRepoKeys[i] = createProject("project" + i);
+      testRepoKeys[i] = createProject(prefix + i);
 
       TestRepository<InMemoryRepository> repo = cloneProject(testRepoKeys[i], admin);
 
@@ -63,7 +67,7 @@
 
   @Test
   public void basicFunctionalityWorks() throws Exception {
-    setupTestRepos();
+    setupTestRepos("project");
 
     // Make sure the manifest exists so the configuration loads successfully.
     Project.NameKey manifestKey = createProject("manifest");
@@ -83,13 +87,13 @@
             + "  srcPath = default.xml\n");
 
     String remoteXml = "  <remote name=\"origin\" fetch=\"" + canonicalWebUrl.get() + "\" />\n";
-    String originXml = "  <default remote=\"origin\" revision=\"refs/heads/master\" />\n";
+    String defaultXml = "  <default remote=\"origin\" revision=\"refs/heads/master\" />\n";
     // XML change will trigger commit to superproject.
     String xml =
         "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
             + "<manifest>\n"
             + remoteXml
-            + originXml
+            + defaultXml
             + "  <project name=\""
             + testRepoKeys[0].get()
             + "\" path=\"project1\" />\n"
@@ -113,7 +117,7 @@
         "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
             + "<manifest>\n"
             + remoteXml
-            + originXml
+            + defaultXml
             + "  <project name=\""
             + testRepoKeys[0].get()
             + "\" path=\"project1\" />\n"
@@ -146,7 +150,7 @@
         "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
             + "<manifest>\n"
             + remoteXml
-            + originXml
+            + defaultXml
             + "  <project name=\""
             + testRepoKeys[1].get()
             + "\" path=\"project3\" />\n"
@@ -163,7 +167,7 @@
 
   @Test
   public void wildcardDestBranchWorks() throws Exception {
-    setupTestRepos();
+    setupTestRepos("project");
 
     // Make sure the manifest exists so the configuration loads successfully.
     Project.NameKey manifestKey = createProject("manifest");
@@ -237,7 +241,7 @@
 
   @Test
   public void manifestIncludesOtherManifest() throws Exception {
-    setupTestRepos();
+    setupTestRepos("project");
 
     String remoteXml = "  <remote name=\"origin\" fetch=\"" + canonicalWebUrl.get() + "\" />\n";
     String originXml = "  <default remote=\"origin\" revision=\"refs/heads/master\" />\n";
@@ -294,5 +298,65 @@
     assertThat(branch.file("project1").getContentType()).isEqualTo("x-git/gitlink; charset=UTF-8");
   }
 
+  @Test
+  public void relativeFetch() throws Exception {
+    // Test the setup that Android uses, where the "fetch" field is relative to the location of the
+    // manifest repo.
+    setupTestRepos("platform/project");
+
+    // The test framework adds more cruft to the prefix.
+    String realPrefix = testRepoKeys[0].get().split("/")[0];
+
+    Project.NameKey manifestKey = createProject(realPrefix + "/manifest");
+    TestRepository<InMemoryRepository> manifestRepo = cloneProject(manifestKey, admin);
+
+    Project.NameKey superKey = createProject("superproject");
+
+    TestRepository<InMemoryRepository> superRepo = cloneProject(superKey, admin);
+
+    pushConfig(
+        "[superproject \""
+            + superKey.get()
+            + ":refs/heads/destbranch\"]\n"
+            + "  srcRepo = "
+            + manifestKey.get()
+            + "\n"
+            + "  srcRef = refs/heads/srcbranch\n"
+            + "  srcPath = default.xml\n");
+
+    String url = canonicalWebUrl.get();
+    String remoteXml = "  <remote name=\"origin\" fetch=\"..\" review=\"" + url + "\" />\n";
+    String defaultXml = "  <default remote=\"origin\" revision=\"refs/heads/master\" />\n";
+
+    String xml =
+        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+            + "<manifest>\n"
+            + remoteXml
+            + defaultXml
+            + "  <project name=\""
+            + testRepoKeys[0].get()
+            + "\" path=\"path1\" />\n"
+            + "</manifest>\n";
+    pushFactory
+        .create(db, admin.getIdent(), manifestRepo, "Subject", "default.xml", xml)
+        .to("refs/heads/srcbranch")
+        .assertOkStatus();
+
+    BranchApi branch = gApi.projects().name(superKey.get()).branch("refs/heads/destbranch");
+    assertThat(branch.file("path1").getContentType()).isEqualTo("x-git/gitlink; charset=UTF-8");
+
+    Config base = new Config();
+    BlobBasedConfig cfg =
+        new BlobBasedConfig(base, branch.file(".gitmodules").asString().getBytes(UTF_8));
+
+    String subUrl = cfg.getString("submodule", "path1", "url");
+
+    // URL is valid.
+    URI.create(subUrl);
+
+    // URL is clean.
+    assertThat(subUrl).doesNotContain("../");
+  }
+
   // TODO - should add tests for all the error handling in configuration parsing?
 }