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>

Johannes Sixt committed May 4, 2025 at 15:06 UTC 1e0a93c3d35c84547b21ba704a9c4383d4360140
5 files changed +11 -12
git-gui.sh
+5 -5
@@ -631,10 +631,10 @@ proc git {args} {
631 return $result
632 }
633
634 -proc _open_stdout_stderr {cmd} {
635 - _trace_exec $cmd
634 +proc safe_open_command {cmd {redir {}}} {
635 + _trace_exec [concat $cmd $redir]
636 if {[catch {
637 - set fd [open [concat [list | ] $cmd] r]
637 + set fd [open [concat [list | ] $cmd $redir] r]
638 } err]} {
639 error $err
640 }
@@ -646,7 +646,7 @@ proc git_read {cmd} {
646 set cmdp [_git_cmd [lindex $cmd 0]]
647 set cmd [lrange $cmd 1 end]
648
649 - return [_open_stdout_stderr [concat $cmdp $cmd]]
649 + return [safe_open_command [concat $cmdp $cmd]]
650 }
651
652 proc git_read_nice {cmd} {
@@ -657,7 +657,7 @@ proc git_read_nice {cmd} {
657 set cmdp [_git_cmd [lindex $cmd 0]]
658 set cmd [lrange $cmd 1 end]
659
660 - return [_open_stdout_stderr [concat $opt $cmdp $cmd]]
660 + return [safe_open_command [concat $opt $cmdp $cmd]]
661 }
662
663 proc git_write {cmd} {
lib/console.tcl
+2 -2
@@ -91,11 +91,11 @@ method _init {} {
91 }
92
93 method exec {cmd {after {}}} {
94 - lappend cmd 2>@1
94 if {[lindex $cmd 0] eq {git}} {
95 + lappend cmd 2>@1
96 set fd_f [git_read [lrange $cmd 1 end]]
97 } else {
98 - set fd_f [_open_stdout_stderr $cmd]
98 + set fd_f [safe_open_command $cmd [list 2>@1]]
99 }
100 fconfigure $fd_f -blocking 0 -translation binary
101 fileevent $fd_f readable [cb _read $fd_f $after]
lib/mergetool.tcl
+2 -2
@@ -343,9 +343,9 @@ proc merge_tool_start {cmdline target backup stages} {
343
344 # Force redirection to avoid interpreting output on stderr
345 # as an error, and launch the tool
346 - lappend cmdline {2>@1}
346 + set redir [list {2>@1}]
347
348 - if {[catch { set mtool_fd [_open_stdout_stderr $cmdline] } err]} {
348 + if {[catch { set mtool_fd [safe_open_command $cmdline $redir] } err]} {
349 delete_temp_files $mtool_tmpfiles
350 error_popup [mc "Could not start the merge tool:\n\n%s" $err]
351 return
lib/sshkey.tcl
+1 -1
@@ -85,7 +85,7 @@ proc make_ssh_key {w} {
85
86 set cmdline [list sh -c {echo | ssh-keygen -q -t rsa -f ~/.ssh/id_rsa 2>&1}]
87
88 - if {[catch { set sshkey_fd [_open_stdout_stderr $cmdline] } err]} {
88 + if {[catch { set sshkey_fd [safe_open_command $cmdline] } err]} {
89 error_popup [mc "Could not start ssh-keygen:\n\n%s" $err]
90 return
91 }
lib/tools.tcl
+1 -2
@@ -130,8 +130,7 @@ proc tools_exec {fullname} {
130 }
131
132 proc tools_run_silent {cmd after} {
133 - lappend cmd 2>@1
134 - set fd [_open_stdout_stderr $cmd]
133 + set fd [safe_open_command $cmd [list 2>@1]]
134
135 fconfigure $fd -blocking 0 -translation binary
136 fileevent $fd readable [list tools_consume_input $fd $after]