FS.runInShell(): handle quoted filters and hooksPath containing blanks
Revert commit2323d7a
. Using $0 in the shell command call results in the command string being taken literally. That was introduced to fix a problem with backslashes, but is actually not correct. First, the problem with backslashes occurred only on Win32/Cygwin, and has been properly fixed in commit6f268f8
. Second, this is used only for hooks (which don't have backslashes in their names) and filter commands from the git config, where the user is responsible for properly quoting or escaping such that the commands work. Third, using $0 actually breaks correctly quoted filter commands like in the bug report. The shell really takes the command literally, and then doesn't find the command because of quotes. So revert this change. At the same time there's a related problem with hooks. If the path to the hook contains blanks, runInShell() would also fail to find the hook. In this case, the command doesn't come from user input but is just a Java File object with an absolute path containing blanks. (Can occur if core.hooksPath points to such a path with blanks, or if the repository has such a path.) The path to the hook as obtained from the file system must be quoted. Add a test for a hook path with a blank. This reverts commit2323d7a1ef
. Bug: 561666 Change-Id: I4d7df13e6c9b245fe1706e191e4316685a8a9d59 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This commit is contained in:
parent
24f82b533a
commit
2640d38f14
|
@ -291,6 +291,38 @@ public void testRunHookHooksPathAbsolute() throws Exception {
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHookPathWithBlank() throws Exception {
|
||||
assumeSupportedPlatform();
|
||||
|
||||
File file = writeHookFile("../../a directory/" + PreCommitHook.NAME,
|
||||
"#!/bin/sh\necho \"test $1 $2\"\nread INPUT\necho $INPUT\n"
|
||||
+ "echo $GIT_DIR\necho $GIT_WORK_TREE\necho 1>&2 \"stderr\"");
|
||||
StoredConfig cfg = db.getConfig();
|
||||
cfg.load();
|
||||
cfg.setString(ConfigConstants.CONFIG_CORE_SECTION, null,
|
||||
ConfigConstants.CONFIG_KEY_HOOKS_PATH,
|
||||
file.getParentFile().getAbsolutePath());
|
||||
cfg.save();
|
||||
try (ByteArrayOutputStream out = new ByteArrayOutputStream();
|
||||
ByteArrayOutputStream err = new ByteArrayOutputStream()) {
|
||||
ProcessResult res = FS.DETECTED.runHookIfPresent(db,
|
||||
PreCommitHook.NAME, new String[] { "arg1", "arg2" },
|
||||
new PrintStream(out), new PrintStream(err), "stdin");
|
||||
|
||||
assertEquals("unexpected hook output",
|
||||
"test arg1 arg2\nstdin\n"
|
||||
+ db.getDirectory().getAbsolutePath() + '\n'
|
||||
+ db.getWorkTree().getAbsolutePath() + '\n',
|
||||
out.toString("UTF-8"));
|
||||
assertEquals("unexpected output on stderr stream", "stderr\n",
|
||||
err.toString("UTF-8"));
|
||||
assertEquals("unexpected exit code", 0, res.getExitCode());
|
||||
assertEquals("unexpected process status", ProcessResult.Status.OK,
|
||||
res.getStatus());
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testFailedPreCommitHookBlockCommit() throws Exception {
|
||||
assumeSupportedPlatform();
|
||||
|
|
|
@ -1747,7 +1747,7 @@ protected ProcessResult internalRunHookIfPresent(Repository repository,
|
|||
return new ProcessResult(Status.NOT_PRESENT);
|
||||
}
|
||||
String cmd = hookFile.getAbsolutePath();
|
||||
ProcessBuilder hookProcess = runInShell(cmd, args);
|
||||
ProcessBuilder hookProcess = runInShell(shellQuote(cmd), args);
|
||||
hookProcess.directory(runDirectory.getAbsoluteFile());
|
||||
Map<String, String> environment = hookProcess.environment();
|
||||
environment.put(Constants.GIT_DIR_KEY,
|
||||
|
@ -1770,6 +1770,21 @@ protected ProcessResult internalRunHookIfPresent(Repository repository,
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Quote a string (such as a file system path obtained from a Java
|
||||
* {@link File} or {@link Path} object) such that it can be passed as first
|
||||
* argument to {@link #runInShell(String, String[])}.
|
||||
* <p>
|
||||
* This default implementation returns the string unchanged.
|
||||
* </p>
|
||||
*
|
||||
* @param cmd
|
||||
* the String to quote
|
||||
* @return the quoted string
|
||||
*/
|
||||
String shellQuote(String cmd) {
|
||||
return cmd;
|
||||
}
|
||||
|
||||
/**
|
||||
* Tries to find a hook matching the given one in the given repository.
|
||||
|
|
|
@ -261,7 +261,7 @@ public ProcessBuilder runInShell(String cmd, String[] args) {
|
|||
List<String> argv = new ArrayList<>(4 + args.length);
|
||||
argv.add("sh"); //$NON-NLS-1$
|
||||
argv.add("-c"); //$NON-NLS-1$
|
||||
argv.add("$0 \"$@\""); //$NON-NLS-1$
|
||||
argv.add(cmd + " \"$@\""); //$NON-NLS-1$
|
||||
argv.add(cmd);
|
||||
argv.addAll(Arrays.asList(args));
|
||||
ProcessBuilder proc = new ProcessBuilder();
|
||||
|
@ -269,6 +269,11 @@ public ProcessBuilder runInShell(String cmd, String[] args) {
|
|||
return proc;
|
||||
}
|
||||
|
||||
@Override
|
||||
String shellQuote(String cmd) {
|
||||
return QuotedString.BOURNE.quote(cmd);
|
||||
}
|
||||
|
||||
/** {@inheritDoc} */
|
||||
@Override
|
||||
public ProcessResult runHookIfPresent(Repository repository, String hookName,
|
||||
|
|
|
@ -149,14 +149,19 @@ public ProcessBuilder runInShell(String cmd, String[] args) {
|
|||
List<String> argv = new ArrayList<>(4 + args.length);
|
||||
argv.add("sh.exe"); //$NON-NLS-1$
|
||||
argv.add("-c"); //$NON-NLS-1$
|
||||
argv.add("$0 \"$@\""); //$NON-NLS-1$
|
||||
argv.add(cmd.replace(File.separatorChar, '/'));
|
||||
argv.add(cmd + " \"$@\""); //$NON-NLS-1$
|
||||
argv.add(cmd);
|
||||
argv.addAll(Arrays.asList(args));
|
||||
ProcessBuilder proc = new ProcessBuilder();
|
||||
proc.command(argv);
|
||||
return proc;
|
||||
}
|
||||
|
||||
@Override
|
||||
String shellQuote(String cmd) {
|
||||
return QuotedString.BOURNE.quote(cmd.replace(File.separatorChar, '/'));
|
||||
}
|
||||
|
||||
/** {@inheritDoc} */
|
||||
@Override
|
||||
public String relativize(String base, String other) {
|
||||
|
|
Loading…
Reference in New Issue