Add API to specify the submodule name

Currently SubmoduleAddCommand always uses the path as submodule name.
This patch lets the caller specify a submodule name.

SubmoduleUpdateCommand still does not make use of the submodule name
(see bug 535027) but Git does.  To avoid triggering CVE-2018-11235,
do some validation on the name to avoid '..' path components.

[jn: fleshed out commit message, mostly to work around flaky CI]

Change-Id: I6879c043c6d7973556e2080387f23c246e3d76a5
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleAddTest.java

@@ -136,7 +136,49 @@ public class SubmoduleAddTest extends RepositoryTestCase {
 			}
 
 			SubmoduleWalk generator = SubmoduleWalk.forIndex(db);
+			generator.loadModulesConfig();
 			assertTrue(generator.next());
+			assertEquals(path, generator.getModuleName());
+			assertEquals(path, generator.getPath());
+			assertEquals(commit, generator.getObjectId());
+			assertEquals(uri, generator.getModulesUrl());
+			assertEquals(path, generator.getModulesPath());
+			assertEquals(uri, generator.getConfigUrl());
+			try (Repository subModRepo = generator.getRepository()) {
+				assertNotNull(subModRepo);
+				assertEquals(subCommit, commit);
+			}
+
+			Status status = Git.wrap(db).status().call();
+			assertTrue(status.getAdded().contains(Constants.DOT_GIT_MODULES));
+			assertTrue(status.getAdded().contains(path));
+		}
+	}
+
+	@Test
+	public void addSubmoduleWithName() throws Exception {
+		try (Git git = new Git(db)) {
+			writeTrashFile("file.txt", "content");
+			git.add().addFilepattern("file.txt").call();
+			RevCommit commit = git.commit().setMessage("create file").call();
+
+			SubmoduleAddCommand command = new SubmoduleAddCommand(db);
+			String name = "testsub";
+			command.setName(name);
+			String path = "sub";
+			command.setPath(path);
+			String uri = db.getDirectory().toURI().toString();
+			command.setURI(uri);
+			ObjectId subCommit;
+			try (Repository repo = command.call()) {
+				assertNotNull(repo);
+				subCommit = repo.resolve(Constants.HEAD);
+			}
+
+			SubmoduleWalk generator = SubmoduleWalk.forIndex(db);
+			generator.loadModulesConfig();
+			assertTrue(generator.next());
+			assertEquals(name, generator.getModuleName());
 			assertEquals(path, generator.getPath());
 			assertEquals(commit, generator.getObjectId());
 			assertEquals(uri, generator.getModulesUrl());
@@ -268,4 +310,18 @@ public class SubmoduleAddTest extends RepositoryTestCase {
 					ConfigConstants.CONFIG_KEY_URL));
 		}
 	}
+
+	@Test
+	public void denySubmoduleWithDotDot() throws Exception {
+		SubmoduleAddCommand command = new SubmoduleAddCommand(db);
+		command.setName("dir/../");
+		command.setPath("sub");
+		command.setURI(db.getDirectory().toURI().toString());
+		try {
+			command.call();
+			fail();
+		} catch (IllegalArgumentException e) {
+			// Expected
+		}
+	}
 }

org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties

@@ -376,6 +376,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}
 invalidObject=Invalid {0} {1}: {2}
 invalidOldIdSent=invalid old id sent
 invalidPacketLineHeader=Invalid packet line header: {0}

org.eclipse.jgit/src/org/eclipse/jgit/api/SubmoduleAddCommand.java

@@ -76,6 +76,8 @@ import org.eclipse.jgit.treewalk.filter.TreeFilter;
 public class SubmoduleAddCommand extends
 		TransportCommand<SubmoduleAddCommand, Repository> {
 
+	private String name;
+
 	private String path;
 
 	private String uri;
@@ -92,6 +94,18 @@ public class SubmoduleAddCommand extends
 		super(repo);
 	}
 
+	/**
+	 * Set the submodule name
+	 *
+	 * @param name
+	 * @return this command
+	 * @since 5.1
+	 */
+	public SubmoduleAddCommand setName(String name) {
+		this.name = name;
+		return this;
+	}
+
 	/**
 	 * Set repository-relative path of submodule
 	 *
@@ -160,6 +174,25 @@ public class SubmoduleAddCommand extends
 			throw new IllegalArgumentException(JGitText.get().pathNotConfigured);
 		if (uri == null || uri.length() == 0)
 			throw new IllegalArgumentException(JGitText.get().uriNotConfigured);
+		if (name == null || name.length() == 0) {
+			// Use the path as the default.
+			name = path;
+		}
+		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 the names with them. The callers need to make sure the
+			// names free from these. 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 IllegalArgumentException(MessageFormat
+					.format(JGitText.get().invalidNameContainsDotDot, name));
+		}
 
 		try {
 			if (submoduleExists())
@@ -193,7 +226,7 @@ public class SubmoduleAddCommand extends
 
 		// Save submodule URL to parent repository's config
 		StoredConfig config = repo.getConfig();
-		config.setString(ConfigConstants.CONFIG_SUBMODULE_SECTION, path,
+		config.setString(ConfigConstants.CONFIG_SUBMODULE_SECTION, name,
 				ConfigConstants.CONFIG_KEY_URL, resolvedUri);
 		try {
 			config.save();
@@ -207,9 +240,9 @@ public class SubmoduleAddCommand extends
 		try {
 			modulesConfig.load();
 			modulesConfig.setString(ConfigConstants.CONFIG_SUBMODULE_SECTION,
-					path, ConfigConstants.CONFIG_KEY_PATH, path);
+					name, ConfigConstants.CONFIG_KEY_PATH, path);
 			modulesConfig.setString(ConfigConstants.CONFIG_SUBMODULE_SECTION,
-					path, ConfigConstants.CONFIG_KEY_URL, uri);
+					name, ConfigConstants.CONFIG_KEY_URL, uri);
 			modulesConfig.save();
 		} catch (IOException e) {
 			throw new JGitInternalException(e.getMessage(), e);

org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java

@@ -437,6 +437,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String invalidLineInConfigFileWithParam;
 	/***/ public String invalidModeFor;
 	/***/ public String invalidModeForPath;
+	/***/ public String invalidNameContainsDotDot;
 	/***/ public String invalidObject;
 	/***/ public String invalidOldIdSent;
 	/***/ public String invalidPacketLineHeader;