Skip to content

Commit de425a7

Browse files
markijbemamarkijbema
committed
refactor: Extract and improve quote handling in command parser
- Extract protectNewlinesInQuotes() as a separate, testable function - Fix quote concatenation handling (e.g., echo '"'A'"' where A is NOT quoted) - Properly track quote boundaries without using toggle-based state - Handle escaped quotes correctly in double-quoted strings - Add comprehensive test suite for protectNewlinesInQuotes(): - Basic quote handling (single/double quotes) - Quote concatenation edge cases - Escaped quotes in different contexts - Unclosed quotes, empty strings, multiple newlines - Real-world git commit message examples - All 153 tests pass
1 parent 129ec91 commit de425a7

File tree

2 files changed

+199
-27
lines changed

2 files changed

+199
-27
lines changed

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

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,127 @@ import {
1212
CommandValidator,
1313
createCommandValidator,
1414
containsDangerousSubstitution,
15+
protectNewlinesInQuotes,
1516
} from "../command-validation"
1617

18+
describe("protectNewlinesInQuotes", () => {
19+
const placeholder = "___NEWLINE___"
20+
21+
describe("basic quote handling", () => {
22+
it("protects newlines in double quotes", () => {
23+
const input = 'echo "hello\nworld"'
24+
const expected = `echo "hello${placeholder}world"`
25+
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
26+
})
27+
28+
it("protects newlines in single quotes", () => {
29+
const input = "echo 'hello\nworld'"
30+
const expected = `echo 'hello${placeholder}world'`
31+
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
32+
})
33+
34+
it("does not protect newlines outside quotes", () => {
35+
const input = "echo hello\necho world"
36+
const expected = "echo hello\necho world"
37+
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
38+
})
39+
})
40+
41+
describe("quote concatenation", () => {
42+
it("handles quote concatenation where content between quotes is NOT quoted", () => {
43+
// In bash: echo '"'A'"' prints "A" (A is not quoted)
44+
const input = `echo '"'A\n'"'`
45+
// The newline after A is NOT inside quotes, so it should NOT be protected
46+
const expected = `echo '"'A\n'"'`
47+
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
48+
})
49+
50+
it("handles alternating quotes correctly", () => {
51+
// echo "hello"world"test" -> hello is quoted, world is not, test is quoted
52+
const input = `echo "hello\n"world\n"test\n"`
53+
const expected = `echo "hello${placeholder}"world\n"test${placeholder}"`
54+
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
55+
})
56+
57+
it("handles single quote after double quote", () => {
58+
const input = `echo "hello"'world\n'`
59+
const expected = `echo "hello"'world${placeholder}'`
60+
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
61+
})
62+
63+
it("handles double quote after single quote", () => {
64+
const input = `echo 'hello'"world\n"`
65+
const expected = `echo 'hello'"world${placeholder}"`
66+
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
67+
})
68+
})
69+
70+
describe("escaped quotes", () => {
71+
it("handles escaped double quotes in double-quoted strings", () => {
72+
const input = 'echo "hello\\"world\n"'
73+
const expected = `echo "hello\\"world${placeholder}"`
74+
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
75+
})
76+
77+
it("does not treat backslash as escape in single quotes", () => {
78+
// In single quotes, backslash is literal (except for \' in some shells)
79+
const input = "echo 'hello\\'world\n'"
80+
// The \\ is literal, the ' ends the quote, so world\n is outside quotes
81+
const expected = "echo 'hello\\'world\n'"
82+
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
83+
})
84+
})
85+
86+
describe("edge cases", () => {
87+
it("handles unclosed quotes", () => {
88+
const input = 'echo "unclosed\n'
89+
const expected = `echo "unclosed${placeholder}`
90+
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
91+
})
92+
93+
it("handles empty string", () => {
94+
expect(protectNewlinesInQuotes("", placeholder)).toBe("")
95+
})
96+
97+
it("handles string with no quotes", () => {
98+
const input = "echo hello\nworld"
99+
expect(protectNewlinesInQuotes(input, placeholder)).toBe(input)
100+
})
101+
102+
it("handles multiple newlines in quotes", () => {
103+
const input = 'echo "line1\nline2\nline3"'
104+
const expected = `echo "line1${placeholder}line2${placeholder}line3"`
105+
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
106+
})
107+
108+
it("handles carriage returns", () => {
109+
const input = 'echo "hello\rworld"'
110+
const expected = `echo "hello${placeholder}world"`
111+
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
112+
})
113+
114+
it("handles CRLF", () => {
115+
const input = 'echo "hello\r\nworld"'
116+
const expected = `echo "hello${placeholder}${placeholder}world"`
117+
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
118+
})
119+
})
120+
121+
describe("real-world git commit examples", () => {
122+
it("protects newlines in git commit message", () => {
123+
const input = `git commit -m "feat: title\n\n- point a\n- point b"`
124+
const expected = `git commit -m "feat: title${placeholder}${placeholder}- point a${placeholder}- point b"`
125+
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
126+
})
127+
128+
it("handles complex git command with multiple quoted sections", () => {
129+
const input = `git add . && git commit -m "feat: title\n\n- point a" && echo "done\n"`
130+
const expected = `git add . && git commit -m "feat: title${placeholder}${placeholder}- point a" && echo "done${placeholder}"`
131+
expect(protectNewlinesInQuotes(input, placeholder)).toBe(expected)
132+
})
133+
})
134+
})
135+
17136
describe("Command Validation", () => {
18137
describe("parseCommand", () => {
19138
it("splits commands by chain operators", () => {

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

Lines changed: 80 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,85 @@ export function containsDangerousSubstitution(source: string): boolean {
122122
)
123123
}
124124

125+
/**
126+
* Protect newlines inside quoted strings by replacing them with a placeholder.
127+
* This handles proper shell quoting rules where quotes can be concatenated.
128+
*
129+
* Examples:
130+
* - "hello\nworld" -> newline is protected (inside double quotes)
131+
* - 'hello\nworld' -> newline is protected (inside single quotes)
132+
* - echo '"'A'"' -> A is NOT quoted (quote concatenation)
133+
* - "hello"world -> world is NOT quoted
134+
*
135+
* @param command - The command string to process
136+
* @param placeholder - The placeholder string to use for protected newlines
137+
* @returns The command with newlines in quotes replaced by placeholder
138+
*/
139+
export function protectNewlinesInQuotes(command: string, placeholder: string): string {
140+
let result = ""
141+
let i = 0
142+
143+
while (i < command.length) {
144+
const char = command[i]
145+
146+
if (char === '"') {
147+
// Start of double-quoted string
148+
result += char
149+
i++
150+
151+
// Process until we find the closing unescaped quote
152+
while (i < command.length) {
153+
const quoteChar = command[i]
154+
const prevChar = i > 0 ? command[i - 1] : ""
155+
156+
if (quoteChar === '"' && prevChar !== "\\") {
157+
// Found closing quote
158+
result += quoteChar
159+
i++
160+
break
161+
} else if (quoteChar === "\n" || quoteChar === "\r") {
162+
// Replace newline inside double quotes
163+
result += placeholder
164+
i++
165+
} else {
166+
result += quoteChar
167+
i++
168+
}
169+
}
170+
} else if (char === "'") {
171+
// Start of single-quoted string
172+
result += char
173+
i++
174+
175+
// Process until we find the closing quote
176+
// Note: In single quotes, backslash does NOT escape (except for \' in some shells)
177+
while (i < command.length) {
178+
const quoteChar = command[i]
179+
180+
if (quoteChar === "'") {
181+
// Found closing quote
182+
result += quoteChar
183+
i++
184+
break
185+
} else if (quoteChar === "\n" || quoteChar === "\r") {
186+
// Replace newline inside single quotes
187+
result += placeholder
188+
i++
189+
} else {
190+
result += quoteChar
191+
i++
192+
}
193+
}
194+
} else {
195+
// Not in quotes, keep character as-is
196+
result += char
197+
i++
198+
}
199+
}
200+
201+
return result
202+
}
203+
125204
/**
126205
* Split a command string into individual sub-commands by
127206
* chaining operators (&&, ||, ;, |, or &) and newlines.
@@ -139,33 +218,7 @@ export function parseCommand(command: string): string[] {
139218
// First, protect newlines inside quoted strings by replacing them with a placeholder
140219
// This prevents splitting multi-line quoted strings (e.g., git commit -m "multi\nline")
141220
const quotedStringPlaceholder = "___NEWLINE_IN_QUOTE___"
142-
let protectedCommand = command
143-
144-
// Track quote state and replace newlines inside quotes
145-
let inDoubleQuote = false
146-
let inSingleQuote = false
147-
let result = ""
148-
149-
for (let i = 0; i < command.length; i++) {
150-
const char = command[i]
151-
const prevChar = i > 0 ? command[i - 1] : ""
152-
153-
// Toggle quote state (ignore escaped quotes)
154-
if (char === '"' && prevChar !== "\\") {
155-
inDoubleQuote = !inDoubleQuote
156-
result += char
157-
} else if (char === "'" && prevChar !== "\\") {
158-
inSingleQuote = !inSingleQuote
159-
result += char
160-
} else if ((char === "\n" || char === "\r") && (inDoubleQuote || inSingleQuote)) {
161-
// Replace newlines inside quotes with placeholder
162-
result += quotedStringPlaceholder
163-
} else {
164-
result += char
165-
}
166-
}
167-
168-
protectedCommand = result
221+
const protectedCommand = protectNewlinesInQuotes(command, quotedStringPlaceholder)
169222

170223
// Split by newlines (handle different line ending formats)
171224
// This regex splits on \r\n (Windows), \n (Unix), or \r (old Mac)

0 commit comments

Comments
 (0)