Merge branch 'stable-4.10' into stable-4.11

* stable-4.10:
  Prepare 4.9.7-SNAPSHOT builds
  JGit v4.9.6.201810051924-r
  Prepare 4.7.6-SNAPSHOT builds
  JGit v4.7.5.201810051826-r
  BaseReceivePack: Validate incoming .gitmodules files
  ObjectChecker: Report .gitmodules files found in the pack
  SubmoduleAddCommand: Reject submodule URIs that look like cli options

Change-Id: Ibd759f5d425f714e79b3137ff8e5b0f989933de0
Signed-off-by: Jonathan Nieder <jrn@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

3 files changed, 213 insertions, 2 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectCheckerTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectCheckerTest.java
index f1c62d4be2..f6879b968b 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectCheckerTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectCheckerTest.java
@@ -837,6 +837,112 @@ public class ObjectCheckerTest {
 	}
 
 	@Test
+	public void testValidTreeWithGitmodules() throws CorruptObjectException {
+		ObjectId treeId = ObjectId
+				.fromString("0123012301230123012301230123012301230123");
+		StringBuilder b = new StringBuilder();
+		ObjectId blobId = entry(b, "100644 .gitmodules");
+
+		byte[] data = encodeASCII(b.toString());
+		checker.checkTree(treeId, data);
+		assertEquals(1, checker.getGitsubmodules().size());
+		assertEquals(treeId, checker.getGitsubmodules().get(0).getTreeId());
+		assertEquals(blobId, checker.getGitsubmodules().get(0).getBlobId());
+	}
+
+	/*
+	 * Windows case insensitivity and long file name handling
+	 * means that .gitmodules has many synonyms.
+	 *
+	 * Examples inspired by git.git's t/t0060-path-utils.sh, by
+	 * Johannes Schindelin and Congyi Wu.
+	 */
+	@Test
+	public void testNTFSGitmodules() throws CorruptObjectException {
+		for (String gitmodules : new String[] {
+			".GITMODULES",
+			".gitmodules",
+			".Gitmodules",
+			".gitmoduleS",
+			"gitmod~1",
+			"GITMOD~1",
+			"gitmod~4",
+			"GI7EBA~1",
+			"gi7eba~9",
+			"GI7EB~10",
+			"GI7E~123",
+			"~1000000",
+			"~9999999"
+		}) {
+			checker = new ObjectChecker(); // Reset the ObjectChecker state.
+			checker.setSafeForWindows(true);
+			ObjectId treeId = ObjectId
+					.fromString("0123012301230123012301230123012301230123");
+			StringBuilder b = new StringBuilder();
+			ObjectId blobId = entry(b, "100644 " + gitmodules);
+
+			byte[] data = encodeASCII(b.toString());
+			checker.checkTree(treeId, data);
+			assertEquals(1, checker.getGitsubmodules().size());
+			assertEquals(treeId, checker.getGitsubmodules().get(0).getTreeId());
+			assertEquals(blobId, checker.getGitsubmodules().get(0).getBlobId());
+		}
+	}
+
+	@Test
+	public void testNotGitmodules() throws CorruptObjectException {
+		for (String notGitmodules : new String[] {
+			".gitmodu",
+			".gitmodules oh never mind",
+		}) {
+			checker = new ObjectChecker(); // Reset the ObjectChecker state.
+			checker.setSafeForWindows(true);
+			ObjectId treeId = ObjectId
+					.fromString("0123012301230123012301230123012301230123");
+			StringBuilder b = new StringBuilder();
+			entry(b, "100644 " + notGitmodules);
+
+			byte[] data = encodeASCII(b.toString());
+			checker.checkTree(treeId, data);
+			assertEquals(0, checker.getGitsubmodules().size());
+		}
+	}
+
+	/*
+	 * TODO HFS: match ".gitmodules" case-insensitively, after stripping out
+	 * certain zero-length Unicode code points that HFS+ strips out
+	 */
+
+	@Test
+	public void testValidTreeWithGitmodulesUppercase()
+			throws CorruptObjectException {
+		ObjectId treeId = ObjectId
+				.fromString("0123012301230123012301230123012301230123");
+		StringBuilder b = new StringBuilder();
+		ObjectId blobId = entry(b, "100644 .GITMODULES");
+
+		byte[] data = encodeASCII(b.toString());
+		checker.setSafeForWindows(true);
+		checker.checkTree(treeId, data);
+		assertEquals(1, checker.getGitsubmodules().size());
+		assertEquals(treeId, checker.getGitsubmodules().get(0).getTreeId());
+		assertEquals(blobId, checker.getGitsubmodules().get(0).getBlobId());
+	}
+
+	@Test
+	public void testTreeWithInvalidGitmodules() throws CorruptObjectException {
+		ObjectId treeId = ObjectId
+				.fromString("0123012301230123012301230123012301230123");
+		StringBuilder b = new StringBuilder();
+		entry(b, "100644 .gitmodulez");
+
+		byte[] data = encodeASCII(b.toString());
+		checker.checkTree(treeId, data);
+		checker.setSafeForWindows(true);
+		assertEquals(0, checker.getGitsubmodules().size());
+	}
+
+	@Test
 	public void testNullSha1InTreeEntry() throws CorruptObjectException {
 		byte[] data = concat(
 				encodeASCII("100644 A"), new byte[] { '\0' },
@@ -1607,11 +1713,20 @@ public class ObjectCheckerTest {
 		checker.checkTree(encodeASCII(b.toString()));
 	}
 
-	private static void entry(StringBuilder b, final String modeName) {
+	/*
+	 * Returns the id generated for the entry
+	 */
+	private static ObjectId entry(StringBuilder b, String modeName) {
+		byte[] id = new byte[OBJECT_ID_LENGTH];
+
 		b.append(modeName);
 		b.append('\0');
-		for (int i = 0; i < OBJECT_ID_LENGTH; i++)
+		for (int i = 0; i < OBJECT_ID_LENGTH; i++) {
 			b.append((char) i);
+			id[i] = (byte) i;
+		}
+
+		return ObjectId.fromRaw(id);
 	}
 
 	private void assertCorrupt(String msg, int type, StringBuilder b) {
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleAddTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleAddTest.java
index f42dd02814..93f47090a3 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleAddTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleAddTest.java
@@ -183,6 +183,37 @@ public class SubmoduleAddTest extends RepositoryTestCase {
 	}
 
 	@Test
+	public void addSubmoduleWithInvalidPath() throws Exception {
+		SubmoduleAddCommand command = new SubmoduleAddCommand(db);
+		command.setPath("-invalid-path");
+		// TODO(ms) set name to a valid value in 5.1.0 and adapt expected
+		// message below
+		command.setURI("http://example.com/repo/x.git");
+		try {
+			command.call().close();
+			fail("Exception not thrown");
+		} catch (IllegalArgumentException e) {
+			// TODO(ms) should check for submodule path, but can't set name
+			// before 5.1.0
+			assertEquals("Invalid submodule name '-invalid-path'",
+					e.getMessage());
+		}
+	}
+
+	@Test
+	public void addSubmoduleWithInvalidUri() throws Exception {
+		SubmoduleAddCommand command = new SubmoduleAddCommand(db);
+		command.setPath("valid-path");
+		command.setURI("-upstream");
+		try {
+			command.call().close();
+			fail("Exception not thrown");
+		} catch (IllegalArgumentException e) {
+			assertEquals("Invalid submodule URL '-upstream'", e.getMessage());
+		}
+	}
+
+	@Test
 	public void addSubmoduleWithRelativeUri() throws Exception {
 		try (Git git = new Git(db)) {
 			writeTrashFile("file.txt", "content");
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java
index 8ef87cb3c1..3836b7da4e 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java
@@ -466,6 +466,71 @@ public class ReceivePackAdvertiseRefsHookTest extends LocalDiskRepositoryTestCas
 	}
 
 	@Test
+	public void testIncludesInvalidGitmodules() throws Exception {
+		final TemporaryBuffer.Heap inBuf = setupSourceRepoInvalidGitmodules();
+		final TemporaryBuffer.Heap outBuf = new TemporaryBuffer.Heap(1024);
+		final ReceivePack rp = new ReceivePack(dst);
+		rp.setCheckReceivedObjects(true);
+		rp.setCheckReferencedObjectsAreReachable(true);
+		rp.setAdvertiseRefsHook(new HidePrivateHook());
+		try {
+			receive(rp, inBuf, outBuf);
+			fail("Expected UnpackException");
+		} catch (UnpackException failed) {
+			Throwable err = failed.getCause();
+			assertTrue(err instanceof IOException);
+		}
+
+		final PacketLineIn r = asPacketLineIn(outBuf);
+		String master = r.readString();
+		int nul = master.indexOf('\0');
+		assertTrue("has capability list", nul > 0);
+		assertEquals(B.name() + ' ' + R_MASTER, master.substring(0, nul));
+		assertSame(PacketLineIn.END, r.readString());
+
+		String errorLine = r.readString();
+		System.out.println(errorLine);
+		assertTrue(errorLine.startsWith(
+				"unpack error Invalid submodule URL '-"));
+		assertEquals("ng refs/heads/s n/a (unpacker error)", r.readString());
+		assertSame(PacketLineIn.END, r.readString());
+	}
+
+	private TemporaryBuffer.Heap setupSourceRepoInvalidGitmodules()
+			throws IOException, Exception, MissingObjectException {
+		String fakeGitmodules = new StringBuilder()
+				.append("[submodule \"test\"]\n")
+				.append("    path = xlib\n")
+				.append("    url = https://example.com/repo/xlib.git\n\n")
+				.append("[submodule \"test2\"]\n")
+				.append("    path = zlib\n")
+				.append("    url = -upayload.sh\n")
+				.toString();
+
+		TestRepository<Repository> s = new TestRepository<>(src);
+		RevBlob blob = s.blob(fakeGitmodules);
+		RevCommit N = s.commit().parent(B)
+				.add(".gitmodules", blob).create();
+		RevTree t = s.parseBody(N).getTree();
+
+		final TemporaryBuffer.Heap pack = new TemporaryBuffer.Heap(1024);
+		packHeader(pack, 3);
+		copy(pack, src.open(N));
+		copy(pack, src.open(t));
+		copy(pack, src.open(blob));
+		digest(pack);
+
+		final TemporaryBuffer.Heap inBuf = new TemporaryBuffer.Heap(1024);
+		final PacketLineOut inPckLine = new PacketLineOut(inBuf);
+		inPckLine.writeString(ObjectId.zeroId().name() + ' ' + N.name() + ' '
+				+ "refs/heads/s" + '\0'
+				+ BasePackPushConnection.CAPABILITY_REPORT_STATUS);
+		inPckLine.end();
+		pack.writeTo(inBuf, PM);
+		return inBuf;
+	}
+
+	@Test
 	public void testUsingUnknownTreeFails() throws Exception {
 		TestRepository<Repository> s = new TestRepository<>(src);
 		RevCommit N = s.commit().parent(B).add("q", s.blob("a")).create();