git-gui: pass redirections as separate argument to _open_stdout_stderr

We are going to treat command arguments and redirections differently to
avoid passing arguments that look like redirections to the command
accidentally. To do so, it will be necessary to know which arguments
are intentional redirections. Rewrite direct callers of
_open_stdout_stderr to pass intentional redirections as a second
(optional) argument.

Passing arbitrary arguments is not safe right now, but we rename it
to safe_open_command anyway to avoid having to touch the call sites
again later when we make it actually safe.

We cannot make the function safe right away because one caller is
git_read, which does not yet know which of its arguments are
redirections. This is the topic of the next commit.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>

Signed-off-by: Taylor Blau <me@ttaylorr.com>

5 files changed, 11 insertions, 12 deletions

diff --git a/git-gui.sh b/git-gui.sh
index 301c7647ec..408149b530 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -631,10 +631,10 @@ proc git {args} {
 	return $result
 }
 
-proc _open_stdout_stderr {cmd} {
-	_trace_exec $cmd
+proc safe_open_command {cmd {redir {}}} {
+	_trace_exec [concat $cmd $redir]
 	if {[catch {
-		set fd [open [concat [list | ] $cmd] r]
+		set fd [open [concat [list | ] $cmd $redir] r]
 	} err]} {
 		error $err
 	}
@@ -646,7 +646,7 @@ proc git_read {cmd} {
 	set cmdp [_git_cmd [lindex $cmd 0]]
 	set cmd [lrange $cmd 1 end]
 
-	return [_open_stdout_stderr [concat $cmdp $cmd]]
+	return [safe_open_command [concat $cmdp $cmd]]
 }
 
 proc git_read_nice {cmd} {
@@ -657,7 +657,7 @@ proc git_read_nice {cmd} {
 	set cmdp [_git_cmd [lindex $cmd 0]]
 	set cmd [lrange $cmd 1 end]
 
-	return [_open_stdout_stderr [concat $opt $cmdp $cmd]]
+	return [safe_open_command [concat $opt $cmdp $cmd]]
 }
 
 proc git_write {cmd} {
diff --git a/lib/console.tcl b/lib/console.tcl
index 44dcdf29be..cc416d4811 100644
--- a/lib/console.tcl
+++ b/lib/console.tcl
@@ -91,11 +91,11 @@ method _init {} {
 }
 
 method exec {cmd {after {}}} {
-	lappend cmd 2>@1
 	if {[lindex $cmd 0] eq {git}} {
+		lappend cmd 2>@1
 		set fd_f [git_read [lrange $cmd 1 end]]
 	} else {
-		set fd_f [_open_stdout_stderr $cmd]
+		set fd_f [safe_open_command $cmd [list 2>@1]]
 	}
 	fconfigure $fd_f -blocking 0 -translation binary
 	fileevent $fd_f readable [cb _read $fd_f $after]
diff --git a/lib/mergetool.tcl b/lib/mergetool.tcl
index 777d7b323b..6b26726418 100644
--- a/lib/mergetool.tcl
+++ b/lib/mergetool.tcl
@@ -343,9 +343,9 @@ proc merge_tool_start {cmdline target backup stages} {
 
 	# Force redirection to avoid interpreting output on stderr
 	# as an error, and launch the tool
-	lappend cmdline {2>@1}
+	set redir [list {2>@1}]
 
-	if {[catch { set mtool_fd [_open_stdout_stderr $cmdline] } err]} {
+	if {[catch { set mtool_fd [safe_open_command $cmdline $redir] } err]} {
 		delete_temp_files $mtool_tmpfiles
 		error_popup [mc "Could not start the merge tool:\n\n%s" $err]
 		return
diff --git a/lib/sshkey.tcl b/lib/sshkey.tcl
index 2e006cb8ca..b32bdd06e9 100644
--- a/lib/sshkey.tcl
+++ b/lib/sshkey.tcl
@@ -85,7 +85,7 @@ proc make_ssh_key {w} {
 
 	set cmdline [list sh -c {echo | ssh-keygen -q -t rsa -f ~/.ssh/id_rsa 2>&1}]
 
-	if {[catch { set sshkey_fd [_open_stdout_stderr $cmdline] } err]} {
+	if {[catch { set sshkey_fd [safe_open_command $cmdline] } err]} {
 		error_popup [mc "Could not start ssh-keygen:\n\n%s" $err]
 		return
 	}
diff --git a/lib/tools.tcl b/lib/tools.tcl
index 413f1a1700..142ffceedd 100644
--- a/lib/tools.tcl
+++ b/lib/tools.tcl
@@ -130,8 +130,7 @@ proc tools_exec {fullname} {
 }
 
 proc tools_run_silent {cmd after} {
-	lappend cmd 2>@1
-	set fd [_open_stdout_stderr $cmd]
+	set fd [safe_open_command $cmd [list 2>@1]]
 
 	fconfigure $fd -blocking 0 -translation binary
 	fileevent $fd readable [list tools_consume_input $fd $after]