Skip to content

Commit 8e3070a

Browse files
avihttaylorr
authored andcommitted
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) <[email protected]> Signed-off-by: Johannes Sixt <[email protected]> Signed-off-by: Taylor Blau <[email protected]>
1 parent 7dd272e commit 8e3070a

File tree

1 file changed

+3
-16
lines changed

1 file changed

+3
-16
lines changed

gitk

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -353,16 +353,6 @@ proc parseviewrevs {view revs} {
353353
return $ret
354354
}
355355

356-
# Escapes a list of filter paths to be passed to git log via stdin. Note that
357-
# paths must not be quoted.
358-
proc escape_filter_paths {paths} {
359-
set escaped [list]
360-
foreach path $paths {
361-
lappend escaped [string map {\\ \\\\ "\ " "\\\ "} $path]
362-
}
363-
return $escaped
364-
}
365-
366356
# Start off a git log process and arrange to read its output
367357
proc start_rev_list {view} {
368358
global startmsecs commitidx viewcomplete curview
@@ -424,8 +414,7 @@ proc start_rev_list {view} {
424414
if {[catch {
425415
set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \
426416
--parents --boundary $args --stdin \
427-
"<<[join [concat $revs "--" \
428-
[escape_filter_paths $files]] "\\n"]"] r]
417+
[list "<<[join [concat $revs "--" $files] "\n"]"]] r]
429418
} err]} {
430419
error_popup "[mc "Error executing git log:"] $err"
431420
return 0
@@ -578,9 +567,7 @@ proc updatecommits {} {
578567
if {[catch {
579568
set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \
580569
--parents --boundary $args --stdin \
581-
"<<[join [concat $revs "--" \
582-
[escape_filter_paths \
583-
$vfilelimit($view)]] "\\n"]"] r]
570+
[list "<<[join [concat $revs "--" $vfilelimit($view)] "\n"]"]] r]
584571
} err]} {
585572
error_popup "[mc "Error executing git log:"] $err"
586573
return
@@ -10258,7 +10245,7 @@ proc getallcommits {} {
1025810245
if {$ids eq "--all"} {
1025910246
set cmd [concat $cmd "--all"]
1026010247
} else {
10261-
set cmd [concat $cmd --stdin "<<[join $ids "\\n"]"]
10248+
set cmd [concat $cmd --stdin [list "<<[join $ids "\n"]"]]
1026210249
}
1026310250
set fd [open $cmd r]
1026410251
fconfigure $fd -blocking 0

0 commit comments

Comments
 (0)