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 c97b31880..c9673a688 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 @@ -44,12 +44,16 @@ package org.eclipse.jgit.gitrepo; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.net.URI; import java.util.HashSet; import java.util.Set; import org.junit.Test; +import org.xml.sax.SAXException; public class ManifestParserTest { @@ -110,4 +114,49 @@ public class ManifestParserTest { "Filtered projects shouldn't contain any unexpected results", results.isEmpty()); } + + @Test + public void testManifestParserWithMissingFetchOnRemote() throws Exception { + String baseUrl = "https://git.google.com/"; + StringBuilder xmlContent = new StringBuilder(); + xmlContent.append("\n") + .append("") + .append("") + .append("") + .append("") + .append("") + .append("") + .append("").append(""); + + ManifestParser parser = new ManifestParser(null, null, "master", + baseUrl, null, null); + try { + parser.read(new ByteArrayInputStream( + xmlContent.toString().getBytes(UTF_8))); + fail("ManifestParser did not throw exception for missing fetch"); + } catch (IOException e) { + assertTrue(e.getCause() instanceof SAXException); + assertTrue(e.getCause().getMessage() + .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 03ed82443..9cf4569d6 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.assertNull; 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 class RepoCommandTest extends RepositoryTestCase { resolveRelativeUris(); } + class IndexedRepos implements RepoCommand.RemoteReader { + Map 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("\n") + .append("") + .append("") + .append("") + .append("") + .append(""); + 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/resources/org/eclipse/jgit/gitrepo/internal/RepoText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/gitrepo/internal/RepoText.properties index 7443ad32f..e942038a3 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/gitrepo/internal/RepoText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/gitrepo/internal/RepoText.properties @@ -3,6 +3,7 @@ errorIncludeFile=Error: unable to read include file {0} errorIncludeNotImplemented=Error: tag not supported as no callback defined. errorNoDefault=Error: no default remote in manifest file. errorNoDefaultFilename=Error: no default remote in manifest file {0}. +errorNoFetch=Error: remote {0} is missing fetch attribute. errorParsingManifestFile=Error occurred during parsing manifest file. errorRemoteUnavailable=Error remote {0} is unavailable. invalidManifest=Invalid manifest. 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 26210ecbb..94c8e437c 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.FileInputStream; 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 class ManifestParser extends DefaultHandler { 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 class ManifestParser extends DefaultHandler { return; // Only do the following after we finished reading everything. - Map remoteUrls = new HashMap<>(); + Map remoteUrls = new HashMap<>(); if (defaultRevision == null && defaultRemote != null) { Remote remote = remotes.get(defaultRemote); if (remote != null) { @@ -287,15 +283,18 @@ public class ManifestParser extends DefaultHandler { revision = r.revision; } } - String remoteUrl = remoteUrls.get(remote); + URI remoteUrl = remoteUrls.get(remote); if (remoteUrl == null) { - remoteUrl = baseUrl.resolve(remotes.get(remote).fetch).toString(); - if (!remoteUrl.endsWith("/")) //$NON-NLS-1$ - remoteUrl = remoteUrl + "/"; //$NON-NLS-1$ + String fetch = remotes.get(remote).fetch; + if (fetch == null) { + throw new SAXException(MessageFormat + .format(RepoText.get().errorNoFetch, remote)); + } + 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); @@ -303,6 +302,23 @@ public class ManifestParser extends DefaultHandler { 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. * diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/internal/RepoText.java b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/internal/RepoText.java index 36b6e3aac..02a2565bd 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/internal/RepoText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/internal/RepoText.java @@ -64,6 +64,7 @@ public class RepoText extends TranslationBundle { /***/ public String errorIncludeNotImplemented; /***/ public String errorNoDefault; /***/ public String errorNoDefaultFilename; + /***/ public String errorNoFetch; /***/ public String errorParsingManifestFile; /***/ public String errorRemoteUnavailable; /***/ public String invalidManifest;