Skip to content

Commit 8a6bfef

Browse files
inimagainimaga
committed
Fix streaming markdown sanitization edge cases
1 parent 2f1ae41 commit 8a6bfef

2 files changed

Lines changed: 118 additions & 43 deletions

File tree

‎src/pages/inbox/conciergeDraftState.ts‎

Lines changed: 103 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -20,36 +20,93 @@ type BuildConciergeDraftReportActionParams = {
2020
reportID: string;
2121
};
2222

23-
/**
24-
* Count non-overlapping occurrences of `needle` in `haystack`.
25-
*/
26-
function countOccurrences(haystack: string, needle: string): number {
27-
let count = 0;
28-
let pos = 0;
29-
while (true) {
30-
const idx = haystack.indexOf(needle, pos);
31-
if (idx === -1) {
23+
type MarkdownRange = {
24+
start: number;
25+
end: number;
26+
};
27+
28+
const CODE_BLOCK_DELIMITER = '```';
29+
const INLINE_CODE_DELIMITER = '`';
30+
31+
function isEscaped(text: string, index: number): boolean {
32+
let slashCount = 0;
33+
let pos = index - 1;
34+
35+
while (pos >= 0 && text[pos] === '\\') {
36+
slashCount++;
37+
pos--;
38+
}
39+
40+
return slashCount % 2 !== 0;
41+
}
42+
43+
function getCodeRanges(text: string): {ranges: MarkdownRange[]; unclosedCodeBlockStart: number | null} {
44+
const ranges: MarkdownRange[] = [];
45+
let unclosedCodeBlockStart: number | null = null;
46+
47+
for (let pos = 0; pos <= text.length - CODE_BLOCK_DELIMITER.length; pos++) {
48+
if (!text.startsWith(CODE_BLOCK_DELIMITER, pos) || isEscaped(text, pos)) {
49+
continue;
50+
}
51+
52+
if (unclosedCodeBlockStart === null) {
53+
unclosedCodeBlockStart = pos;
54+
} else {
55+
ranges.push({start: unclosedCodeBlockStart, end: pos + CODE_BLOCK_DELIMITER.length});
56+
unclosedCodeBlockStart = null;
57+
}
58+
pos += CODE_BLOCK_DELIMITER.length - 1;
59+
}
60+
61+
let lineStart = 0;
62+
63+
while (lineStart <= text.length) {
64+
const nextNewline = text.indexOf('\n', lineStart);
65+
const lineEnd = nextNewline === -1 ? text.length : nextNewline;
66+
let openingDelimiterIndex: number | null = null;
67+
68+
for (let pos = lineStart; pos < lineEnd; pos++) {
69+
const isInCodeRange = ranges.some((range) => pos >= range.start && pos < range.end);
70+
if (text[pos] !== INLINE_CODE_DELIMITER || isEscaped(text, pos) || isInCodeRange) {
71+
continue;
72+
}
73+
74+
if (openingDelimiterIndex === null) {
75+
openingDelimiterIndex = pos;
76+
} else {
77+
ranges.push({start: openingDelimiterIndex, end: pos + INLINE_CODE_DELIMITER.length});
78+
openingDelimiterIndex = null;
79+
}
80+
}
81+
82+
if (nextNewline === -1) {
3283
break;
3384
}
34-
count++;
35-
pos = idx + needle.length;
85+
lineStart = nextNewline + 1;
3686
}
37-
return count;
87+
88+
return {ranges, unclosedCodeBlockStart};
3889
}
3990

40-
/**
41-
* If the last line of `text` contains an odd number of `delimiter` occurrences,
42-
* the final one opened a construct that was never closed. Strip from that
43-
* opening delimiter to the end of the string.
44-
*/
45-
function stripUnpairedLastLineDelimiter(text: string, delimiter: string): string {
91+
function stripUnpairedLastLineDelimiter(text: string, delimiter: string, ignoredRanges: MarkdownRange[] = []): string {
4692
const lastNewline = text.lastIndexOf('\n');
47-
const lastLine = text.substring(lastNewline + 1);
48-
const count = countOccurrences(lastLine, delimiter);
93+
const lastLineStart = lastNewline + 1;
94+
const delimiterIndexes: number[] = [];
95+
96+
for (let pos = lastLineStart; pos <= text.length - delimiter.length; pos++) {
97+
const isInIgnoredRange = ignoredRanges.some((range) => pos >= range.start && pos < range.end);
98+
if (!text.startsWith(delimiter, pos) || isEscaped(text, pos) || isInIgnoredRange) {
99+
continue;
100+
}
101+
102+
delimiterIndexes.push(pos);
103+
pos += delimiter.length - 1;
104+
}
49105

50-
if (count > 0 && count % 2 !== 0) {
51-
return text.substring(0, text.lastIndexOf(delimiter));
106+
if (delimiterIndexes.length > 0 && delimiterIndexes.length % 2 !== 0) {
107+
return text.substring(0, delimiterIndexes.at(-1));
52108
}
109+
53110
return text;
54111
}
55112

@@ -63,33 +120,36 @@ function stripIncompleteMarkdown(markdown: string): string {
63120
return markdown;
64121
}
65122

66-
let result = markdown;
123+
const initialCodeState = getCodeRanges(markdown);
124+
let codeRanges = initialCodeState.ranges;
125+
let result = initialCodeState.unclosedCodeBlockStart === null ? markdown : markdown.substring(0, initialCodeState.unclosedCodeBlockStart);
67126

68-
// 1. Incomplete link/image: find the last '[' and check whether a
69-
// complete [text](url) follows it. If not, strip from '[' (or '![').
70-
const lastOpenBracket = result.lastIndexOf('[');
71-
if (lastOpenBracket !== -1) {
72-
const tail = result.substring(lastOpenBracket);
73-
if (!/^\[[^\]]*\]\([^)]*\)/.test(tail)) {
74-
const stripFrom = lastOpenBracket > 0 && result[lastOpenBracket - 1] === '!' ? lastOpenBracket - 1 : lastOpenBracket;
75-
result = result.substring(0, stripFrom);
76-
}
77-
}
127+
// Strip incomplete inline code before looking for other markdown so code
128+
// contents don't look like unfinished links or emphasis.
129+
codeRanges = codeRanges.filter((range) => range.end <= result.length);
130+
result = stripUnpairedLastLineDelimiter(result, INLINE_CODE_DELIMITER, codeRanges);
78131

79-
// 2. Unclosed bold (**) on the last line.
80-
result = stripUnpairedLastLineDelimiter(result, '**');
132+
codeRanges = getCodeRanges(result).ranges;
133+
for (let openBracketIndex = result.length - 1; openBracketIndex >= 0; openBracketIndex--) {
134+
const isInCodeRange = codeRanges.some((range) => openBracketIndex >= range.start && openBracketIndex < range.end);
135+
if (result[openBracketIndex] !== '[' || isEscaped(result, openBracketIndex) || isInCodeRange) {
136+
continue;
137+
}
81138

82-
// 3. Unclosed strikethrough (~~) on the last line.
83-
result = stripUnpairedLastLineDelimiter(result, '~~');
139+
const closeBracketIndex = result.indexOf(']', openBracketIndex + 1);
140+
const stripFrom = openBracketIndex > 0 && result[openBracketIndex - 1] === '!' && !isEscaped(result, openBracketIndex - 1) ? openBracketIndex - 1 : openBracketIndex;
84141

85-
// 4. Unclosed code block (``` spans multiple lines).
86-
const codeBlockCount = countOccurrences(result, '```');
87-
if (codeBlockCount % 2 !== 0) {
88-
result = result.substring(0, result.lastIndexOf('```'));
142+
if (closeBracketIndex === -1) {
143+
result = result.substring(0, stripFrom);
144+
} else if (result[closeBracketIndex + 1] === '(' && result.indexOf(')', closeBracketIndex + 2) === -1) {
145+
result = result.substring(0, stripFrom);
146+
}
147+
break;
89148
}
90149

91-
// 5. Unclosed inline code (`) on the last line (after code-block handling).
92-
result = stripUnpairedLastLineDelimiter(result, '`');
150+
codeRanges = getCodeRanges(result).ranges;
151+
result = stripUnpairedLastLineDelimiter(result, '**', codeRanges);
152+
result = stripUnpairedLastLineDelimiter(result, '~~', codeRanges);
93153

94154
return result;
95155
}

‎tests/unit/pages/inbox/conciergeDraftState.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,11 @@ describe('conciergeDraftState', () => {
163163
expect(stripIncompleteMarkdown('[done](https://a.com) and [broken')).toBe('[done](https://a.com) and ');
164164
});
165165

166+
it('preserves bracketed text that is not a link', () => {
167+
const complete = 'The accepted values are [yes/no] for this setting';
168+
expect(stripIncompleteMarkdown(complete)).toBe(complete);
169+
});
170+
166171
it('strips an incomplete image syntax', () => {
167172
expect(stripIncompleteMarkdown('Here is ![alt')).toBe('Here is ');
168173
});
@@ -195,6 +200,11 @@ describe('conciergeDraftState', () => {
195200
expect(stripIncompleteMarkdown(complete)).toBe(complete);
196201
});
197202

203+
it('preserves a complete code block ending at the closing fence', () => {
204+
const complete = 'Before\n```\ncode\n```';
205+
expect(stripIncompleteMarkdown(complete)).toBe(complete);
206+
});
207+
198208
// --- Inline code (`) ---
199209
it('strips trailing unclosed inline code', () => {
200210
expect(stripIncompleteMarkdown('Run `command')).toBe('Run ');
@@ -205,6 +215,11 @@ describe('conciergeDraftState', () => {
205215
expect(stripIncompleteMarkdown(complete)).toBe(complete);
206216
});
207217

218+
it('preserves markdown-looking text inside complete inline code', () => {
219+
const complete = 'Use `[accountID]` and `**not bold` in the payload';
220+
expect(stripIncompleteMarkdown(complete)).toBe(complete);
221+
});
222+
208223
// --- Streaming integration ---
209224
it('strips incomplete markdown during a streaming draft event', () => {
210225
const draft = applyConciergeDraftEvent(null, createDraftEvent({bodyMarkdown: 'Check [this link'}), REPORT_ID);

0 commit comments

Comments
 (0)