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>

diff --git a/git-gui.sh b/git-gui.sh
index 301c764..408149b 100755 (executable)
--- 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 44dcdf2..cc416d4 100644 (file)
--- 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 777d7b3..6b26726 100644 (file)
--- 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 2e006cb..b32bdd0 100644 (file)
--- 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 413f1a1..142ffce 100644 (file)
--- 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]