Skip to content

Commit 352a88b

Browse files
committed
fix(website): avoid exponential pandoc parse on sidebar titles (#14576)
The navigation envelope concentrated every injected inline title into a single hidden markdown paragraph. Pandoc's markdown reader backtracks exponentially over unresolved emphasis candidates inside consecutive bracketed inlines (jgm/pandoc#11687), so a site whose page titles merely contain double-underscore names (e.g. `Class.__method__()`) hung on every page render: ~20 such titles pushed parse time past 180s, with nothing user-visible to debug. Joining the spans with a blank line puts each title in its own paragraph, so pandoc parses each independently and parse time stays linear. This keeps markdown processing intact (shortcodes, icons, bold/code in titles still render), unlike escaping or raw-wrapping the injected content.
1 parent 5807604 commit 352a88b

5 files changed

Lines changed: 168 additions & 2 deletions

File tree

news/changelog-1.10.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ All changes included in 1.10:
102102
- ([#14461](https://github.com/quarto-dev/quarto-cli/issues/14461)): Fix `quarto render --to pdf` aborting with `ERROR: Problem running 'fmtutil-sys --all' to rebuild format tree.` when an automatically-installed LaTeX package's post-update format rebuild fails. Format-tree rebuild is now treated as best-effort housekeeping (matching upstream `tinytex` R behavior) — the failure is logged as a warning and the package install completes.
103103
- ([#14472](https://github.com/quarto-dev/quarto-cli/issues/14472)): Add support for Kotlin in code annotations and YAML cell options. (author: @barendgehrels)
104104
- ([#14529](https://github.com/quarto-dev/quarto-cli/issues/14529)): Fix bundled Julia engine path leaking into rendered YAML metadata and pandoc log output when running an installed Quarto. The internal subtree-engine filter only matched the source-tree share-path layout (`resources/extension-subtrees/`) and missed installed layouts where the path is `share/extension-subtrees/`.
105+
- ([#14576](https://github.com/quarto-dev/quarto-cli/issues/14576)): Fix website render hanging when many sidebar titles contain double-underscore (dunder) names such as `Transcript.__getitem__()`.
105106
- ([#14582](https://github.com/quarto-dev/quarto-cli/issues/14582)): Fix format detection for extension formats (e.g. `acm-pdf`) in project preview, manuscript notebooks, MECA bundles, and website format ordering.
106107
- ([#14583](https://github.com/quarto-dev/quarto-cli/issues/14583)): Fix a shortcode used as an image source (e.g. `![]({{< meta logo >}})`) getting the `default-image-extension` appended, producing a doubled extension once the shortcode resolves.
107108
- ([#14595](https://github.com/quarto-dev/quarto-cli/issues/14595)): Fix reload preview in code-server environment

src/core/markdown-pipeline.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ const markdownEnvelopeWriter = (envelopeId: string) => {
116116
renderList.push(hiddenSpan(id, value));
117117
},
118118
toMarkdown: () => {
119-
const contents = renderList.join("\n");
119+
const contents = renderList.join("\n\n");
120120
return `\n:::{#${envelopeId} .hidden}\n${contents}\n:::\n`;
121121
},
122122
};
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/*
2+
* render-sidebar-markdown-titles.test.ts
3+
*
4+
* Copyright (C) 2020-2024 Posit Software, PBC
5+
*
6+
*/
7+
import { join } from "../../../src/deno_ral/path.ts";
8+
import { ensureDirSync, safeRemoveSync } from "../../../src/deno_ral/fs.ts";
9+
import { testQuartoCmd } from "../../test.ts";
10+
import { ensureFileRegexMatches, noErrors } from "../../verify.ts";
11+
12+
// Regression test for GH #14576 / upstream jgm/pandoc#11687.
13+
//
14+
// Sidebar titles are rendered through the navigation "envelope": each title
15+
// becomes an inline span, the spans are joined into a single markdown document,
16+
// pandoc renders it, and the results are read back. When many sidebar titles
17+
// wrap text around a dunder name (e.g. `Transcript.__getitem__()`), joining the
18+
// spans into one paragraph made pandoc's markdown reader backtrack
19+
// exponentially over the unresolved `_`-emphasis candidates, hanging the
20+
// render. The fix joins each span as its own paragraph so the parse stays
21+
// linear. Pre-fix this render hangs (the test fails by timing out); post-fix it
22+
// completes quickly.
23+
//
24+
// The project is generated on the fly rather than stored as fixtures: the many
25+
// dunder pages would otherwise bloat the repo and each be picked up as its own
26+
// smoke-all render target.
27+
28+
const dunderTitles = [
29+
"__init__", "__call__", "__repr__", "__str__", "__len__",
30+
"__getitem__", "__setitem__", "__delitem__", "__iter__", "__next__",
31+
"__enter__", "__exit__", "__add__", "__sub__", "__mul__",
32+
"__eq__", "__hash__", "__new__", "__del__", "__contains__",
33+
];
34+
35+
const pageName = (i: number) => `page${String(i + 1).padStart(2, "0")}.qmd`;
36+
37+
const projectDir = Deno.makeTempDirSync({ prefix: "quarto-sidebar-titles" });
38+
const outputDir = join(projectDir, "_site");
39+
40+
const writeProject = () => {
41+
ensureDirSync(projectDir);
42+
43+
const sidebarContents = [
44+
"index.qmd",
45+
...dunderTitles.map((_, i) => pageName(i)),
46+
"bold-page.qmd",
47+
"code-page.qmd",
48+
].map((p) => ` - ${p}`).join("\n");
49+
50+
Deno.writeTextFileSync(
51+
join(projectDir, "_quarto.yml"),
52+
`project:
53+
type: website
54+
website:
55+
title: "Sidebar Titles"
56+
sidebar:
57+
contents:
58+
${sidebarContents}
59+
format:
60+
html:
61+
theme: cosmo
62+
`,
63+
);
64+
65+
Deno.writeTextFileSync(
66+
join(projectDir, "index.qmd"),
67+
`---\ntitle: "Home"\n---\n\nSidebar markdown-title regression guard for #14576.\n`,
68+
);
69+
70+
dunderTitles.forEach((dunder, i) => {
71+
Deno.writeTextFileSync(
72+
join(projectDir, pageName(i)),
73+
`---\ntitle: "Transcript.${dunder}()"\n---\n\nPage for \`Transcript.${dunder}()\`.\n`,
74+
);
75+
});
76+
77+
Deno.writeTextFileSync(
78+
join(projectDir, "bold-page.qmd"),
79+
`---\ntitle: "A **bold** word"\n---\n\nBold title page.\n`,
80+
);
81+
Deno.writeTextFileSync(
82+
join(projectDir, "code-page.qmd"),
83+
`---\ntitle: "A \`code\` word"\n---\n\nCode title page.\n`,
84+
);
85+
};
86+
87+
testQuartoCmd(
88+
"render",
89+
[projectDir],
90+
[
91+
noErrors,
92+
ensureFileRegexMatches(
93+
join(outputDir, "index.html"),
94+
[
95+
// Dunder titles render literally (intraword_underscores keeps `__x__`
96+
// out of emphasis), and reaching this assertion at all proves the
97+
// render did not hang.
98+
/Transcript\.__getitem__\(\)/,
99+
// Markdown inside titles still renders correctly through the envelope.
100+
/A <strong>bold<\/strong> word/,
101+
/A <code>code<\/code> word/,
102+
],
103+
[
104+
// A dunder name must not be turned into spurious emphasis.
105+
/<strong>getitem<\/strong>/,
106+
],
107+
),
108+
],
109+
{
110+
// Performance budget: the healthy render finishes in well under this.
111+
// Pre-fix the exponential parse blows far past it, so the test fails fast
112+
// (instead of riding the default 10-minute timeout) when the fix regresses.
113+
timeout: 120000,
114+
setup: async () => {
115+
writeProject();
116+
},
117+
teardown: async () => {
118+
safeRemoveSync(projectDir, { recursive: true });
119+
},
120+
},
121+
);

tests/test.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ export interface TestContext {
8585

8686
// environment to pass to downstream processes
8787
env?: Record<string, string>;
88+
89+
// Maximum time (ms) the quarto command may run before the test fails.
90+
// Defaults to 600000 (10 minutes). Lower it to assert a performance budget
91+
// (e.g. a render that must not regress into a hang).
92+
timeout?: number;
8893
}
8994

9095
// Allow to merge test contexts in Tests helpers
@@ -124,6 +129,8 @@ export function mergeTestContexts(baseContext: TestContext, additionalContext?:
124129
ignore: additionalContext.ignore ?? baseContext.ignore,
125130
// merge env with additional context taking precedence
126131
env: { ...baseContext.env, ...additionalContext.env },
132+
// override timeout if provided
133+
timeout: additionalContext.timeout ?? baseContext.timeout,
127134
};
128135
}
129136

@@ -141,8 +148,13 @@ export function testQuartoCmd(
141148
test({
142149
name,
143150
execute: async () => {
151+
const timeoutMs = context?.timeout ?? 600000;
144152
const timeout = new Promise((_resolve, reject) => {
145-
setTimeout(reject, 600000, "timed out after 10 minutes");
153+
setTimeout(
154+
reject,
155+
timeoutMs,
156+
`timed out after ${timeoutMs}ms`,
157+
);
146158
});
147159
await Promise.race([
148160
quarto([cmd, ...args], undefined, context?.env),
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* markdown-pipeline.test.ts
3+
*
4+
* Copyright (C) 2020-2022 Posit Software, PBC
5+
*
6+
*/
7+
import { unitTest } from "../test.ts";
8+
import { assertStringIncludes } from "testing/asserts";
9+
import { createMarkdownRenderEnvelope } from "../../src/core/markdown-pipeline.ts";
10+
11+
// Each inline span must be separated by a blank line (double newline) so that
12+
// pandoc's markdown reader parses each as its own paragraph. A single newline
13+
// joins all titles into one paragraph, triggering exponential emphasis-matcher
14+
// parse time on many `__x__`-style titles (GH #14576).
15+
// deno-lint-ignore require-await
16+
unitTest(
17+
"markdown-pipeline - inline spans separated by blank line (#14576)",
18+
async () => {
19+
const envelope = createMarkdownRenderEnvelope("test-envelope", {
20+
inlines: {
21+
first: "alpha",
22+
second: "beta",
23+
},
24+
});
25+
26+
assertStringIncludes(
27+
envelope,
28+
"render-id=\"Zmlyc3Q=\"}\n\n[beta]",
29+
"Inline spans must be separated by a blank line so pandoc parses each as its own paragraph",
30+
);
31+
},
32+
);

0 commit comments

Comments
 (0)