Merge pull request #4192 from greatroar/quote

ui/termstatus: Quote funny filenames

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

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)",

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
+}

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 {