From 8e3070aa5e331be45d4d03e3be41f84494fce129 Mon Sep 17 00:00:00 2001 From: "Avi Halachmi (:avih)" Date: Fri, 7 Mar 2025 13:48:57 +0200 Subject: [PATCH] gitk: encode arguments correctly with "open" While "exec" uses a normal arguments list which is applied as command + arguments (and redirections, etc), "open" uses a single argument which is this command+arguments, where the command and arguments are a list inside this one argument to "open". Commit bb5cb23 (gitk: prevent overly long command lines 2023-05-08) changed several values from individual arguments in that list (hashes and file names), to a single value which is fed to git via redirection to its stdin using "open" [1]. However, it didn't ensure correctly that this aggregate value in this string is interpreted as a single element in this command+args list. It did just enough so that newlines (which is how these elements are concatenated) don't split this single list element. A followup commit at the same patchset: 7dd272e (gitk: escape file paths before piping to git log 2023-05-08) added a bit more, by escaping backslahes and spaces at the file names, so that at least it doesn't break when such file names get used there. But these are not enough. At the very least tab is missing, and more, and trying to manually escape every possible thing which can affect how this string is interpreted in a list is a sub-par approach. The solution is simply to tell tcl "this is a single list element". which we can do by aggregating this value completely normally (hashes and files separated by newlines), and then do [list $value]. So this is what this commit does, for all 3 places where bb5cb23 changed individual elements into an aggregate value. [1] That was not a fully accurate description. The accurate version is that this string originally included two lists: hashes and files. When used with "open" these lists correctly become the individual elements of these lists, even if they contain spaces etc, so the arguments which were used at this "git" commands were correct. Commit bb5cb23 couldn't use these two lists as-is, because it needed to process the individual elements in them (one element per line of the aggregate value), and the issue is that ensuring this aggregate is indeed interpreted as a single list element was sub-par. Note: all the (double) quotes before/after the modification are not required and with zero effect, even for \n. But this commit preserves the original quoting form intentionally. It can be cleaned up later. Signed-off-by: Avi Halachmi (:avih) Signed-off-by: Johannes Sixt Signed-off-by: Taylor Blau --- gitk | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/gitk b/gitk index df3ba2ea99..1f669c5190 100755 --- a/gitk +++ b/gitk @@ -353,16 +353,6 @@ proc parseviewrevs {view revs} { return $ret } -# Escapes a list of filter paths to be passed to git log via stdin. Note that -# paths must not be quoted. -proc escape_filter_paths {paths} { - set escaped [list] - foreach path $paths { - lappend escaped [string map {\\ \\\\ "\ " "\\\ "} $path] - } - return $escaped -} - # Start off a git log process and arrange to read its output proc start_rev_list {view} { global startmsecs commitidx viewcomplete curview @@ -424,8 +414,7 @@ proc start_rev_list {view} { if {[catch { set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \ --parents --boundary $args --stdin \ - "<<[join [concat $revs "--" \ - [escape_filter_paths $files]] "\\n"]"] r] + [list "<<[join [concat $revs "--" $files] "\n"]"]] r] } err]} { error_popup "[mc "Error executing git log:"] $err" return 0 @@ -578,9 +567,7 @@ proc updatecommits {} { if {[catch { set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \ --parents --boundary $args --stdin \ - "<<[join [concat $revs "--" \ - [escape_filter_paths \ - $vfilelimit($view)]] "\\n"]"] r] + [list "<<[join [concat $revs "--" $vfilelimit($view)] "\n"]"]] r] } err]} { error_popup "[mc "Error executing git log:"] $err" return @@ -10258,7 +10245,7 @@ proc getallcommits {} { if {$ids eq "--all"} { set cmd [concat $cmd "--all"] } else { - set cmd [concat $cmd --stdin "<<[join $ids "\\n"]"] + set cmd [concat $cmd --stdin [list "<<[join $ids "\n"]"]] } set fd [open $cmd r] fconfigure $fd -blocking 0 -- 2.47.2