diff --git a/.golangci.yaml b/.golangci.yaml index fcf9fd96..82c18774 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -17,6 +17,7 @@ linters: - fmt.Fprintln - io.WriteString - os.Setenv + - os.Unsetenv - syscall.Close issues: max-issues-per-linter: 0 # no limit diff --git a/app.go b/app.go index 725962b0..46b51726 100644 --- a/app.go +++ b/app.go @@ -160,13 +160,7 @@ func loadFiles() (clipboard clipboard, err error) { } func saveFiles(clipboard clipboard) error { - for _, path := range clipboard.paths { - // the clipboard file stores one path per line so a newline cannot be saved - if strings.ContainsAny(path, "\n\r") { - return fmt.Errorf("cannot copy %s because the name contains a newline", path) - } - } - + // clipboard.paths is already checked by currFileOrSelections, so no newline check is needed here if err := os.MkdirAll(filepath.Dir(gFilesPath), 0o700); err != nil { return fmt.Errorf("creating data directory: %w", err) } @@ -568,7 +562,10 @@ func (app *app) runCmdSync(cmd *exec.Cmd, pauseAfter bool) { // ! Yes No Yes Yes Yes Pause and then resume // & No Yes No No No Do nothing func (app *app) runShell(s string, args []string, prefix string) { - app.nav.exportFiles() + // report names that exportFiles dropped so shell commands do not fail silently + for _, msg := range app.nav.exportFiles() { + app.ui.echoerr(msg) + } app.ui.exportSizes() app.exportMode() exportLfPath() diff --git a/client.go b/client.go index cae82ea4..c957c238 100644 --- a/client.go +++ b/client.go @@ -7,7 +7,6 @@ import ( "log" "net" "os" - "slices" "strings" "sync" "time" @@ -96,7 +95,8 @@ func run() { // rejected unconditionally (frame integrity for line-oriented consumers); // control bytes are stripped only when stdout is a terminal. func printPath(label, path string, stdoutIsTerminal bool) { - if strings.ContainsAny(path, "\n\r") { + if containsNewline(path) { + fmt.Fprintf(os.Stderr, "lf: %s: skipping path with a newline: %q\n", label, path) log.Printf("%s: skipping path with newline: %q", label, path) return } @@ -107,7 +107,8 @@ func printPath(label, path string, stdoutIsTerminal bool) { } func writeLastDir(filename, lastDir string) { - if strings.ContainsAny(lastDir, "\n\r") { + if containsNewline(lastDir) { + fmt.Fprintf(os.Stderr, "lf: last-dir: skipping path with a newline: %q\n", lastDir) log.Printf("last-dir: path contains newline: %q", lastDir) return } @@ -132,14 +133,8 @@ func writeSelection(filename string, selection []string) { } defer f.Close() - filtered := slices.DeleteFunc(slices.Clone(selection), func(s string) bool { - if strings.ContainsAny(s, "\n\r") { - log.Printf("selection: skipping path with newline: %q", s) - return true - } - return false - }) - _, err = f.WriteString(strings.Join(filtered, "\n")) + // the selection is already checked by currFileOrSelections before quitting + _, err = f.WriteString(strings.Join(selection, "\n")) if err != nil { log.Printf("writing selection file: %s", err) } diff --git a/complete.go b/complete.go index dd404362..603c3f92 100644 --- a/complete.go +++ b/complete.go @@ -273,6 +273,11 @@ func matchFile(s string, dirOnly bool, escape, unescape func(string) string) (ma continue } + // skip newline names because the escaped form is a shell line continuation + if containsNewline(f.Name()) { + continue + } + name := f.Name() if isDir { name += string(filepath.Separator) diff --git a/doc.md b/doc.md index aed366bb..96fc28dd 100644 --- a/doc.md +++ b/doc.md @@ -32,6 +32,9 @@ This documentation can either be read from the terminal using `lf -doc` or onlin You can also use the `help` command (default ``) inside lf to view the documentation in a pager. A man page with the same content is also available in the repository at https://github.com/gokcehan/lf/blob/master/lf.1 +Names containing a newline or carriage return are displayed using replacement characters and can be navigated and previewed, but following the POSIX.1-2024 recommendation, operations on them (e.g. `copy`, `cut`, `delete`, `open`, `toggle`, `tag`, `mark-save`) are refused with an error, and such names are excluded from `$f`, `$fs`, `$fx` and `$fv` (with `$PWD` and `$OLDPWD` unset when affected) with a warning before shell commands run. +Use `rename` to remove the newline from the name. + # OPTIONS ## POSITIONAL ARGUMENTS @@ -602,6 +605,7 @@ A custom `delete` command can be defined to override this default. Rename the current file using the built-in method. A custom `rename` command can be defined to override this default. +Renaming a file to a name containing a newline is refused, but a name containing a newline can be edited to remove it. ## read (modal) (default `:`) diff --git a/eval.go b/eval.go index 07b15709..6797da8a 100644 --- a/eval.go +++ b/eval.go @@ -926,7 +926,13 @@ func insert(app *app, arg string) { case app.ui.cmdPrefix == "mark-save: ": normal(app) - app.nav.marks[arg] = app.nav.currDir().path + path := app.nav.currDir().path + if containsNewline(path) { + // refuse instead of storing a mark that writeMarks would silently drop + app.ui.echoerrf("mark-save: %s", errNewlinePath(path)) + return + } + app.nav.marks[arg] = path if err := app.nav.writeMarks(); err != nil { app.ui.echoerrf("mark-save: %s", err) return @@ -1113,11 +1119,21 @@ func (e *callExpr) eval(app *app, _ []string) { onChdir(app) } else { if gSelectionPath != "" || gPrintSelection { - app.selectionOut, _ = app.nav.currFileOrSelections() + list, err := app.nav.currFileOrSelections() + if err != nil { + app.ui.echoerrf("open: %s", err) + return + } + app.selectionOut = list app.quitChan <- struct{}{} return } + // refuse a newline name here because a custom open or the default opener would run with an empty $f + if containsNewline(curr.path) { + app.ui.echoerrf("open: %s", errNewlinePath(curr.path)) + return + } if cmd, ok := gOpts.cmds["open"]; ok { cmd.eval(app, e.args) } @@ -1176,7 +1192,9 @@ func (e *callExpr) eval(app *app, _ []string) { } case "toggle": if len(e.args) == 0 { - app.nav.toggle() + if err := app.nav.toggle(); err != nil { + app.ui.echoerrf("toggle: %s", err) + } } else { for _, path := range e.args { path, err := filepath.Abs(replaceTilde(path)) @@ -1190,11 +1208,15 @@ func (e *callExpr) eval(app *app, _ []string) { continue } - app.nav.toggleSelection(path) + if err := app.nav.toggleSelection(path); err != nil { + app.ui.echoerrf("toggle: %s", err) + } } } case "invert": - app.nav.invert() + if skipped := app.nav.invert(); skipped > 0 { + app.ui.echomsg(skipMsg("invert", skipped)) + } case "unselect": app.nav.unselect() case "glob-select": @@ -1202,19 +1224,27 @@ func (e *callExpr) eval(app *app, _ []string) { app.ui.echoerr("glob-select: requires a pattern to match") return } - if err := app.nav.globSel(e.args[0], false); err != nil { + skipped, err := app.nav.globSel(e.args[0], false) + if err != nil { app.ui.echoerrf("%s", err) return } + if skipped > 0 { + app.ui.echomsg(skipMsg("glob-select", skipped)) + } case "glob-unselect": if len(e.args) != 1 { app.ui.echoerr("glob-unselect: requires a pattern to match") return } - if err := app.nav.globSel(e.args[0], true); err != nil { + skipped, err := app.nav.globSel(e.args[0], true) + if err != nil { app.ui.echoerrf("%s", err) return } + if skipped > 0 { + app.ui.echomsg(skipMsg("glob-unselect", skipped)) + } case "copy": if err := app.nav.save(clipboardCopy); err != nil { app.ui.echoerrf("copy: %s", err) @@ -1292,6 +1322,13 @@ func (e *callExpr) eval(app *app, _ []string) { app.nav.reload() app.ui.loadFile(app, true) case "delete": + // check the files before dispatch so a custom delete command is refused like the builtin + list, err := app.nav.currFileOrSelections() + if err != nil { + app.ui.echoerrf("delete: %s", err) + return + } + if cmd, ok := gOpts.cmds["delete"]; ok { cmd.eval(app, e.args) app.nav.unselect() @@ -1305,12 +1342,6 @@ func (e *callExpr) eval(app *app, _ []string) { } } } else { - list, err := app.nav.currFileOrSelections() - if err != nil { - app.ui.echoerrf("delete: %s", err) - return - } - if app.ui.cmdPrefix == ">" { return } @@ -1688,11 +1719,18 @@ func (e *callExpr) eval(app *app, _ []string) { dir.visualWrap = 0 case "visual-accept": dir := app.nav.currDir() + skipped := 0 for _, path := range dir.visualSelections() { - if _, ok := app.nav.selections[path]; !ok { - app.nav.selections[path] = app.nav.selectionInd - app.nav.selectionInd++ + if _, ok := app.nav.selections[path]; ok { + continue } + // toggleSelection only refuses newline paths + if err := app.nav.toggleSelection(path); err != nil { + skipped++ + } + } + if skipped > 0 { + app.ui.echomsg(skipMsg("visual-accept", skipped)) } // resetting Visual mode here instead of inside `normal()` // allows us to use Visual mode inside search, find etc. @@ -1864,6 +1902,11 @@ func (e *callExpr) eval(app *app, _ []string) { if !filepath.IsAbs(newPath) { newPath = filepath.Join(wd, newPath) } + // reject before the create-parent prompt so no newline dir is made + if err := checkRenameTarget(oldPath, newPath); err != nil { + app.ui.echoerrf("rename: %s", err) + return + } if oldPath == newPath { return } diff --git a/main.go b/main.go index 7d988794..0f244831 100644 --- a/main.go +++ b/main.go @@ -78,7 +78,12 @@ func exportEnvVars() { if err != nil { fmt.Fprintf(os.Stderr, "getting current directory: %s\n", err) } - os.Setenv("OLDPWD", dir) + if containsNewline(dir) { + // a newline in $OLDPWD would reach shell commands + os.Unsetenv("OLDPWD") + } else { + os.Setenv("OLDPWD", dir) + } level, err := strconv.Atoi(envLevel) if err != nil { diff --git a/misc.go b/misc.go index 2fe15310..6d4b5222 100644 --- a/misc.go +++ b/misc.go @@ -4,6 +4,7 @@ import ( "bufio" "bytes" "cmp" + "errors" "fmt" "io" "io/fs" @@ -592,6 +593,24 @@ func getWidths(wtot int, ratios []int, drawbox bool, borderstyle borderStyle) [] return widths } +// errNewline is the sentinel error for names refused due to a newline or carriage return. +var errNewline = errors.New("name contains a newline") + +// containsNewline reports whether a name or path contains a newline or carriage return. +func containsNewline(s string) bool { + return strings.ContainsAny(s, "\n\r") +} + +// errNewlinePath wraps errNewline with the offending path and a hint to fix it. +func errNewlinePath(path string) error { + return fmt.Errorf("%q: %w; use rename to fix the name", path, errNewline) +} + +// skipMsg formats the notice for names skipped because they contain a newline. +func skipMsg(cmd string, skipped int) string { + return fmt.Sprintf("%s: skipped %d name(s) containing a newline; use rename to fix", cmd, skipped) +} + // We don't need no generic code // We don't need no type control // No dark templates in compiler diff --git a/nav.go b/nav.go index 397aab35..67a89dd9 100644 --- a/nav.go +++ b/nav.go @@ -462,7 +462,7 @@ type nav struct { marks map[string]string renameOldPath string renameNewPath string - selections map[string]int + selections map[string]int // never contains a path with a newline, enforced in toggleSelection tags map[string]string selectionInd int height int @@ -711,34 +711,67 @@ func (nav *nav) position() { } } -func (nav *nav) exportFiles() { +// exportFiles sets $f, $fs, $fv, $fx and $PWD, returning warnings for names dropped due to a newline. +func (nav *nav) exportFiles() []string { + var warnings []string + var currFile string if curr := nav.currFile(); curr != nil { - currFile = quoteString(curr.path) + if containsNewline(curr.path) { + warnings = append(warnings, fmt.Sprintf("shell: left $f empty: %s", errNewlinePath(curr.path))) + } else { + currFile = quoteString(curr.path) + } } var selections []string + dropped := 0 for _, selection := range nav.currSelections() { + // filter stale entries so a regression in toggleSelection cannot reach $fs and $fx + if containsNewline(selection) { + dropped++ + continue + } selections = append(selections, quoteString(selection)) } + if dropped > 0 { + warnings = append(warnings, fmt.Sprintf("shell: dropped %d name(s) with a newline from $fs and $fx", dropped)) + } currSelections := strings.Join(selections, gOpts.filesep) var vSelections []string + dropped = 0 for _, selection := range nav.currDir().visualSelections() { + // $fv is filesep-joined, so refuse newline paths like $f above + if containsNewline(selection) { + dropped++ + continue + } vSelections = append(vSelections, quoteString(selection)) } + if dropped > 0 { + warnings = append(warnings, fmt.Sprintf("shell: dropped %d name(s) with a newline from $fv", dropped)) + } currVSelections := strings.Join(vSelections, gOpts.filesep) os.Setenv("f", currFile) os.Setenv("fs", currSelections) os.Setenv("fv", currVSelections) - os.Setenv("PWD", quoteString(nav.currDir().path)) + if pwd := nav.currDir().path; containsNewline(pwd) { + // a newline in $PWD would reach shell commands + warnings = append(warnings, fmt.Sprintf("shell: unset $PWD: %q: %s", pwd, errNewline)) + os.Unsetenv("PWD") + } else { + os.Setenv("PWD", quoteString(pwd)) + } if len(selections) == 0 { os.Setenv("fx", currFile) } else { os.Setenv("fx", currSelections) } + + return warnings } func (nav *nav) preloadLoop(ui *ui) { @@ -1250,22 +1283,28 @@ func (nav *nav) move(index int) bool { } } -func (nav *nav) toggleSelection(path string) { +func (nav *nav) toggleSelection(path string) error { if _, ok := nav.selections[path]; ok { delete(nav.selections, path) if len(nav.selections) == 0 { nav.selectionInd = 0 } - } else { - nav.selections[path] = nav.selectionInd - nav.selectionInd++ + return nil + } + if containsNewline(path) { + // a newline path must never enter the selections because they are written line by line + return errNewlinePath(path) } + nav.selections[path] = nav.selectionInd + nav.selectionInd++ + return nil } -func (nav *nav) toggle() { +func (nav *nav) toggle() error { if curr := nav.currFile(); curr != nil { - nav.toggleSelection(curr.path) + return nav.toggleSelection(curr.path) } + return nil } func (nav *nav) tagToggleSelection(path, tag string) { @@ -1310,10 +1349,16 @@ func (nav *nav) tag(tag string) error { return nil } -func (nav *nav) invert() { +// invert toggles all files in the current directory, returning the number of newline names skipped. +func (nav *nav) invert() int { + skipped := 0 for _, file := range nav.currDir().files { - nav.toggleSelection(file.path) + // toggleSelection only refuses newline paths + if err := nav.toggleSelection(file.path); err != nil { + skipped++ + } } + return skipped } func (nav *nav) unselect() { @@ -1563,10 +1608,27 @@ func (nav *nav) del(app *app) error { return nil } +// checkRenameTarget refuses a newline in the rename target, exempting components inherited from the directory of oldPath. +func checkRenameTarget(oldPath, newPath string) error { + rel, ok := strings.CutPrefix(newPath, filepath.Dir(oldPath)+string(filepath.Separator)) + if !ok { + rel = newPath + } + if containsNewline(rel) { + return fmt.Errorf("%q: %w", rel, errNewline) + } + return nil +} + func (nav *nav) rename() error { oldPath := nav.renameOldPath newPath := nav.renameNewPath + // refuse creating a newline name (POSIX.1-2024 / Austin Group #251) + if err := checkRenameTarget(oldPath, newPath); err != nil { + return err + } + if err := os.Rename(oldPath, newPath); err != nil { return err } @@ -1637,29 +1699,34 @@ func (nav *nav) cd(path string) error { return nil } -func (nav *nav) globSel(pattern string, invert bool) error { +// globSel toggles files matching pattern, returning the number of newline names skipped. +func (nav *nav) globSel(pattern string, invert bool) (int, error) { dir := nav.currDir() anyMatched := false + skipped := 0 for i := range dir.files { matched, err := filepath.Match(pattern, dir.files[i].Name()) if err != nil { - return fmt.Errorf("glob-select: %w", err) + return skipped, fmt.Errorf("glob-select: %w", err) } if matched { anyMatched = true fpath := filepath.Join(dir.path, dir.files[i].Name()) if _, ok := nav.selections[fpath]; ok == invert { - nav.toggleSelection(fpath) + // toggleSelection only refuses newline paths + if err := nav.toggleSelection(fpath); err != nil { + skipped++ + } } } } if !anyMatched { - return fmt.Errorf("glob-select: pattern not found: %s", pattern) + return skipped, fmt.Errorf("glob-select: pattern not found: %s", pattern) } - return nil + return skipped, nil } func findMatch(name, pattern string) bool { @@ -1881,7 +1948,7 @@ func (nav *nav) writeMarks() error { if strings.Contains(gOpts.tempmarks, k) { continue } - if strings.ContainsAny(nav.marks[k], "\n\r") { + if containsNewline(nav.marks[k]) { log.Printf("marks: skipping mark '%s' with newline in path: %q", k, nav.marks[k]) continue } @@ -1940,7 +2007,7 @@ func (nav *nav) writeTags() error { defer f.Close() for _, k := range slices.Sorted(maps.Keys(nav.tags)) { - if strings.ContainsAny(k, "\n\r") { + if containsNewline(k) { log.Printf("tags: skipping tag with newline in path: %q", k) continue } @@ -1996,12 +2063,17 @@ func (nav *nav) currSelections() []string { return paths } +// currFileOrSelections returns the files a command works on and refuses paths with a newline. func (nav *nav) currFileOrSelections() ([]string, error) { if sel := nav.currSelections(); len(sel) > 0 { return sel, nil } if curr := nav.currFile(); curr != nil { + // selections are checked when they are added, so only the current file needs a check + if containsNewline(curr.path) { + return nil, errNewlinePath(curr.path) + } return []string{curr.path}, nil } diff --git a/newline_test.go b/newline_test.go new file mode 100644 index 00000000..48ae2dc2 --- /dev/null +++ b/newline_test.go @@ -0,0 +1,39 @@ +package main + +import ( + "errors" + "testing" +) + +// selections must never contain a newline path, saveFiles and writeSelection rely on this invariant +func TestToggleSelectionRefusesNewline(t *testing.T) { + nav := &nav{selections: map[string]int{}} + + if err := nav.toggleSelection("/tmp/a\nb"); !errors.Is(err, errNewline) { + t.Errorf("toggleSelection(newline path) = %v, want errNewline", err) + } + if len(nav.selections) != 0 { + t.Error("newline path entered selections") + } + + if err := nav.toggleSelection("/tmp/ok"); err != nil { + t.Errorf("toggleSelection(clean path) = %v, want nil", err) + } + + // unselecting a stale newline entry must still work so users can recover + nav.selections["/tmp/x\ry"] = 5 + if err := nav.toggleSelection("/tmp/x\ry"); err != nil { + t.Errorf("unselecting stale newline entry = %v, want nil", err) + } + if _, ok := nav.selections["/tmp/x\ry"]; ok { + t.Error("stale newline entry not removed") + } +} + +// rename must refuse a newline target before touching the filesystem +func TestRenameRefusesNewlineTarget(t *testing.T) { + nav := &nav{renameOldPath: "/tmp/a", renameNewPath: "/tmp/a\nb"} + if err := nav.rename(); !errors.Is(err, errNewline) { + t.Errorf("rename to newline target = %v, want errNewline", err) + } +} diff --git a/ui.go b/ui.go index d55a0666..afb38810 100644 --- a/ui.go +++ b/ui.go @@ -1300,7 +1300,8 @@ func listJumps(jumps []string, ind int) string { fmt.Fprintln(t, " jump\tpath") // print jumps in order of most recent, Vim uses the opposite order for i := len(jumps) - 1; i >= 0; i-- { - path := jumps[i] + // sanitize the path so a raw newline cannot break the query response + path := sanitizeName(jumps[i]) switch { case i < ind: fmt.Fprintf(t, " %*d\t%s\n", maxlength, ind-i, path) @@ -1354,7 +1355,7 @@ func listFilesInCurrDir(nav *nav) string { b := new(strings.Builder) for _, file := range dir.files { - if strings.ContainsAny(file.path, "\n\r") { + if containsNewline(file.path) { continue } fmt.Fprintln(b, file.path)