Merge branch 'stable-5.1'

* stable-5.1:
  Prepare 5.1.3-SNAPSHOT builds
  JGit v5.1.2.201810061102-r
  Prepare 4.11.5-SNAPSHOT builds
  JGit v4.11.4.201810060650-r
  Fix configuration of maven-javadoc-plugin
  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
  Revert "Configure WindowCache settings to use in JGit CLI"

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

14 files changed, 680 insertions, 57 deletions

diff --git a/org.eclipse.jgit.junit/.settings/.api_filters b/org.eclipse.jgit.junit/.settings/.api_filters
deleted file mode 100644
index e5de787fa6..0000000000
--- a/org.eclipse.jgit.junit/.settings/.api_filters
+++ /dev/null
@@ -1,35 +0,0 @@
-<?xml version="1.0" encoding="UTF-8" standalone="no"?>
-<component id="org.eclipse.jgit.junit" version="2">
-    <resource path="src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java" type="org.eclipse.jgit.junit.LocalDiskRepositoryTestCase">
-        <filter comment="Don't care about non-API in tests." id="643842064">
-            <message_arguments>
-                <message_argument value="FileRepository"/>
-                <message_argument value="LocalDiskRepositoryTestCase"/>
-                <message_argument value="createBareRepository()"/>
-            </message_arguments>
-        </filter>
-        <filter comment="Don't care about non-API in tests." id="643842064">
-            <message_arguments>
-                <message_argument value="FileRepository"/>
-                <message_argument value="LocalDiskRepositoryTestCase"/>
-                <message_argument value="createRepository(boolean, boolean)"/>
-            </message_arguments>
-        </filter>
-        <filter comment="Don't care about non-API in tests." id="643842064">
-            <message_arguments>
-                <message_argument value="FileRepository"/>
-                <message_argument value="LocalDiskRepositoryTestCase"/>
-                <message_argument value="createWorkRepository()"/>
-            </message_arguments>
-        </filter>
-    </resource>
-    <resource path="src/org/eclipse/jgit/junit/RepositoryTestCase.java" type="org.eclipse.jgit.junit.RepositoryTestCase">
-        <filter comment="Don't care about non-API in tests." id="627060751">
-            <message_arguments>
-                <message_argument value="FileRepository"/>
-                <message_argument value="RepositoryTestCase"/>
-                <message_argument value="db"/>
-            </message_arguments>
-        </filter>
-    </resource>
-</component>
diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Main.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Main.java
index 5ed286baa9..ad10ec9e44 100644
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Main.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Main.java
@@ -71,7 +71,6 @@ import org.eclipse.jgit.lib.RepositoryBuilder;
 import org.eclipse.jgit.pgm.internal.CLIText;
 import org.eclipse.jgit.pgm.opt.CmdLineParser;
 import org.eclipse.jgit.pgm.opt.SubcommandHandler;
-import org.eclipse.jgit.storage.file.WindowCacheConfig;
 import org.eclipse.jgit.transport.HttpTransport;
 import org.eclipse.jgit.transport.http.apache.HttpClientConnectionFactory;
 import org.eclipse.jgit.util.CachedAuthenticator;
@@ -106,19 +105,10 @@ public class Main {
 
 	private ExecutorService gcExecutor;
 
-	private static final int MB = 1024 * 1024;
-
 	/**
 	 * <p>Constructor for Main.</p>
 	 */
 	public Main() {
-		final WindowCacheConfig c = new WindowCacheConfig();
-		c.setPackedGitMMAP(true);
-		c.setPackedGitWindowSize(8 * 1024);
-		c.setPackedGitLimit(10 * MB);
-		c.setDeltaBaseCacheLimit(10 * MB);
-		c.setStreamFileThreshold(50 * MB);
-		c.install();
 		HttpTransport.setConnectionFactory(new HttpClientConnectionFactory());
 		BuiltinLFS.register();
 		gcExecutor = Executors.newSingleThreadExecutor(new ThreadFactory() {
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 7ea504374c..531c918e9b 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, 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 0676eab2d6..1ff64a2e28 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
@@ -226,6 +226,34 @@ public class SubmoduleAddTest extends RepositoryTestCase {
 	}
 
 	@Test
+	public void addSubmoduleWithInvalidPath() throws Exception {
+		SubmoduleAddCommand command = new SubmoduleAddCommand(db);
+		command.setPath("-invalid-path");
+		command.setName("sub");
+		command.setURI("http://example.com/repo/x.git");
+		try {
+			command.call().close();
+			fail("Exception not thrown");
+		} catch (IllegalArgumentException e) {
+			assertEquals("Invalid submodule path '-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 9cc6cfd393..c959f6c497 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
@@ -469,6 +469,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();
diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters
index cc68ac9a46..e4b7ba7794 100644
--- a/org.eclipse.jgit/.settings/.api_filters
+++ b/org.eclipse.jgit/.settings/.api_filters
@@ -1,5 +1,21 @@
 <?xml version="1.0" encoding="UTF-8" standalone="no"?>
 <component id="org.eclipse.jgit" version="2">
+    <resource path="src/org/eclipse/jgit/lib/GitmoduleEntry.java" type="org.eclipse.jgit.lib.GitmoduleEntry">
+        <filter id="1109393411">
+            <message_arguments>
+                <message_argument value="4.7.5"/>
+                <message_argument value="org.eclipse.jgit.lib.GitmoduleEntry"/>
+            </message_arguments>
+        </filter>
+    </resource>
+    <resource path="src/org/eclipse/jgit/lib/ObjectChecker.java" type="org.eclipse.jgit.lib.ObjectChecker">
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="4.7.5"/>
+                <message_argument value="getGitsubmodules()"/>
+            </message_arguments>
+        </filter>
+    </resource>
     <resource path="src/org/eclipse/jgit/revwalk/DepthWalk.java" type="org.eclipse.jgit.revwalk.DepthWalk">
         <filter id="404000815">
             <message_arguments>
diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index 58f4519171..3f1d21289f 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -367,6 +367,7 @@ invalidEncryption=Invalid encryption
 invalidExpandWildcard=ExpandFromSource on a refspec that can have mismatched wildcards does not make sense.
 invalidFilter=Invalid filter: {0}
 invalidGitdirRef = Invalid .git reference in file ''{0}''
+invalidGitModules=Invalid .gitmodules file
 invalidGitType=invalid git type: {0}
 invalidId=Invalid id: {0}
 invalidId0=Invalid id
@@ -379,7 +380,7 @@ invalidLineInConfigFile=Invalid line in config file
 invalidLineInConfigFileWithParam=Invalid line in config file: {0}
 invalidModeFor=Invalid mode {0} for {1} {2} in {3}.
 invalidModeForPath=Invalid mode {0} for path {1}
-invalidNameContainsDotDot=Invalid name (contains ".."): {1}
+invalidNameContainsDotDot=Invalid name (contains ".."): {0}
 invalidObject=Invalid {0} {1}: {2}
 invalidOldIdSent=invalid old id sent
 invalidPacketLineHeader=Invalid packet line header: {0}
@@ -663,7 +664,10 @@ storePushCertMultipleRefs=Store push certificate for {0} refs
 storePushCertOneRef=Store push certificate for {0}
 storePushCertReflog=Store push certificate
 submoduleExists=Submodule ''{0}'' already exists in the index
+submoduleNameInvalid=Invalid submodule name ''{0}''
 submoduleParentRemoteUrlInvalid=Cannot remove segment from remote url ''{0}''
+submodulePathInvalid=Invalid submodule path ''{0}''
+submoduleUrlInvalid=Invalid submodule URL ''{0}''
 submodulesNotSupported=Submodules are not supported
 supportOnlyPackIndexVersion2=Only support index version 2
 symlinkCannotBeWrittenAsTheLinkTarget=Symlink "{0}" cannot be written as the link target cannot be read from within Java.
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/SubmoduleAddCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/SubmoduleAddCommand.java
index b8d4468a2c..244a15686f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/SubmoduleAddCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/SubmoduleAddCommand.java
@@ -51,6 +51,7 @@ import org.eclipse.jgit.api.errors.JGitInternalException;
 import org.eclipse.jgit.api.errors.NoFilepatternException;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.internal.submodule.SubmoduleValidator;
 import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.NullProgressMonitor;
@@ -195,6 +196,14 @@ public class SubmoduleAddCommand extends
 		}
 
 		try {
+			SubmoduleValidator.assertValidSubmoduleName(name);
+			SubmoduleValidator.assertValidSubmodulePath(path);
+			SubmoduleValidator.assertValidSubmoduleUri(uri);
+		} catch (SubmoduleValidator.SubmoduleValidationException e) {
+			throw new IllegalArgumentException(e.getMessage());
+		}
+
+		try {
 			if (submoduleExists())
 				throw new JGitInternalException(MessageFormat.format(
 						JGitText.get().submoduleExists, path));
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index 96f428a9af..c11ae5a526 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -428,6 +428,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String invalidExpandWildcard;
 	/***/ public String invalidFilter;
 	/***/ public String invalidGitdirRef;
+	/***/ public String invalidGitModules;
 	/***/ public String invalidGitType;
 	/***/ public String invalidId;
 	/***/ public String invalidId0;
@@ -724,8 +725,11 @@ public class JGitText extends TranslationBundle {
 	/***/ public String storePushCertOneRef;
 	/***/ public String storePushCertReflog;
 	/***/ public String submoduleExists;
-	/***/ public String submodulesNotSupported;
+	/***/ public String submoduleNameInvalid;
 	/***/ public String submoduleParentRemoteUrlInvalid;
+	/***/ public String submodulePathInvalid;
+	/***/ public String submodulesNotSupported;
+	/***/ public String submoduleUrlInvalid;
 	/***/ public String supportOnlyPackIndexVersion2;
 	/***/ public String symlinkCannotBeWrittenAsTheLinkTarget;
 	/***/ public String systemConfigFileInvalid;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/submodule/SubmoduleValidator.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/submodule/SubmoduleValidator.java
new file mode 100644
index 0000000000..d4bba2d0d1
--- /dev/null
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/submodule/SubmoduleValidator.java
@@ -0,0 +1,180 @@
+/*
+ * Copyright (C) 2018, Google LLC.
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.eclipse.jgit.internal.submodule;
+
+import java.io.IOException;
+import java.text.MessageFormat;
+
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ConfigConstants;
+
+/**
+ * Validations for the git submodule fields (name, path, uri).
+ *
+ * Invalid values in these fields can cause security problems as reported in
+ * CVE-2018-11235 and and CVE-2018-17456
+ */
+public class SubmoduleValidator {
+
+	/**
+	 * Error validating a git submodule declaration
+	 */
+	public static class SubmoduleValidationException extends Exception {
+
+		/**
+		 * @param message
+		 *            Description of the problem
+		 */
+		public SubmoduleValidationException(String message) {
+			super(message);
+		}
+
+		private static final long serialVersionUID = 1L;
+	}
+
+	/**
+	 * Validate name for a submodule
+	 *
+	 * @param name
+	 *            name of a submodule
+	 * @throws SubmoduleValidationException
+	 *             name doesn't seem valid (detail in message)
+	 */
+	public static void assertValidSubmoduleName(String name)
+			throws SubmoduleValidationException {
+		if (name.contains("/../") || name.contains("\\..\\") //$NON-NLS-1$ //$NON-NLS-2$
+				|| name.startsWith("../") || name.startsWith("..\\") //$NON-NLS-1$ //$NON-NLS-2$
+				|| name.endsWith("/..") || name.endsWith("\\..")) { //$NON-NLS-1$ //$NON-NLS-2$
+			// Submodule names are used to store the submodule repositories
+			// under $GIT_DIR/modules. Having ".." in submodule names makes a
+			// vulnerability (CVE-2018-11235
+			// https://bugs.eclipse.org/bugs/show_bug.cgi?id=535027#c0)
+			// Reject names containing ".." path segments. We don't
+			// automatically replace these characters or canonicalize by
+			// regarding the name as a file path.
+			// Since Path class is platform dependent, we manually check '/' and
+			// '\\' patterns here.
+			throw new SubmoduleValidationException(MessageFormat
+					.format(JGitText.get().invalidNameContainsDotDot, name));
+		}
+
+		if (name.startsWith("-")) { //$NON-NLS-1$
+			throw new SubmoduleValidationException(
+					MessageFormat.format(
+							JGitText.get().submoduleNameInvalid, name));
+		}
+	}
+
+	/**
+	 * Validate URI for a submodule
+	 *
+	 * @param uri
+	 *            uri of a submodule
+	 * @throws SubmoduleValidationException
+	 *             uri doesn't seem valid
+	 */
+	public static void assertValidSubmoduleUri(String uri)
+			throws SubmoduleValidationException {
+		if (uri.startsWith("-")) { //$NON-NLS-1$
+			throw new SubmoduleValidationException(
+					MessageFormat.format(
+							JGitText.get().submoduleUrlInvalid, uri));
+		}
+	}
+
+	/**
+	 * Validate path for a submodule
+	 *
+	 * @param path
+	 *            path of a submodule
+	 * @throws SubmoduleValidationException
+	 *             path doesn't look right
+	 */
+	public static void assertValidSubmodulePath(String path)
+			throws SubmoduleValidationException {
+
+		if (path.startsWith("-")) { //$NON-NLS-1$
+			throw new SubmoduleValidationException(
+					MessageFormat.format(
+							JGitText.get().submodulePathInvalid, path));
+		}
+	}
+
+	/**
+	 * @param gitModulesContents
+	 *            Contents of a .gitmodule file. They will be parsed internally.
+	 * @throws IOException
+	 *             If the contents
+	 */
+	public static void assertValidGitModulesFile(String gitModulesContents)
+			throws IOException {
+		// Validate .gitmodules file
+		Config c = new Config();
+		try {
+			c.fromText(gitModulesContents);
+			for (String subsection : c.getSubsections(
+					ConfigConstants.CONFIG_SUBMODULE_SECTION)) {
+				String url = c.getString(
+						ConfigConstants.CONFIG_SUBMODULE_SECTION,
+						subsection, ConfigConstants.CONFIG_KEY_URL);
+				assertValidSubmoduleUri(url);
+
+				assertValidSubmoduleName(subsection);
+
+				String path = c.getString(
+						ConfigConstants.CONFIG_SUBMODULE_SECTION, subsection,
+						ConfigConstants.CONFIG_KEY_PATH);
+				assertValidSubmodulePath(path);
+			}
+		} catch (ConfigInvalidException e) {
+			throw new IOException(
+					MessageFormat.format(
+							JGitText.get().invalidGitModules,
+							e));
+		} catch (SubmoduleValidationException e) {
+			throw new IOException(e.getMessage(), e);
+		}
+	}
+}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/GitmoduleEntry.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/GitmoduleEntry.java
new file mode 100644
index 0000000000..bded527519
--- /dev/null
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/GitmoduleEntry.java
@@ -0,0 +1,86 @@
+/*
+ * Copyright (C) 2018, Google LLC.
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.eclipse.jgit.lib;
+
+import org.eclipse.jgit.lib.AnyObjectId;
+
+/**
+ * A .gitmodules file found in the pack. Store the blob of the file itself (e.g.
+ * to access its contents) and the tree where it was found (e.g. to check if it
+ * is in the root)
+ *
+ * @since 4.7.5
+ */
+public final class GitmoduleEntry {
+	private final AnyObjectId treeId;
+
+	private final AnyObjectId blobId;
+
+	/**
+	 * A record of (tree, blob) for a .gitmodule file in a pack
+	 *
+	 * @param treeId
+	 *            tree id containing a .gitmodules entry
+	 * @param blobId
+	 *            id of the blob of the .gitmodules file
+	 */
+	public GitmoduleEntry(AnyObjectId treeId, AnyObjectId blobId) {
+		// AnyObjectId's are reused, must keep a copy.
+		this.treeId = treeId.copy();
+		this.blobId = blobId.copy();
+	}
+
+	/**
+	 * @return Id of a .gitmodules file found in the pack
+	 */
+	public AnyObjectId getBlobId() {
+		return blobId;
+	}
+
+	/**
+	 * @return Id of a tree object where the .gitmodules file was found
+	 */
+	public AnyObjectId getTreeId() {
+		return treeId;
+	}
+}
\ No newline at end of file
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java
index 689659b398..59e2834cb8 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java
@@ -44,6 +44,7 @@
 
 package org.eclipse.jgit.lib;
 
+import static org.eclipse.jgit.lib.Constants.DOT_GIT_MODULES;
 import static org.eclipse.jgit.lib.Constants.OBJECT_ID_LENGTH;
 import static org.eclipse.jgit.lib.Constants.OBJECT_ID_STRING_LENGTH;
 import static org.eclipse.jgit.lib.Constants.OBJ_BAD;
@@ -84,8 +85,10 @@ import static org.eclipse.jgit.util.RawParseUtils.parseBase10;
 
 import java.text.MessageFormat;
 import java.text.Normalizer;
+import java.util.ArrayList;
 import java.util.EnumSet;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Locale;
 import java.util.Set;
 
@@ -136,6 +139,9 @@ public class ObjectChecker {
 	/** Header "tagger " */
 	public static final byte[] tagger = Constants.encodeASCII("tagger "); //$NON-NLS-1$
 
+	/** Path ".gitmodules" */
+	private static final byte[] dotGitmodules = Constants.encodeASCII(DOT_GIT_MODULES);
+
 	/**
 	 * Potential issues identified by the checker.
 	 *
@@ -199,6 +205,8 @@ public class ObjectChecker {
 	private boolean windows;
 	private boolean macosx;
 
+	private final List<GitmoduleEntry> gitsubmodules = new ArrayList<>();
+
 	/**
 	 * Enable accepting specific malformed (but not horribly broken) objects.
 	 *
@@ -684,9 +692,15 @@ public class ObjectChecker {
 				throw new CorruptObjectException(
 						JGitText.get().corruptObjectTruncatedInObjectId);
 			}
+
 			if (ObjectId.zeroId().compareTo(raw, ptr - OBJECT_ID_LENGTH) == 0) {
 				report(NULL_SHA1, id, JGitText.get().corruptObjectZeroId);
 			}
+
+			if (id != null && isGitmodules(raw, lastNameB, lastNameE, id)) {
+				ObjectId blob = ObjectId.fromRaw(raw, ptr - OBJECT_ID_LENGTH);
+				gitsubmodules.add(new GitmoduleEntry(id, blob));
+			}
 		}
 	}
 
@@ -861,10 +875,9 @@ public class ObjectChecker {
 
 	// Mac's HFS+ folds permutations of ".git" and Unicode ignorable characters
 	// to ".git" therefore we should prevent such names
-	private boolean isMacHFSGit(byte[] raw, int ptr, int end,
+	private boolean isMacHFSPath(byte[] raw, int ptr, int end, byte[] path,
 			@Nullable AnyObjectId id) throws CorruptObjectException {
 		boolean ignorable = false;
-		byte[] git = new byte[] { '.', 'g', 'i', 't' };
 		int g = 0;
 		while (ptr < end) {
 			switch (raw[ptr]) {
@@ -920,17 +933,31 @@ public class ObjectChecker {
 				}
 				return false;
 			default:
-				if (g == 4)
+				if (g == path.length) {
 					return false;
-				if (raw[ptr++] != git[g++])
+				}
+				if (toLower(raw[ptr++]) != path[g++]) {
 					return false;
+				}
 			}
 		}
-		if (g == 4 && ignorable)
+		if (g == path.length && ignorable) {
 			return true;
+		}
 		return false;
 	}
 
+	private boolean isMacHFSGit(byte[] raw, int ptr, int end,
+			@Nullable AnyObjectId id) throws CorruptObjectException {
+		byte[] git = new byte[] { '.', 'g', 'i', 't' };
+		return isMacHFSPath(raw, ptr, end, git, id);
+	}
+
+	private boolean isMacHFSGitmodules(byte[] raw, int ptr, int end,
+			@Nullable AnyObjectId id) throws CorruptObjectException {
+		return isMacHFSPath(raw, ptr, end, dotGitmodules, id);
+	}
+
 	private boolean checkTruncatedIgnorableUTF8(byte[] raw, int ptr, int end,
 			@Nullable AnyObjectId id) throws CorruptObjectException {
 		if ((ptr + 2) >= end) {
@@ -1037,6 +1064,104 @@ public class ObjectChecker {
 				&& toLower(buf[p + 2]) == 't';
 	}
 
+	/**
+	 * Check if the filename contained in buf[start:end] could be read as a
+	 * .gitmodules file when checked out to the working directory.
+	 *
+	 * This ought to be a simple comparison, but some filesystems have peculiar
+	 * rules for normalizing filenames:
+	 *
+	 * NTFS has backward-compatibility support for 8.3 synonyms of long file
+	 * names (see
+	 * https://web.archive.org/web/20160318181041/https://usn.pw/blog/gen/2015/06/09/filenames/
+	 * for details). NTFS is also case-insensitive.
+	 *
+	 * MacOS's HFS+ folds away ignorable Unicode characters in addition to case
+	 * folding.
+	 *
+	 * @param buf
+	 *            byte array to decode
+	 * @param start
+	 *            position where a supposed filename is starting
+	 * @param end
+	 *            position where a supposed filename is ending
+	 * @param id
+	 *            object id for error reporting
+	 *
+	 * @return true if the filename in buf could be a ".gitmodules" file
+	 * @throws CorruptObjectException
+	 */
+	private boolean isGitmodules(byte[] buf, int start, int end, @Nullable AnyObjectId id)
+			throws CorruptObjectException {
+		// Simple cases first.
+		if (end - start < 8) {
+			return false;
+		}
+		return (end - start == dotGitmodules.length
+				&& RawParseUtils.match(buf, start, dotGitmodules) != -1)
+			|| (macosx && isMacHFSGitmodules(buf, start, end, id))
+			|| (windows && isNTFSGitmodules(buf, start, end));
+	}
+
+	private boolean matchLowerCase(byte[] b, int ptr, byte[] src) {
+		if (ptr + src.length > b.length) {
+			return false;
+		}
+		for (int i = 0; i < src.length; i++, ptr++) {
+			if (toLower(b[ptr]) != src[i]) {
+				return false;
+			}
+		}
+		return true;
+	}
+
+	// .gitmodules, case-insensitive, or an 8.3 abbreviation of the same.
+	private boolean isNTFSGitmodules(byte[] buf, int start, int end) {
+		if (end - start == 11) {
+			return matchLowerCase(buf, start, dotGitmodules);
+		}
+
+		if (end - start != 8) {
+			return false;
+		}
+
+		// "gitmod" or a prefix of "gi7eba", followed by...
+		byte[] gitmod = new byte[]{'g', 'i', 't', 'm', 'o', 'd', '~'};
+		if (matchLowerCase(buf, start, gitmod)) {
+			start += 6;
+		} else {
+			byte[] gi7eba = new byte[]{'g', 'i', '7', 'e', 'b', 'a'};
+			for (int i = 0; i < gi7eba.length; i++, start++) {
+				byte c = (byte) toLower(buf[start]);
+				if (c == '~') {
+					break;
+				}
+				if (c != gi7eba[i]) {
+					return false;
+				}
+			}
+		}
+
+		// ... ~ and a number
+		if (end - start < 2) {
+			return false;
+		}
+		if (buf[start] != '~') {
+			return false;
+		}
+		start++;
+		if (buf[start] < '1' || buf[start] > '9') {
+			return false;
+		}
+		start++;
+		for (; start != end; start++) {
+			if (buf[start] < '0' || buf[start] > '9') {
+				return false;
+			}
+		}
+		return true;
+	}
+
 	private static boolean isGitTilde1(byte[] buf, int p, int end) {
 		if (end - p != 5)
 			return false;
@@ -1113,4 +1238,17 @@ public class ObjectChecker {
 		String n = RawParseUtils.decode(raw, ptr, end).toLowerCase(Locale.US);
 		return macosx ? Normalizer.normalize(n, Normalizer.Form.NFC) : n;
 	}
+
+	/**
+	 * Get the list of".gitmodules" files found in the pack. For each, report
+	 * its blob id (e.g. to validate its contents) and the tree where it was
+	 * found (e.g. to check if it is in the root)
+	 *
+	 * @return List of pairs of ids <tree, blob>
+	 *
+	 * @since 4.7.5
+	 */
+	public List<GitmoduleEntry> getGitsubmodules() {
+		return gitsubmodules;
+	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java
index c426e3c05d..d3419bc201 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java
@@ -43,6 +43,7 @@
 
 package org.eclipse.jgit.transport;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_ATOMIC;
 import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_DELETE_REFS;
 import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_OFS_DELTA;
@@ -76,14 +77,19 @@ import org.eclipse.jgit.errors.PackProtocolException;
 import org.eclipse.jgit.errors.TooLargePackException;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.internal.storage.file.PackLock;
+import org.eclipse.jgit.internal.submodule.SubmoduleValidator;
+import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.BatchRefUpdate;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.GitmoduleEntry;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectChecker;
+import org.eclipse.jgit.lib.ObjectDatabase;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectIdSubclassMap;
 import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectLoader;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Ref;
@@ -1185,8 +1191,10 @@ public abstract class BaseReceivePack {
 	 */
 	protected void receivePackAndCheckConnectivity() throws IOException {
 		receivePack();
-		if (needCheckConnectivity())
+		if (needCheckConnectivity()) {
+			checkSubmodules();
 			checkConnectivity();
+		}
 		parser = null;
 	}
 
@@ -1510,6 +1518,21 @@ public abstract class BaseReceivePack {
 				|| !getClientShallowCommits().isEmpty();
 	}
 
+	private void checkSubmodules()
+			throws IOException {
+		ObjectDatabase odb = db.getObjectDatabase();
+		if (objectChecker == null) {
+			return;
+		}
+		for (GitmoduleEntry entry : objectChecker.getGitsubmodules()) {
+			AnyObjectId blobId = entry.getBlobId();
+			ObjectLoader blob = odb.open(blobId, Constants.OBJ_BLOB);
+
+			SubmoduleValidator.assertValidGitModulesFile(
+					new String(blob.getBytes(), UTF_8));
+		}
+	}
+
 	private void checkConnectivity() throws IOException {
 		ObjectIdSubclassMap<ObjectId> baseObjects = null;
 		ObjectIdSubclassMap<ObjectId> providedObjects = null;
diff --git a/pom.xml b/pom.xml
index 2d1013699d..698bc14739 100644
--- a/pom.xml
+++ b/pom.xml
@@ -475,7 +475,7 @@
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-javadoc-plugin</artifactId>
         <configuration>
-          <additionalparam>-Xdoclint:-missing</additionalparam>
+          <additionalJOption>-Xdoclint:-missing</additionalJOption>
           <encoding>${project.build.sourceEncoding}</encoding>
           <quiet>true</quiet>
           <excludePackageNames>org.eclipse.jgit.http.test</excludePackageNames>
@@ -554,7 +554,7 @@
           </reportSet>
         </reportSets>
         <configuration>
-          <additionalparam>-Xdoclint:none</additionalparam>
+          <additionalJOption>-Xdoclint:-missing</additionalJOption>
         </configuration>
       </plugin>
       <plugin>