In the repo manifest documentation [1] the fetch attribute is marked as "#REQUIRED". If the fetch attribute is not specified, this would previously result in NullPointerException. Throw a SAXException instead. [1] https://gerrit.googlesource.com/git-repo/+/master/docs/manifest-format.txt Change-Id: Ib8ed8cee6074fe6bf8f9ac6fc7a1664a547d2d49 Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>tags/v4.8.0.201705170830-rc1
@@ -44,12 +44,15 @@ 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.util.HashSet; | |||
import java.util.Set; | |||
import org.junit.Test; | |||
import org.xml.sax.SAXException; | |||
public class ManifestParserTest { | |||
@@ -110,4 +113,34 @@ 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("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n") | |||
.append("<manifest>") | |||
.append("<remote name=\"remote1\" />") | |||
.append("<default revision=\"master\" remote=\"remote1\" />") | |||
.append("<project path=\"foo\" name=\"").append("foo") | |||
.append("\" groups=\"a,test\" />") | |||
.append("<project path=\"bar\" name=\"").append("bar") | |||
.append("\" groups=\"notdefault\" />") | |||
.append("<project path=\"foo/a\" name=\"").append("a") | |||
.append("\" groups=\"a\" />") | |||
.append("<project path=\"b\" name=\"").append("b") | |||
.append("\" groups=\"b\" />").append("</manifest>"); | |||
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")); | |||
} | |||
} | |||
} |
@@ -3,6 +3,7 @@ errorIncludeFile=Error: unable to read include file {0} | |||
errorIncludeNotImplemented=Error: <include> 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. |
@@ -289,7 +289,12 @@ public class ManifestParser extends DefaultHandler { | |||
} | |||
String remoteUrl = remoteUrls.get(remote); | |||
if (remoteUrl == null) { | |||
remoteUrl = baseUrl.resolve(remotes.get(remote).fetch).toString(); | |||
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$ | |||
remoteUrls.put(remote, remoteUrl); |
@@ -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; |