fix: Windows escaping issues (#314)

* fix: Windows escaping issue Signed-off-by: Valentin Kiselev <[email protected]> * feature: Wrap every file in quotes if they are provided Signed-off-by: Valentin Kiselev <[email protected]> * fix: Fix shellquote, replace quotes if explicitly set Signed-off-by: Valentin Kiselev <[email protected]> * docs: Write about quoting of shorthands Signed-off-by: Valentin Kiselev <[email protected]> * test: Add a test for replaceQuoted Signed-off-by: Valentin Kiselev <[email protected]> Signed-off-by: Valentin Kiselev <[email protected]>
evilmartians · Aug 11, 2022 · f91d56f · f91d56f
1 parent 5c83629
commit f91d56f
Show file tree

Hide file tree

Showing 4 changed files with 138 additions and 11 deletions.
diff --git a/docs/full_guide.md b/docs/full_guide.md
@@ -168,6 +168,24 @@ pre-commit:
 
 Note: If using `all_files` with RuboCop, it will ignore RuboCop's `Exclude` configuration setting. To avoid this, pass `--force-exclusion`.
 
+If you want to have all you files quoted with double quotes `"` or single quotes `'`, quote the appropriate shorthand:
+
+```yml
+pre-commit
+ commands:
+ lint:
+ glob: "*.js"
+ # Quoting with double quotes `"` might be helpful for Windows users
+ run: yarn eslint "{staged_files}" # will run `yarn eslint "file1.js" "file2.js" "[strange name].js"`
+ test:
+ glob: "*.{spec.js}"
+ run: yarn test '{staged_files}' # will run `yarn eslint 'file1.spec.js' 'file2.spec.js' '[strange name].spec.js'`
+ format:
+ glob: "*.js"
+ # Will quote where needed with single quotes
+ run: yarn test {staged_files} # will run `yarn eslint file1.js file2.js '[strange name].spec.js'`
+```
+
 ## Custom file list
 
 Lefthook can be even more specific in selecting files.

diff --git a/internal/lefthook/runner/execute_windows.go b/internal/lefthook/runner/execute_windows.go
@@ -5,12 +5,18 @@ import (
  "os"
  "os/exec"
  "path/filepath"
+ "strings"
+ "syscall"
 )
 
 type CommandExecutor struct{}
 
 func (e CommandExecutor) Execute(root string, args []string) (*bytes.Buffer, error) {
- command := exec.Command(args[0], args[1:]...)
+ command := exec.Command(args[0])
+ command.SysProcAttr = &syscall.SysProcAttr{
+ CmdLine: strings.Join(args, " "),
+ }
+
  rootDir, _ := filepath.Abs(root)
  command.Dir = rootDir
 

diff --git a/internal/lefthook/runner/runner.go b/internal/lefthook/runner/runner.go
@@ -5,6 +5,7 @@ import (
  "fmt"
  "os"
  "path/filepath"
+ "regexp"
  "sort"
  "strings"
  "sync"
@@ -24,6 +25,8 @@ const (
  executableMask os.FileMode = 0o111
 )
 
+var surroundingQuotesRegexp = regexp.MustCompile(`^'(.*)'$`)
+
 type Runner struct {
  fs afero.Fs
  repo *git.Repository
@@ -284,14 +287,12 @@ func (r *Runner) buildCommandArgs(command *config.Command) []string {
  return nil
  }
 
- filesStr := prepareFiles(command, files)
- if len(filesStr) == 0 {
+ filesPrepared := prepareFiles(command, files)
+ if len(filesPrepared) == 0 {
  return nil
  }
 
- runString = strings.ReplaceAll(
- runString, filesType, filesStr,
- )
+ runString = replaceQuoted(runString, filesType, filesPrepared)
  }
  }
 
@@ -300,12 +301,14 @@ func (r *Runner) buildCommandArgs(command *config.Command) []string {
  runString = strings.ReplaceAll(runString, fmt.Sprintf("{%d}", i+1), gitArg)
  }
 
+ log.Debug("Executing command is: ", runString)
+
  return strings.Split(runString, " ")
 }
 
-func prepareFiles(command *config.Command, files []string) string {
+func prepareFiles(command *config.Command, files []string) []string {
  if files == nil {
- return ""
+ return []string{}
  }
 
  log.Debug("Files before filters:\n", files)
@@ -323,11 +326,39 @@ func prepareFiles(command *config.Command, files []string) string {
  filesEsc = append(filesEsc, shellescape.Quote(fileName))
  }
  }
- files = filesEsc
 
- log.Debug("Files after escaping:\n", files)
+ log.Debug("Files after escaping:\n", filesEsc)
+
+ return filesEsc
+}
+
+func replaceQuoted(source, substitution string, files []string) string {
+ for _, elem := range [][]string{
+ {"\"", "\"" + substitution + "\""},
+ {"'", "'" + substitution + "'"},
+ {"", substitution},
+ } {
+ quote := elem[0]
+ sub := elem[1]
+ if !strings.Contains(source, sub) {
+ continue
+ }
+
+ quotedFiles := files
+ if len(quote) != 0 {
+ quotedFiles = make([]string, 0, len(files))
+ for _, fileName := range files {
+ quotedFiles = append(quotedFiles,
+ quote+surroundingQuotesRegexp.ReplaceAllString(fileName, "$1")+quote)
+ }
+ }
+
+ source = strings.ReplaceAll(
+ source, sub, strings.Join(quotedFiles, " "),
+ )
+ }
 
- return strings.Join(files, " ")
+ return source
 }
 
 func (r *Runner) run(name, root, failText string, args []string) {

diff --git a/internal/lefthook/runner/runner_test.go b/internal/lefthook/runner/runner_test.go
@@ -274,3 +274,75 @@ func resultsMatch(a, b []Result) bool {
 
  return true
 }
+
+func TestReplaceQuoted(t *testing.T) {
+ for i, tt := range [...]struct {
+ name, source, substitution string
+ files []string
+ result string
+ }{
+ {
+ name: "without substitutions",
+ source: "echo",
+ substitution: "{staged_files}",
+ files: []string{"a", "b"},
+ result: "echo",
+ },
+ {
+ name: "with simple substitution",
+ source: "echo {staged_files}",
+ substitution: "{staged_files}",
+ files: []string{"test.rb", "README"},
+ result: "echo test.rb README",
+ },
+ {
+ name: "with single quoted substitution",
+ source: "echo '{staged_files}'",
+ substitution: "{staged_files}",
+ files: []string{"test.rb", "README"},
+ result: "echo 'test.rb' 'README'",
+ },
+ {
+ name: "with double quoted substitution",
+ source: `echo "{staged_files}"`,
+ substitution: "{staged_files}",
+ files: []string{"test.rb", "README"},
+ result: `echo "test.rb" "README"`,
+ },
+ {
+ name: "with escaped files double quoted",
+ source: `echo "{staged_files}"`,
+ substitution: "{staged_files}",
+ files: []string{"'test me.rb'", "README"},
+ result: `echo "test me.rb" "README"`,
+ },
+ {
+ name: "with escaped files single quoted",
+ source: "echo '{staged_files}'",
+ substitution: "{staged_files}",
+ files: []string{"'test me.rb'", "README"},
+ result: `echo 'test me.rb' 'README'`,
+ },
+ {
+ name: "with escaped files",
+ source: "echo {staged_files}",
+ substitution: "{staged_files}",
+ files: []string{"'test me.rb'", "README"},
+ result: `echo 'test me.rb' README`,
+ },
+ {
+ name: "with many substitutions",
+ source: `echo "{staged_files}" {staged_files}`,
+ substitution: "{staged_files}",
+ files: []string{"'test me.rb'", "README"},
+ result: `echo "test me.rb" "README" 'test me.rb' README`,
+ },
+ } {
+ t.Run(fmt.Sprintf("%d: %s", i, tt.name), func(t *testing.T) {
+ result := replaceQuoted(tt.source, tt.substitution, tt.files)
+ if result != tt.result {
+ t.Errorf("Expected `%s` to eq `%s`", result, tt.result)
+ }
+ })
+ }
+}