From f342db7666fd49acee80e1c5c30acff04aa0c82c Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Sat, 11 Feb 2023 14:51:58 +0100 Subject: [PATCH] ui/termstatus: Quote funny filenames Fixes #2260, #4191. --- changelog/unreleased/issue-2260 | 13 +++++++++++ internal/ui/backup/text.go | 2 ++ internal/ui/termstatus/status.go | 29 ++++++++++++++++++------ internal/ui/termstatus/status_test.go | 32 ++++++++++++++++++++++++++- 4 files changed, 68 insertions(+), 8 deletions(-) create mode 100644 changelog/unreleased/issue-2260 diff --git a/changelog/unreleased/issue-2260 b/changelog/unreleased/issue-2260 new file mode 100644 index 000000000..96c79a035 --- /dev/null +++ b/changelog/unreleased/issue-2260 @@ -0,0 +1,13 @@ +Bugfix: Exotic filenames no longer break restic backup's status output + +Restic backup shows the names of files that it is working on. In previous +versions of restic, those names were printed without first sanitizing them, +so that filenames containing newlines or terminal control characters could +mess up restic backup's output or even change the state of a terminal. + +Filenames are now checked and quoted if they contain non-printable or +non-Unicode characters. + +https://github.com/restic/restic/issues/2260 +https://github.com/restic/restic/issues/4191 +https://github.com/restic/restic/pull/4192 diff --git a/internal/ui/backup/text.go b/internal/ui/backup/text.go index 0c5f897dd..acb2a8d3a 100644 --- a/internal/ui/backup/text.go +++ b/internal/ui/backup/text.go @@ -86,6 +86,8 @@ func (b *TextProgress) Error(item string, err error) error { // CompleteItem is the status callback function for the archiver when a // file/dir has been saved successfully. func (b *TextProgress) CompleteItem(messageType, item string, previous, current *restic.Node, s archiver.ItemStats, d time.Duration) { + item = termstatus.Quote(item) + switch messageType { case "dir new": b.VV("new %v, saved in %.3fs (%v added, %v stored, %v metadata)", diff --git a/internal/ui/termstatus/status.go b/internal/ui/termstatus/status.go index fdc7e14f6..a1b7a5fcc 100644 --- a/internal/ui/termstatus/status.go +++ b/internal/ui/termstatus/status.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "os" + "strconv" "strings" "unicode" @@ -325,6 +326,7 @@ func wideRune(r rune) bool { } // SetStatus updates the status lines. +// The lines should not contain newlines; this method adds them. func (t *Terminal) SetStatus(lines []string) { if len(lines) == 0 { return @@ -341,21 +343,34 @@ func (t *Terminal) SetStatus(lines []string) { } } - // make sure that all lines have a line break and are not too long + // Sanitize lines and truncate them if they're too long. for i, line := range lines { - line = strings.TrimRight(line, "\n") + line = Quote(line) if width > 0 { line = Truncate(line, width-2) } - lines[i] = line + "\n" + if i < len(lines)-1 { // Last line gets no line break. + lines[i] = line + "\n" + } } - // make sure the last line does not have a line break - last := len(lines) - 1 - lines[last] = strings.TrimRight(lines[last], "\n") - select { case t.status <- status{lines: lines}: case <-t.closed: } } + +// Quote lines with funny characters in them, meaning control chars, newlines, +// tabs, anything else non-printable and invalid UTF-8. +// +// This is intended to produce a string that does not mess up the terminal +// rather than produce an unambiguous quoted string. +func Quote(line string) string { + for _, r := range line { + // The replacement character usually means the input is not UTF-8. + if r == unicode.ReplacementChar || !unicode.IsPrint(r) { + return strconv.Quote(line) + } + } + return line +} diff --git a/internal/ui/termstatus/status_test.go b/internal/ui/termstatus/status_test.go index ce18f42e6..40a908deb 100644 --- a/internal/ui/termstatus/status_test.go +++ b/internal/ui/termstatus/status_test.go @@ -1,6 +1,36 @@ package termstatus -import "testing" +import ( + "strconv" + "testing" + + rtest "github.com/restic/restic/internal/test" +) + +func TestQuote(t *testing.T) { + for _, c := range []struct { + in string + needQuote bool + }{ + {"foo.bar/baz", false}, + {"föó_bàŕ-bãẑ", false}, + {" foo ", false}, + {"foo bar", false}, + {"foo\nbar", true}, + {"foo\rbar", true}, + {"foo\abar", true}, + {"\xff", true}, + {`c:\foo\bar`, false}, + // Issue #2260: terminal control characters. + {"\x1bm_red_is_beautiful", true}, + } { + if c.needQuote { + rtest.Equals(t, strconv.Quote(c.in), Quote(c.in)) + } else { + rtest.Equals(t, c.in, Quote(c.in)) + } + } +} func TestTruncate(t *testing.T) { var tests = []struct {