Cleanup and test trailing slash handling in ManifestParser
This is a workaround for
https://bugs.openjdk.java.net/browse/JDK-4666701.
Change-Id: Idd04657e8d95a841d72230f8881b6b899daadbc2
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/ManifestParserTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/ManifestParserTest.java
index 4cd89e0..c9673a6 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/ManifestParserTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/ManifestParserTest.java
@@ -48,6 +48,7 @@
import java.io.ByteArrayInputStream;
import java.io.IOException;
+import java.net.URI;
import java.util.HashSet;
import java.util.Set;
@@ -143,4 +144,19 @@ public void testManifestParserWithMissingFetchOnRemote() throws Exception {
.contains("is missing fetch attribute"));
}
}
+
+ void testNormalize(String in, String want) {
+ URI got = ManifestParser.normalizeEmptyPath(URI.create(in));
+ if (!got.toString().equals(want)) {
+ fail(String.format("normalize(%s) = %s want %s", in, got, want));
+ }
+ }
+
+ @Test
+ public void testNormalizeEmptyPath() {
+ testNormalize("http://a.b", "http://a.b/");
+ testNormalize("http://a.b/", "http://a.b/");
+ testNormalize("", "");
+ testNormalize("a/b", "a/b");
+ }
}
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java
index 03ed824..9cf4569 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java
@@ -48,15 +48,28 @@
import static org.junit.Assert.assertTrue;
import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.FileReader;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.api.errors.InvalidRemoteException;
+import org.eclipse.jgit.api.errors.RefNotFoundException;
import org.eclipse.jgit.junit.JGitTestUtil;
import org.eclipse.jgit.junit.RepositoryTestCase;
+import org.eclipse.jgit.lib.BlobBasedConfig;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.util.FS;
import org.junit.Test;
@@ -124,6 +137,108 @@ public void setUp() throws Exception {
resolveRelativeUris();
}
+ class IndexedRepos implements RepoCommand.RemoteReader {
+ Map<String, Repository> uriRepoMap;
+ IndexedRepos() {
+ uriRepoMap = new HashMap<>();
+ }
+
+ void put(String u, Repository r) {
+ uriRepoMap.put(u, r);
+ }
+
+ @Override
+ public ObjectId sha1(String uri, String refname) throws GitAPIException {
+ if (!uriRepoMap.containsKey(uri)) {
+ return null;
+ }
+
+ Repository r = uriRepoMap.get(uri);
+ try {
+ Ref ref = r.findRef(refname);
+ if (ref == null) return null;
+
+ ref = r.peel(ref);
+ ObjectId id = ref.getObjectId();
+ return id;
+ } catch (IOException e) {
+ throw new InvalidRemoteException("", e);
+ }
+ }
+
+ @Override
+ public byte[] readFile(String uri, String refName, String path)
+ throws GitAPIException, IOException {
+ Repository repo = uriRepoMap.get(uri);
+
+ String idStr = refName + ":" + path;
+ ObjectId id = repo.resolve(idStr);
+ if (id == null) {
+ throw new RefNotFoundException(
+ String.format("repo %s does not have %s", repo.toString(), idStr));
+ }
+ try (ObjectReader reader = repo.newObjectReader()) {
+ return reader.open(id).getCachedBytes(Integer.MAX_VALUE);
+ }
+ }
+ }
+
+ @Test
+ public void absoluteRemoteURL() throws Exception {
+ Repository child =
+ Git.cloneRepository().setURI(groupADb.getDirectory().toURI().toString())
+ .setDirectory(createUniqueTestGitDir(true))
+ .setBare(true).call().getRepository();
+ Repository dest = Git.cloneRepository()
+ .setURI(db.getDirectory().toURI().toString()).setDirectory(createUniqueTestGitDir(true))
+ .setBare(true).call().getRepository();
+ String abs = "https://chromium.googlesource.com";
+ String repoUrl = "https://chromium.googlesource.com/chromium/src";
+ boolean fetchSlash = false;
+ boolean baseSlash = false;
+ do {
+ do {
+ String fetchUrl = fetchSlash ? abs + "/" : abs;
+ String baseUrl = baseSlash ? abs + "/" : abs;
+
+ StringBuilder xmlContent = new StringBuilder();
+ xmlContent.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n")
+ .append("<manifest>")
+ .append("<remote name=\"origin\" fetch=\"" + fetchUrl + "\" />")
+ .append("<default revision=\"master\" remote=\"origin\" />")
+ .append("<project path=\"src\" name=\"chromium/src\" />")
+ .append("</manifest>");
+ RepoCommand cmd = new RepoCommand(dest);
+
+ IndexedRepos repos = new IndexedRepos();
+ repos.put(repoUrl, child);
+
+ RevCommit commit = cmd
+ .setInputStream(new ByteArrayInputStream(xmlContent.toString().getBytes(StandardCharsets.UTF_8)))
+ .setRemoteReader(repos)
+ .setURI(baseUrl)
+ .setRecordRemoteBranch(true)
+ .setRecordSubmoduleLabels(true)
+ .call();
+
+ String idStr = commit.getId().name() + ":" + ".gitmodules";
+ ObjectId modId = dest.resolve(idStr);
+
+ try (ObjectReader reader = dest.newObjectReader()) {
+ byte[] bytes = reader.open(modId).getCachedBytes(Integer.MAX_VALUE);
+ Config base = new Config();
+ BlobBasedConfig cfg = new BlobBasedConfig(base, bytes);
+ String subUrl = cfg.getString("submodule", "src", "url");
+ assertEquals("https://chromium.googlesource.com/chromium/src", subUrl);
+ }
+ fetchSlash = !fetchSlash;
+ } while (fetchSlash);
+ baseSlash = !baseSlash;
+ } while (baseSlash);
+ child.close();
+ dest.close();
+ }
+
@Test
public void testAddRepoManifest() throws Exception {
StringBuilder xmlContent = new StringBuilder();
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/ManifestParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/ManifestParser.java
index 73b2e6d..94c8e43 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/ManifestParser.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/ManifestParser.java
@@ -46,6 +46,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.net.URI;
+import java.net.URISyntaxException;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Collections;
@@ -124,12 +125,7 @@ public ManifestParser(IncludedFileReader includedReader, String filename,
this.filename = filename;
this.defaultBranch = defaultBranch;
this.rootRepo = rootRepo;
-
- // Strip trailing '/' to match repo behavior.
- while (baseUrl.endsWith("/")) { //$NON-NLS-1$
- baseUrl = baseUrl.substring(0, baseUrl.length()-1);
- }
- this.baseUrl = URI.create(baseUrl);
+ this.baseUrl = normalizeEmptyPath(URI.create(baseUrl));
plusGroups = new HashSet<>();
minusGroups = new HashSet<>();
@@ -257,7 +253,7 @@ public void endDocument() throws SAXException {
return;
// Only do the following after we finished reading everything.
- Map<String, String> remoteUrls = new HashMap<>();
+ Map<String, URI> remoteUrls = new HashMap<>();
if (defaultRevision == null && defaultRemote != null) {
Remote remote = remotes.get(defaultRemote);
if (remote != null) {
@@ -287,20 +283,18 @@ public void endDocument() throws SAXException {
revision = r.revision;
}
}
- String remoteUrl = remoteUrls.get(remote);
+ URI remoteUrl = remoteUrls.get(remote);
if (remoteUrl == null) {
String fetch = remotes.get(remote).fetch;
if (fetch == null) {
throw new SAXException(MessageFormat
.format(RepoText.get().errorNoFetch, remote));
}
- remoteUrl = baseUrl.resolve(fetch).toString();
- if (!remoteUrl.endsWith("/")) //$NON-NLS-1$
- remoteUrl = remoteUrl + "/"; //$NON-NLS-1$
+ remoteUrl = normalizeEmptyPath(baseUrl.resolve(fetch));
remoteUrls.put(remote, remoteUrl);
}
- proj.setUrl(remoteUrl + proj.getName())
- .setDefaultRevision(revision);
+ proj.setUrl(remoteUrl.resolve(proj.getName()).toString())
+ .setDefaultRevision(revision);
}
filteredProjects.addAll(projects);
@@ -308,6 +302,23 @@ public void endDocument() throws SAXException {
removeOverlaps();
}
+ static URI normalizeEmptyPath(URI u) {
+ // URI.create("scheme://host").resolve("a/b") => "scheme://hosta/b"
+ // That seems like bug https://bugs.openjdk.java.net/browse/JDK-4666701.
+ // We workaround this by special casing the empty path case.
+ if (u.getHost() != null && !u.getHost().isEmpty() &&
+ (u.getPath() == null || u.getPath().isEmpty())) {
+ try {
+ return new URI(u.getScheme(),
+ u.getUserInfo(), u.getHost(), u.getPort(),
+ "/", u.getQuery(), u.getFragment()); //$NON-NLS-1$
+ } catch (URISyntaxException x) {
+ throw new IllegalArgumentException(x.getMessage(), x);
+ }
+ }
+ return u;
+ }
+
/**
* Getter for projects.
*
