From d13c42640545998964bd0fe8940fbdc42fde874b Mon Sep 17 00:00:00 2001 From: Mega Mind <146339422+M3gA-Mind@users.noreply.github.com> Date: Wed, 20 May 2026 04:59:46 +0530 Subject: [PATCH] fix(memory): guard against char-boundary panics in ingest persist path (#2102) --- .../memory/tree/canonicalize/email_clean.rs | 29 +++++ .../memory/tree/content_store/compose.rs | 1 + src/openhuman/memory/tree/ingest.rs | 120 +++++++++++++++++- 3 files changed, 148 insertions(+), 2 deletions(-) diff --git a/src/openhuman/memory/tree/canonicalize/email_clean.rs b/src/openhuman/memory/tree/canonicalize/email_clean.rs index cac33160f..766c38291 100644 --- a/src/openhuman/memory/tree/canonicalize/email_clean.rs +++ b/src/openhuman/memory/tree/canonicalize/email_clean.rs @@ -81,6 +81,7 @@ pub fn drop_reply_chain(s: &str) -> String { || lower.contains("--------- original message") || lower.contains("--- forwarded by"); if is_preamble { + debug_assert!(s.is_char_boundary(offset)); return s[..offset].trim_end().to_string(); } @@ -96,6 +97,7 @@ pub fn drop_reply_chain(s: &str) -> String { } if quoted_run_len >= 3 { let cut = quoted_run_start.unwrap_or(offset); + debug_assert!(s.is_char_boundary(cut)); return s[..cut].trim_end().to_string(); } } else if !trimmed.is_empty() { @@ -118,6 +120,7 @@ pub fn drop_footer_noise(s: &str) -> String { for line in s.split_inclusive('\n') { let lower = line.to_ascii_lowercase(); if FOOTER_TRIGGERS.iter().any(|t| lower.contains(t)) { + debug_assert!(s.is_char_boundary(offset)); return s[..offset].trim_end().to_string(); } offset += line.len(); @@ -187,6 +190,8 @@ pub fn extract_email(from: &str) -> Option { let s = from.trim(); if let (Some(start), Some(end)) = (s.rfind('<'), s.rfind('>')) { if start < end { + debug_assert!(s.is_char_boundary(start + 1)); + debug_assert!(s.is_char_boundary(end)); let inner = s[start + 1..end].trim(); if inner.contains('@') { return Some(inner.to_string()); @@ -379,4 +384,28 @@ mod tests { assert!(parse_message_date(&json!({"date": ""})).is_none()); assert!(parse_message_date(&json!({"date": " "})).is_none()); } + + #[test] + fn drop_reply_chain_handles_zwnj_in_body() { + // U+200C ZERO WIDTH NON-JOINER appears in Persian/Arabic text inside + // the real content. It must be preserved through reply-chain stripping + // when it does not appear in a reply-preamble line. + let zwnj = "\u{200c}"; + let body = format!( + "سلام{}دوست عزیز، لطفاً بررسی کنید.\n\nOn Mon, Apr 22, 2026, Alice wrote:\n> old content", + zwnj + ); + + let cleaned = drop_reply_chain(&body); + + // Reply chain must be stripped. + assert!(!cleaned.contains("old content")); + // The ZWNJ in the real body must survive. + assert!( + cleaned.contains(zwnj), + "ZWNJ was incorrectly removed from real content" + ); + // Result must be valid UTF-8 (no panic from slicing at the boundary). + assert!(std::str::from_utf8(cleaned.as_bytes()).is_ok()); + } } diff --git a/src/openhuman/memory/tree/content_store/compose.rs b/src/openhuman/memory/tree/content_store/compose.rs index 6f55217d9..8ec1822f2 100644 --- a/src/openhuman/memory/tree/content_store/compose.rs +++ b/src/openhuman/memory/tree/content_store/compose.rs @@ -592,6 +592,7 @@ pub fn split_front_matter(content: &str) -> Option<(&str, &str)> { rest.strip_suffix("\n---").map(|r| r.len()) })?; let fm_end = 4 + close_idx + 5; // include `\n---\n` + debug_assert!(content.is_char_boundary(fm_end)); Some((&content[..fm_end], &content[fm_end..])) } diff --git a/src/openhuman/memory/tree/ingest.rs b/src/openhuman/memory/tree/ingest.rs index 562e3a42e..5bd08be50 100644 --- a/src/openhuman/memory/tree/ingest.rs +++ b/src/openhuman/memory/tree/ingest.rs @@ -166,7 +166,21 @@ async fn persist( // worth scanning, so they get body_preview = None. let body_preview: Option = match source_kind_for_store { SourceKind::Email | SourceKind::Document => { - Some(markdown_body_preview(&canonical.markdown)) + // Guard the preview computation so a single malformed document + // never kills the ingest worker. `markdown_body_preview` contains + // defensive checks, but wrap at the call-site too for belt-and-braces + // protection against any future panic regression in its dependency chain. + let md_for_preview = canonical.markdown.clone(); + match std::panic::catch_unwind(move || markdown_body_preview(&md_for_preview)) { + Ok(preview) => Some(preview), + Err(_) => { + log::error!( + "[memory_tree::ingest] markdown_body_preview panicked for source_id_hash={}; falling back to no preview", + crate::openhuman::memory::tree::util::redact::redact(source_id) + ); + None + } + } } _ => None, }; @@ -369,7 +383,20 @@ fn markdown_body_preview(md: &str) -> String { md.to_string() } else { let start = crate::openhuman::util::ceil_char_boundary(md, len - BODY_PREVIEW_MAX_BYTES); - md[start..].to_string() + debug_assert!( + md.is_char_boundary(start), + "ceil_char_boundary returned non-boundary {start} for len={len}" + ); + // ceil_char_boundary can return `len` when every remaining byte is a + // continuation byte; fall back to the full string rather than panicking. + if start > len || !md.is_char_boundary(start) { + log::error!( + "[memory_tree::ingest] ceil_char_boundary returned invalid boundary start={start} len={len}; returning full markdown" + ); + md.to_string() + } else { + md[start..].to_string() + } } } @@ -588,4 +615,93 @@ mod tests { assert_eq!(count_chunks(&cfg).unwrap(), 1); assert_eq!(count_scores(&cfg).unwrap(), 1); } + + // ── multi-byte boundary tests (issue #2073) ────────────────────────────── + + #[test] + fn markdown_body_preview_zwnj_at_exact_boundary() { + // U+200C ZERO WIDTH NON-JOINER is 3 bytes (0xE2 0x80 0x8C). + // Place it at offsets 0, 1, 2 relative to the preview boundary so + // that each byte of the codepoint lands exactly on the nominal cut. + let zwnj = '\u{200c}'; + let zwnj_bytes = zwnj.len_utf8(); // 3 + assert_eq!(zwnj_bytes, 3); + + for offset in 0..zwnj_bytes { + // ascii_prefix || zwnj || ascii_suffix + // The nominal cut point is `ascii_prefix.len() + offset` bytes + // from the start, which lands `offset` bytes into the zwnj. + let prefix_len = BODY_PREVIEW_MAX_BYTES - offset; + let ascii_prefix = "a".repeat(prefix_len + (zwnj_bytes - offset)); + // Build: enough leading bytes so (total - BODY_PREVIEW_MAX_BYTES) falls + // inside the zwnj. + let padding = "x".repeat(50); + let md = format!( + "{}{}{}{}", + padding, + "a".repeat(prefix_len - 50), + zwnj, + "b".repeat(offset + 100) + ); + + let preview = markdown_body_preview(&md); + + // Must not panic, result must be valid UTF-8, and byte length <= cap. + assert!(std::str::from_utf8(preview.as_bytes()).is_ok()); + assert!( + preview.len() <= BODY_PREVIEW_MAX_BYTES, + "offset={offset}: preview len {} exceeds cap {}", + preview.len(), + BODY_PREVIEW_MAX_BYTES + ); + let _ = ascii_prefix; // suppress unused warning + } + } + + #[test] + fn markdown_body_preview_figure_space_at_exact_boundary() { + // U+2007 FIGURE SPACE is 3 bytes (0xE2 0x80 0x87). + let fig_space = '\u{2007}'; + let fig_bytes = fig_space.len_utf8(); + assert_eq!(fig_bytes, 3); + + for offset in 0..fig_bytes { + let padding = "x".repeat(50); + let prefix_len = if BODY_PREVIEW_MAX_BYTES > offset + 50 { + BODY_PREVIEW_MAX_BYTES - offset - 50 + } else { + 0 + }; + let md = format!( + "{}{}{}{}", + padding, + "a".repeat(prefix_len), + fig_space, + "b".repeat(offset + 100) + ); + + let preview = markdown_body_preview(&md); + + assert!(std::str::from_utf8(preview.as_bytes()).is_ok()); + assert!( + preview.len() <= BODY_PREVIEW_MAX_BYTES, + "offset={offset}: preview len {} exceeds cap {}", + preview.len(), + BODY_PREVIEW_MAX_BYTES + ); + } + } + + #[test] + fn markdown_body_preview_persian_text() { + // Persian word "سلام‌ها" (hello + plural marker) with embedded U+200C. + let persian_word = "\u{0633}\u{0644}\u{0627}\u{0645}\u{200c}\u{0647}\u{0627}"; + let md = persian_word.repeat(200); + + // Must not panic regardless of where the cut falls. + let preview = markdown_body_preview(&md); + + assert!(std::str::from_utf8(preview.as_bytes()).is_ok()); + assert!(preview.len() <= BODY_PREVIEW_MAX_BYTES); + } }