ManifestParser: Throw exception if remote does not have fetch attribute

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>

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 c97b318800c18874b94c83ea9ab5097abbf3359f..4cd89e0aa9197d8bfce12172137ecf3fdc90730e 100644 (file)
--- 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,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"));
+               }
+       }
 }

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 7443ad32f03c74ffcadbd1c94be58d0ef6cb3294..e942038a3d4b457135ff9370dc9ff588f0330fcc 100644 (file)
--- 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: <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.

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 26210ecbb955888ea655805f41b537195fd93b96..73b2e6dcfaf984ba51eb984d0bf5b5f50201e294 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/ManifestParser.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/ManifestParser.java
@@ -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);

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 36b6e3aac4559d910ec12887bcd2ca0165d3489c..02a2565bdd1f28f138a60517cf4e3ca61a66d3a6 100644 (file)
--- 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;