Skip to content

Commit 2bea62b

Browse files
Micro-perf polish: cached sanitizer, single-pass escape_html, clamp
Three small wins bundled because they're all one-file drive-bys: 1. render_markdown: cache ammonia::Builder and pulldown_cmark Options in a OnceLock so the config isn't re-constructed on every render. High-traffic dashboard / memo pages re-render constantly, and the Builder's allowed-tag and allowed-attribute sets add up. 2. escape_html: collapse the five chained .replace() passes (each of which allocates a fresh String) into a single scan that pushes the entity references into one pre-allocated buffer. Extract it to the shared markdown module so the duplicate copy in services::comparison can drop. 3. Replace .min(10.0).max(0.0) with .clamp(0.0, 10.0) in the score pipelines. Same semantics, slightly cheaper, and Clippy-approved. Added unit tests for the shared escape_html and render_markdown sanitization pass. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
1 parent 05f8a6c commit 2bea62b

4 files changed

Lines changed: 77 additions & 18 deletions

File tree

src/markdown.rs

Lines changed: 70 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,76 @@
1+
use ammonia::Builder;
12
use pulldown_cmark::{html, Options, Parser};
3+
use std::sync::OnceLock;
24

3-
pub fn render_markdown(markdown: &str) -> String {
4-
let mut options = Options::empty();
5-
options.insert(Options::ENABLE_STRIKETHROUGH);
6-
options.insert(Options::ENABLE_TABLES);
7-
options.insert(Options::ENABLE_TASKLISTS);
8-
options.insert(Options::ENABLE_FOOTNOTES);
5+
/// Pre-configured ammonia sanitizer + pulldown-cmark option set. Cached in
6+
/// a `OnceLock` so every render reuses the same configured `Builder` rather
7+
/// than rebuilding the allowed-tag and allowed-attribute sets on each call.
8+
static SANITIZER: OnceLock<Builder<'static>> = OnceLock::new();
9+
static MARKDOWN_OPTIONS: OnceLock<Options> = OnceLock::new();
10+
11+
fn sanitizer() -> &'static Builder<'static> {
12+
SANITIZER.get_or_init(Builder::default)
13+
}
914

10-
let parser = Parser::new_ext(markdown, options);
15+
fn markdown_options() -> Options {
16+
*MARKDOWN_OPTIONS.get_or_init(|| {
17+
let mut options = Options::empty();
18+
options.insert(Options::ENABLE_STRIKETHROUGH);
19+
options.insert(Options::ENABLE_TABLES);
20+
options.insert(Options::ENABLE_TASKLISTS);
21+
options.insert(Options::ENABLE_FOOTNOTES);
22+
options
23+
})
24+
}
25+
26+
pub fn render_markdown(markdown: &str) -> String {
27+
let parser = Parser::new_ext(markdown, markdown_options());
1128
let mut html_output = String::new();
1229
html::push_html(&mut html_output, parser);
13-
ammonia::clean(&html_output)
30+
sanitizer().clean(&html_output).to_string()
31+
}
32+
33+
/// Escape a string for safe insertion into HTML text / attribute context.
34+
/// Single-pass, allocates once with a best-effort capacity estimate. Mirrors
35+
/// the old five-chained-`.replace()` behaviour but avoids building four
36+
/// throw-away intermediate strings per call.
37+
pub fn escape_html(raw: &str) -> String {
38+
let mut out = String::with_capacity(raw.len());
39+
for c in raw.chars() {
40+
match c {
41+
'&' => out.push_str("&amp;"),
42+
'<' => out.push_str("&lt;"),
43+
'>' => out.push_str("&gt;"),
44+
'"' => out.push_str("&quot;"),
45+
'\'' => out.push_str("&#39;"),
46+
_ => out.push(c),
47+
}
48+
}
49+
out
50+
}
51+
52+
#[cfg(test)]
53+
mod tests {
54+
use super::*;
55+
56+
#[test]
57+
fn escape_html_replaces_specials() {
58+
assert_eq!(
59+
escape_html("<a href=\"x\">O'Reilly & Sons</a>"),
60+
"&lt;a href=&quot;x&quot;&gt;O&#39;Reilly &amp; Sons&lt;/a&gt;"
61+
);
62+
}
63+
64+
#[test]
65+
fn escape_html_passes_through_plain_text() {
66+
let s = "Hello, world!";
67+
assert_eq!(escape_html(s), s);
68+
}
69+
70+
#[test]
71+
fn render_markdown_sanitizes_script_tags() {
72+
let raw = "Hello <script>alert('x')</script> world";
73+
let out = render_markdown(raw);
74+
assert!(!out.contains("<script"), "rendered output: {out}");
75+
}
1476
}

src/services/comparison.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
use crate::{app_state::AppState, models::ComparisonRunWithDetails, status::RunStatus};
1+
use crate::{
2+
app_state::AppState, markdown::escape_html, models::ComparisonRunWithDetails,
3+
status::RunStatus,
4+
};
25
use anyhow::Result;
36

47
pub async fn sync_comparisons_for_run(state: &AppState, run_id: &str) -> Result<()> {
@@ -134,10 +137,4 @@ fn build_terminal_rollup(
134137
(status, summary, final_html)
135138
}
136139

137-
fn escape_html(raw: &str) -> String {
138-
raw.replace('&', "&amp;")
139-
.replace('<', "&lt;")
140-
.replace('>', "&gt;")
141-
.replace('"', "&quot;")
142-
.replace('\'', "&#39;")
143-
}
140+

src/services/opportunity_ranker.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ pub fn calculate_overall_score(
4343
+ (coverage_gap * 0.25)
4444
+ (timing * 0.20);
4545

46-
weighted_score.min(10.0).max(0.0)
46+
weighted_score.clamp(0.0, 10.0)
4747
}
4848

4949
/// Rank opportunities by overall score.

src/services/signal_detector.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ pub fn calculate_signal_strength(signals: &[ScanSignal]) -> f64 {
9696
.sum();
9797

9898
let avg_weight = total_weight / signals.len() as f64;
99-
(avg_weight * 10.0).min(10.0).max(0.0)
99+
(avg_weight * 10.0).clamp(0.0, 10.0)
100100
}
101101

102102
/// Calculate timing score based on signal freshness.

0 commit comments

Comments
 (0)