fix(memory): guard against char-boundary panics in ingest persist path (#2102)

This commit is contained in:
Mega Mind
2026-05-19 16:29:46 -07:00
committed by GitHub
parent 4d28b1bc00
commit d13c426405
3 changed files with 148 additions and 2 deletions
@@ -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<String> {
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());
}
}
@@ -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..]))
}
+118 -2
View File
@@ -166,7 +166,21 @@ async fn persist(
// worth scanning, so they get body_preview = None.
let body_preview: Option<String> = 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);
}
}