Skip to content

Commit a34dfca

Browse files
authored
fix(timestamp): escape live table-cell pipe after an escaped backslash (#227)
escapeForTableCell used a `(?<!\\)\|` lookbehind that treated any pipe preceded by a backslash as already escaped. The sequence `\\|` (an escaped backslash followed by a genuinely live pipe) slipped through unescaped, so the pipe stayed active and broke the markdown table row when a captured timestamp landed in a table cell. Adjacent pipes (`||`) had the same class of gap. Key the escape on the parity of the preceding backslash run instead: a pipe is already escaped only when it follows an odd-length run of backslashes, so an even-length run (including zero) is escaped exactly once. This neutralizes a pipe inside a table cell regardless of any preceding backslashes while leaving genuinely-escaped pipes untouched. Adds unit tests for the escaped-backslash-then-pipe (`a\\|b`), odd-run (`a\\\|b`), and adjacent-pipe (`a||b`) cases. Resolves deepsec finding other-escaping-gap-3d20dd1596.
1 parent a5683db commit a34dfca

2 files changed

Lines changed: 29 additions & 1 deletion

File tree

src/utility/prepareTimestampInsertion.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,24 @@ describe("escapeForTableCell", () => {
9191
expect(escapeForTableCell("a \\| b")).toBe("a \\| b");
9292
});
9393

94+
it("escapes a live pipe that follows an escaped backslash (\\\\|)", () => {
95+
// `\\|` is an escaped backslash followed by a genuinely live pipe. A
96+
// naive `(?<!\\)` lookbehind sees the backslash and skips the pipe,
97+
// leaving it active and breaking the row. The even-length (2) backslash
98+
// run means the pipe is live and must be escaped.
99+
expect(escapeForTableCell("a\\\\|b")).toBe("a\\\\\\|b");
100+
});
101+
102+
it("leaves a pipe escaped after an odd-length backslash run (\\\\\\|)", () => {
103+
// Three backslashes = an escaped backslash plus an already-escaped pipe;
104+
// the pipe must be left alone so the cell is never double-escaped.
105+
expect(escapeForTableCell("a\\\\\\|b")).toBe("a\\\\\\|b");
106+
});
107+
108+
it("escapes both of two adjacent live pipes", () => {
109+
expect(escapeForTableCell("a||b")).toBe("a\\|\\|b");
110+
});
111+
94112
it("collapses newlines (LF, CRLF, CR) to single spaces", () => {
95113
expect(escapeForTableCell("line1\nline2")).toBe("line1 line2");
96114
expect(escapeForTableCell("line1\r\nline2")).toBe("line1 line2");

src/utility/prepareTimestampInsertion.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,19 @@ export function isInsideTable(
8080
* space (a raw newline would end the row) and escape any unescaped pipe (a raw
8181
* pipe would open a new column). Already-escaped pipes (`\|`) are left alone so
8282
* the cell is never double-escaped.
83+
*
84+
* A pipe is "already escaped" only when it follows an odd-length run of
85+
* backslashes; an even-length run (including zero) leaves the pipe live. Keying
86+
* on the parity of the preceding backslash run - rather than the mere presence
87+
* of one backslash - neutralizes pipes after an escaped backslash (`\\|`) and
88+
* adjacent pipes (`||`), both of which a simple `(?<!\\)` lookbehind misses.
8389
*/
8490
export function escapeForTableCell(text: string): string {
85-
return text.replace(/\r\n?|\n/g, " ").replace(/(?<!\\)\|/g, "\\|");
91+
return text
92+
.replace(/\r\n?|\n/g, " ")
93+
.replace(/(\\*)\|/g, (match, backslashes: string) =>
94+
backslashes.length % 2 === 0 ? `${backslashes}\\|` : match,
95+
);
8696
}
8797

8898
/**

0 commit comments

Comments
 (0)