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>

1 files changed, 24 insertions, 13 deletions

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 73b2e6dcfa..94c8e437c3 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<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 class ManifestParser extends DefaultHandler {
 					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 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.
 	 *