fix: Windows escaping issues (#314)

* fix: Windows escaping issue Signed-off-by: Valentin Kiselev <mrexox@evilmartians.com> * feature: Wrap every file in quotes if they are provided Signed-off-by: Valentin Kiselev <mrexox@evilmartians.com> * fix: Fix shellquote, replace quotes if explicitly set Signed-off-by: Valentin Kiselev <mrexox@evilmartians.com> * docs: Write about quoting of shorthands Signed-off-by: Valentin Kiselev <mrexox@evilmartians.com> * test: Add a test for replaceQuoted Signed-off-by: Valentin Kiselev <mrexox@evilmartians.com> Signed-off-by: Valentin Kiselev <mrexox@evilmartians.com>
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)
+			}
+		})
+	}
+}