Skip to content

Commit db9abab

Browse files
markijbemamarkijbema
committed
add proper support for carriage return
1 parent 4010b6f commit db9abab

File tree

2 files changed

+70
-47
lines changed

2 files changed

+70
-47
lines changed

‎webview-ui/src/utils/__tests__/command-validation.spec.ts‎

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -14,28 +14,30 @@ import {
1414
containsDangerousSubstitution,
1515
protectNewlinesInQuotes,
1616
NEWLINE_PLACEHOLDER,
17+
CARRIAGE_RETURN_PLACEHOLDER,
1718
} from "../command-validation"
1819

1920
describe("protectNewlinesInQuotes", () => {
20-
const placeholder = NEWLINE_PLACEHOLDER
21+
const newlinePlaceholder = NEWLINE_PLACEHOLDER
22+
const crPlaceholder = CARRIAGE_RETURN_PLACEHOLDER
2123

2224
describe("basic quote handling", () => {
2325
it("protects newlines in double quotes", () => {
2426
const input = 'echo "hello\nworld"'
25-
const expected = `echo "hello${placeholder}world"`
26-
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
27+
const expected = `echo "hello${newlinePlaceholder}world"`
28+
expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected)
2729
})
2830

2931
it("protects newlines in single quotes", () => {
3032
const input = "echo 'hello\nworld'"
31-
const expected = `echo 'hello${placeholder}world'`
32-
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
33+
const expected = `echo 'hello${newlinePlaceholder}world'`
34+
expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected)
3335
})
3436

3537
it("does not protect newlines outside quotes", () => {
3638
const input = "echo hello\necho world"
3739
const expected = "echo hello\necho world"
38-
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
40+
expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected)
3941
})
4042
})
4143

@@ -45,91 +47,91 @@ describe("protectNewlinesInQuotes", () => {
4547
const input = `echo '"'A\n'"'`
4648
// The newline after A is NOT inside quotes, so it should NOT be protected
4749
const expected = `echo '"'A\n'"'`
48-
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
50+
expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected)
4951
})
5052

5153
it("handles alternating quotes correctly", () => {
5254
// echo "hello"world"test" -> hello is quoted, world is not, test is quoted
5355
const input = `echo "hello\n"world\n"test\n"`
54-
const expected = `echo "hello${placeholder}"world\n"test${placeholder}"`
55-
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
56+
const expected = `echo "hello${newlinePlaceholder}"world\n"test${newlinePlaceholder}"`
57+
expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected)
5658
})
5759

5860
it("handles single quote after double quote", () => {
5961
const input = `echo "hello"'world\n'`
60-
const expected = `echo "hello"'world${placeholder}'`
61-
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
62+
const expected = `echo "hello"'world${newlinePlaceholder}'`
63+
expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected)
6264
})
6365

6466
it("handles double quote after single quote", () => {
6567
const input = `echo 'hello'"world\n"`
66-
const expected = `echo 'hello'"world${placeholder}"`
67-
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
68+
const expected = `echo 'hello'"world${newlinePlaceholder}"`
69+
expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected)
6870
})
6971
})
7072

7173
describe("escaped quotes", () => {
7274
it("handles escaped double quotes in double-quoted strings", () => {
7375
const input = 'echo "hello\\"world\n"'
74-
const expected = `echo "hello\\"world${placeholder}"`
75-
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
76+
const expected = `echo "hello\\"world${newlinePlaceholder}"`
77+
expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected)
7678
})
7779

7880
it("does not treat backslash as escape in single quotes", () => {
7981
// In single quotes, backslash is literal (except for \' in some shells)
8082
const input = "echo 'hello\\'world\n'"
8183
// The \\ is literal, the ' ends the quote, so world\n is outside quotes
8284
const expected = "echo 'hello\\'world\n'"
83-
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
85+
expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected)
8486
})
8587
})
8688

8789
describe("edge cases", () => {
8890
it("handles unclosed quotes", () => {
8991
const input = 'echo "unclosed\n'
90-
const expected = `echo "unclosed${placeholder}`
91-
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
92+
const expected = `echo "unclosed${newlinePlaceholder}`
93+
expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected)
9294
})
9395

9496
it("handles empty string", () => {
95-
expect(protectNewlinesInQuotes("", placeholder)).toBe("")
97+
expect(protectNewlinesInQuotes("", newlinePlaceholder, crPlaceholder)).toBe("")
9698
})
9799

98100
it("handles string with no quotes", () => {
99101
const input = "echo hello\nworld"
100-
expect(protectNewlinesInQuotes(input, placeholder)).toBe(input)
102+
expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(input)
101103
})
102104

103105
it("handles multiple newlines in quotes", () => {
104106
const input = 'echo "line1\nline2\nline3"'
105-
const expected = `echo "line1${placeholder}line2${placeholder}line3"`
106-
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
107+
const expected = `echo "line1${newlinePlaceholder}line2${newlinePlaceholder}line3"`
108+
expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected)
107109
})
108110

109111
it("handles carriage returns", () => {
110112
const input = 'echo "hello\rworld"'
111-
const expected = `echo "hello${placeholder}world"`
112-
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
113+
const expected = `echo "hello${crPlaceholder}world"`
114+
expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected)
113115
})
114116

115117
it("handles CRLF", () => {
116118
const input = 'echo "hello\r\nworld"'
117-
const expected = `echo "hello${placeholder}${placeholder}world"`
118-
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
119+
const expected = `echo "hello${crPlaceholder}${newlinePlaceholder}world"`
120+
expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected)
119121
})
120122
})
121123

122124
describe("real-world git commit examples", () => {
123125
it("protects newlines in git commit message", () => {
124126
const input = `git commit -m "feat: title\n\n- point a\n- point b"`
125-
const expected = `git commit -m "feat: title${placeholder}${placeholder}- point a${placeholder}- point b"`
126-
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
127+
const expected = `git commit -m "feat: title${newlinePlaceholder}${newlinePlaceholder}- point a${newlinePlaceholder}- point b"`
128+
expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected)
127129
})
128130

129131
it("handles complex git command with multiple quoted sections", () => {
130132
const input = `git add . && git commit -m "feat: title\n\n- point a" && echo "done\n"`
131-
const expected = `git add . && git commit -m "feat: title${placeholder}${placeholder}- point a" && echo "done${placeholder}"`
132-
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
133+
const expected = `git add . && git commit -m "feat: title${newlinePlaceholder}${newlinePlaceholder}- point a" && echo "done${newlinePlaceholder}"`
134+
expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected)
133135
})
134136
})
135137
})

‎webview-ui/src/utils/command-validation.ts‎

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@ import { parse } from "shell-quote"
33
type ShellToken = string | { op: string } | { command: string }
44

55
/**
6-
* Placeholder used to protect newlines within quoted strings during command parsing.
7-
* This constant is used by the protectNewlinesInQuotes function to temporarily replace
6+
* Placeholders used to protect newlines within quoted strings during command parsing.
7+
* These constants are used by the protectNewlinesInQuotes function to temporarily replace
88
* newlines that appear inside quotes, preventing them from being treated as command separators.
9+
* We use separate placeholders for \n and \r to preserve the original line ending type.
910
*/
1011
export const NEWLINE_PLACEHOLDER = "___NEWLINE___"
12+
export const CARRIAGE_RETURN_PLACEHOLDER = "___CARRIAGE_RETURN___"
1113

1214
/**
1315
* # Command Denylist Feature - Longest Prefix Match Strategy
@@ -130,8 +132,9 @@ export function containsDangerousSubstitution(source: string): boolean {
130132
}
131133

132134
/**
133-
* Protect newlines inside quoted strings by replacing them with a placeholder.
135+
* Protect newlines inside quoted strings by replacing them with placeholders.
134136
* This handles proper shell quoting rules where quotes can be concatenated.
137+
* Uses separate placeholders for \n and \r to preserve the original line ending type.
135138
*
136139
* Examples:
137140
* - "hello\nworld" -> newline is protected (inside double quotes)
@@ -140,10 +143,15 @@ export function containsDangerousSubstitution(source: string): boolean {
140143
* - "hello"world -> world is NOT quoted
141144
*
142145
* @param command - The command string to process
143-
* @param placeholder - The placeholder string to use for protected newlines
144-
* @returns The command with newlines in quotes replaced by placeholder
146+
* @param newlinePlaceholder - The placeholder string to use for \n characters
147+
* @param carriageReturnPlaceholder - The placeholder string to use for \r characters
148+
* @returns The command with newlines in quotes replaced by placeholders
145149
*/
146-
export function protectNewlinesInQuotes(command: string, placeholder: string): string {
150+
export function protectNewlinesInQuotes(
151+
command: string,
152+
newlinePlaceholder: string,
153+
carriageReturnPlaceholder: string,
154+
): string {
147155
let result = ""
148156
let i = 0
149157

@@ -165,9 +173,13 @@ export function protectNewlinesInQuotes(command: string, placeholder: string): s
165173
result += quoteChar
166174
i++
167175
break
168-
} else if (quoteChar === "\n" || quoteChar === "\r") {
169-
// Replace newline inside double quotes
170-
result += placeholder
176+
} else if (quoteChar === "\n") {
177+
// Replace \n inside double quotes
178+
result += newlinePlaceholder
179+
i++
180+
} else if (quoteChar === "\r") {
181+
// Replace \r inside double quotes
182+
result += carriageReturnPlaceholder
171183
i++
172184
} else {
173185
result += quoteChar
@@ -189,9 +201,13 @@ export function protectNewlinesInQuotes(command: string, placeholder: string): s
189201
result += quoteChar
190202
i++
191203
break
192-
} else if (quoteChar === "\n" || quoteChar === "\r") {
193-
// Replace newline inside single quotes
194-
result += placeholder
204+
} else if (quoteChar === "\n") {
205+
// Replace \n inside single quotes
206+
result += newlinePlaceholder
207+
i++
208+
} else if (quoteChar === "\r") {
209+
// Replace \r inside single quotes
210+
result += carriageReturnPlaceholder
195211
i++
196212
} else {
197213
result += quoteChar
@@ -222,10 +238,11 @@ export function protectNewlinesInQuotes(command: string, placeholder: string): s
222238
export function parseCommand(command: string): string[] {
223239
if (!command?.trim()) return []
224240

225-
// First, protect newlines inside quoted strings by replacing them with a placeholder
241+
// First, protect newlines inside quoted strings by replacing them with placeholders
226242
// This prevents splitting multi-line quoted strings (e.g., git commit -m "multi\nline")
227-
const quotedStringPlaceholder = "___NEWLINE_IN_QUOTE___"
228-
const protectedCommand = protectNewlinesInQuotes(command, quotedStringPlaceholder)
243+
const newlinePlaceholder = "___NEWLINE_IN_QUOTE___"
244+
const carriageReturnPlaceholder = "___CR_IN_QUOTE___"
245+
const protectedCommand = protectNewlinesInQuotes(command, newlinePlaceholder, carriageReturnPlaceholder)
229246

230247
// Split by newlines (handle different line ending formats)
231248
// This regex splits on \r\n (Windows), \n (Unix), or \r (old Mac)
@@ -241,8 +258,12 @@ export function parseCommand(command: string): string[] {
241258
allCommands.push(...lineCommands)
242259
}
243260

244-
// Restore newlines in quoted strings
245-
return allCommands.map((cmd) => cmd.replace(new RegExp(quotedStringPlaceholder, "g"), "\n"))
261+
// Restore newlines and carriage returns in quoted strings
262+
return allCommands.map((cmd) =>
263+
cmd
264+
.replace(new RegExp(newlinePlaceholder, "g"), "\n")
265+
.replace(new RegExp(carriageReturnPlaceholder, "g"), "\r"),
266+
)
246267
}
247268

248269
/**

0 commit comments

Comments
 (0)