From b040ee66ab483ec44ae0a57634e6ee732a87bdb9 Mon Sep 17 00:00:00 2001 From: Christopher Milne-O'Grady Date: Fri, 1 Nov 2024 19:27:07 +0000 Subject: [PATCH 1/4] expand: fix sequential '\' in double quote literals --- expand/expand.go | 29 +++++++++++++--------- expand/expand_test.go | 58 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 12 deletions(-) diff --git a/expand/expand.go b/expand/expand.go index fe285a2e..8cc0c1ce 100644 --- a/expand/expand.go +++ b/expand/expand.go @@ -503,18 +503,7 @@ func (cfg *Config) wordField(wps []syntax.WordPart, ql quoteLevel) ([]fieldPart, } } if ql == quoteDouble && strings.Contains(s, "\\") { - sb := cfg.strBuilder() - for i := 0; i < len(s); i++ { - b := s[i] - if b == '\\' && i+1 < len(s) { - switch s[i+1] { - case '"', '\\', '$', '`': // special chars - continue - } - } - sb.WriteByte(b) - } - s = sb.String() + s = cfg.handleDoubleQuotedEscapeCharacters(s) } if i := strings.IndexByte(s, '\x00'); i >= 0 { s = s[:i] @@ -566,6 +555,22 @@ func (cfg *Config) wordField(wps []syntax.WordPart, ql quoteLevel) ([]fieldPart, return field, nil } +func (cfg *Config) handleDoubleQuotedEscapeCharacters(s string) string { + buf := cfg.strBuilder() + for i := 0; i < len(s); i++ { + b := s[i] + if b == '\\' && i+1 < len(s) { + switch s[i+1] { + case '"', '\\', '$', '`': // special chars + i++ + b = s[i] // write the special char, skipping the backslash + } + } + buf.WriteByte(b) + } + return buf.String() +} + func (cfg *Config) cmdSubst(cs *syntax.CmdSubst) (string, error) { if cfg.CmdSubst == nil { return "", UnexpectedCommandError{Node: cs} diff --git a/expand/expand_test.go b/expand/expand_test.go index ede52c5e..c19dfa0e 100644 --- a/expand/expand_test.go +++ b/expand/expand_test.go @@ -140,3 +140,61 @@ var _ fs.DirEntry = (*mockFileInfo)(nil) func (fi *mockFileInfo) Name() string { return fi.name } func (fi *mockFileInfo) Type() fs.FileMode { return fi.typ } + +func TestBackslashInDoublequote(t *testing.T) { + tests := []struct { + test string + src string + want string + }{ + { + test: `Backslash for special character (\) preserves special character`, + src: `Hello \\ World`, + want: `Hello \ World`, + }, + { + test: `Backslash for special character (") preserves special character`, + src: `Hello \" World`, + want: `Hello " World`, + }, + { + test: `Backslash for special character ($) preserves special character`, + src: `Hello \$ World`, + want: `Hello $ World`, + }, + { + test: "Backslash for special character (`) preserves special character", + src: "Hello \\` World", + want: "Hello ` World", + }, + { + test: "Backslash for non-special character (b) is preserved", + src: `Hello \b World`, + want: `Hello \b World`, + }, + { + test: "No backkslash is no-op", + src: `Hello World`, + want: `Hello World`, + }, + { + test: `Multiple special characters are be preserved`, + src: `Hello \"\"\" World`, + want: `Hello """ World`, + }, + { + test: `Multiple backslashes are preserved`, + src: `Hello \\\\ World`, + want: `Hello \\ World`, + }, + } + cfg := &Config{} + for _, tc := range tests { + t.Run(tc.test, func(t *testing.T) { + got := cfg.handleDoubleQuotedEscapeCharacters(tc.src) + if got != tc.want { + t.Fatalf("wanted '%s', got '%s'", tc.want, got) + } + }) + } +} From 18de4421474f4ce5e99e926592d6732f3be4de18 Mon Sep 17 00:00:00 2001 From: ogchris Date: Tue, 5 Nov 2024 14:56:43 +0000 Subject: [PATCH 2/4] expand: inline double quote escape character handling Use interp_test instead of expand_test --- expand/expand.go | 30 ++++++++++------------ expand/expand_test.go | 58 ------------------------------------------- interp/interp_test.go | 5 ++++ 3 files changed, 18 insertions(+), 75 deletions(-) diff --git a/expand/expand.go b/expand/expand.go index 8cc0c1ce..9a5362ef 100644 --- a/expand/expand.go +++ b/expand/expand.go @@ -503,7 +503,19 @@ func (cfg *Config) wordField(wps []syntax.WordPart, ql quoteLevel) ([]fieldPart, } } if ql == quoteDouble && strings.Contains(s, "\\") { - s = cfg.handleDoubleQuotedEscapeCharacters(s) + buf := cfg.strBuilder() + for i := 0; i < len(s); i++ { + b := s[i] + if b == '\\' && i+1 < len(s) { + switch s[i+1] { + case '"', '\\', '$', '`': // special chars + i++ + b = s[i] // write the special char, skipping the backslash + } + } + buf.WriteByte(b) + } + s = buf.String() } if i := strings.IndexByte(s, '\x00'); i >= 0 { s = s[:i] @@ -555,22 +567,6 @@ func (cfg *Config) wordField(wps []syntax.WordPart, ql quoteLevel) ([]fieldPart, return field, nil } -func (cfg *Config) handleDoubleQuotedEscapeCharacters(s string) string { - buf := cfg.strBuilder() - for i := 0; i < len(s); i++ { - b := s[i] - if b == '\\' && i+1 < len(s) { - switch s[i+1] { - case '"', '\\', '$', '`': // special chars - i++ - b = s[i] // write the special char, skipping the backslash - } - } - buf.WriteByte(b) - } - return buf.String() -} - func (cfg *Config) cmdSubst(cs *syntax.CmdSubst) (string, error) { if cfg.CmdSubst == nil { return "", UnexpectedCommandError{Node: cs} diff --git a/expand/expand_test.go b/expand/expand_test.go index c19dfa0e..ede52c5e 100644 --- a/expand/expand_test.go +++ b/expand/expand_test.go @@ -140,61 +140,3 @@ var _ fs.DirEntry = (*mockFileInfo)(nil) func (fi *mockFileInfo) Name() string { return fi.name } func (fi *mockFileInfo) Type() fs.FileMode { return fi.typ } - -func TestBackslashInDoublequote(t *testing.T) { - tests := []struct { - test string - src string - want string - }{ - { - test: `Backslash for special character (\) preserves special character`, - src: `Hello \\ World`, - want: `Hello \ World`, - }, - { - test: `Backslash for special character (") preserves special character`, - src: `Hello \" World`, - want: `Hello " World`, - }, - { - test: `Backslash for special character ($) preserves special character`, - src: `Hello \$ World`, - want: `Hello $ World`, - }, - { - test: "Backslash for special character (`) preserves special character", - src: "Hello \\` World", - want: "Hello ` World", - }, - { - test: "Backslash for non-special character (b) is preserved", - src: `Hello \b World`, - want: `Hello \b World`, - }, - { - test: "No backkslash is no-op", - src: `Hello World`, - want: `Hello World`, - }, - { - test: `Multiple special characters are be preserved`, - src: `Hello \"\"\" World`, - want: `Hello """ World`, - }, - { - test: `Multiple backslashes are preserved`, - src: `Hello \\\\ World`, - want: `Hello \\ World`, - }, - } - cfg := &Config{} - for _, tc := range tests { - t.Run(tc.test, func(t *testing.T) { - got := cfg.handleDoubleQuotedEscapeCharacters(tc.src) - if got != tc.want { - t.Fatalf("wanted '%s', got '%s'", tc.want, got) - } - }) - } -} diff --git a/interp/interp_test.go b/interp/interp_test.go index 6462145a..6c9262f0 100644 --- a/interp/interp_test.go +++ b/interp/interp_test.go @@ -400,6 +400,11 @@ var runTests = []runTest{ {`echo \\`, "\\\n"}, {`echo \\\\`, "\\\\\n"}, {`echo \`, "\n"}, + + // escape characters in double quote literal + {`echo "\\"`, "\\\n"}, // special character is preserved + {`echo "\b"`, "\\b\n"}, // non-special character has both characters preserved + {`echo "\\\\"`, "\\\\\n"}, // sequential backslashes (escape characters repeated sequentially) // vars {"foo_interp_missing=bar_interp_missing; echo $foo_interp_missing", "bar_interp_missing\n"}, From edb5bbc6144a7e4cec16be05130862b1fc217c34 Mon Sep 17 00:00:00 2001 From: ogchris Date: Tue, 5 Nov 2024 14:59:01 +0000 Subject: [PATCH 3/4] keep original string builder name. --- expand/expand.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/expand/expand.go b/expand/expand.go index 9a5362ef..121cbb63 100644 --- a/expand/expand.go +++ b/expand/expand.go @@ -503,7 +503,7 @@ func (cfg *Config) wordField(wps []syntax.WordPart, ql quoteLevel) ([]fieldPart, } } if ql == quoteDouble && strings.Contains(s, "\\") { - buf := cfg.strBuilder() + sb := cfg.strBuilder() for i := 0; i < len(s); i++ { b := s[i] if b == '\\' && i+1 < len(s) { @@ -513,9 +513,9 @@ func (cfg *Config) wordField(wps []syntax.WordPart, ql quoteLevel) ([]fieldPart, b = s[i] // write the special char, skipping the backslash } } - buf.WriteByte(b) + sb.WriteByte(b) } - s = buf.String() + s = sb.String() } if i := strings.IndexByte(s, '\x00'); i >= 0 { s = s[:i] From c416907cb7eb85e53c297d16bb9eb639fa4f6c18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Fri, 8 Nov 2024 10:04:40 +0000 Subject: [PATCH 4/4] gofmt --- interp/interp_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/interp/interp_test.go b/interp/interp_test.go index 6c9262f0..de7ca44c 100644 --- a/interp/interp_test.go +++ b/interp/interp_test.go @@ -400,10 +400,10 @@ var runTests = []runTest{ {`echo \\`, "\\\n"}, {`echo \\\\`, "\\\\\n"}, {`echo \`, "\n"}, - + // escape characters in double quote literal - {`echo "\\"`, "\\\n"}, // special character is preserved - {`echo "\b"`, "\\b\n"}, // non-special character has both characters preserved + {`echo "\\"`, "\\\n"}, // special character is preserved + {`echo "\b"`, "\\b\n"}, // non-special character has both characters preserved {`echo "\\\\"`, "\\\\\n"}, // sequential backslashes (escape characters repeated sequentially) // vars