From 0dcef63a885a49913c9ac2bee71cb624b546fc45 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 21:38:01 -0500 Subject: [PATCH] craft-captions: reject signed SCC code words and MCC time code fields (ports of two FilmCraft PRs) (#1060) * craft-captions: reject signed or short SCC code words on import A code word must be exactly four ASCII hex digits. u16::from_str_radix accepted a leading '+', so "+942" was decoded as 0x0942 without the 'bad code word' warning. Ported from storytold/filmcraft#4429. * craft-captions: reject signed timecode fields in MCC import Each time code field must be non-empty ASCII digits before it is parsed. i64::parse accepted a leading '+' or '-', so 00:00:+0:+0 read as a valid frame position and a '-' field could move a cue before zero. Ported from storytold/filmcraft#4509. --------- Co-authored-by: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> --- crates/captions/src/mcc.rs | 19 ++++++++++++++++++- crates/captions/src/scc.rs | 14 +++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/crates/captions/src/mcc.rs b/crates/captions/src/mcc.rs index 23f5544c6..ba964c1d6 100644 --- a/crates/captions/src/mcc.rs +++ b/crates/captions/src/mcc.rs @@ -734,7 +734,8 @@ pub fn parse(text: &str) -> Result { continue; }; let tc = tc.split('.').next().unwrap_or(tc); - let parts: Option> = tc.split([':', ';']).map(|p| p.parse().ok()).collect(); + let parts: Option> = + tc.split([':', ';']).map(|p| if !p.is_empty() && p.bytes().all(|b| b.is_ascii_digit()) { p.parse().ok() } else { None }).collect(); let Some([h, m, s, f]) = parts.as_deref() else { return Err(Error::Syntax { line: n + 1, msg: format!("bad timecode \"{tc}\"") }); }; @@ -854,6 +855,22 @@ mod tests { } /// Reject malformed fields instead of silently dropping them from the timecode. + /// Reject timecode fields carrying a sign instead of only ASCII digits. + #[test] + fn rejects_signed_timecode_fields() { + let doc = Document { cues: vec![cue(0, 60, "Caption")], ..Default::default() }; + let text = write(&doc, false); + assert!(text.contains("00:00:00:00")); + assert!(parse(&text).is_ok()); + for bad in ["00:00:+0:+0", "00:00:-0:-0", "00:00:00:-1", "00:00:00:+1"] { + let signed = text.replacen("00:00:00:00", bad, 1); + match parse(&signed) { + Err(Error::Syntax { msg, .. }) => assert!(msg.contains("bad timecode"), "{msg}"), + other => panic!("signed timecode {bad} accepted: {other:?}"), + } + } + } + #[test] fn rejects_timecodes_with_unparseable_extra_fields() { let doc = Document { cues: vec![cue(60, 120, "Caption")], ..Default::default() }; diff --git a/crates/captions/src/scc.rs b/crates/captions/src/scc.rs index 08d023760..354bc88f2 100644 --- a/crates/captions/src/scc.rs +++ b/crates/captions/src/scc.rs @@ -295,7 +295,8 @@ pub fn parse(text: &str) -> Result { // the first time code says how the file counts (drop-frame or not) doc.labels.get_or_insert((RATE, tc.contains([';', '.', ',']))); for (k, w) in it.enumerate() { - let Ok(v) = u16::from_str_radix(w, 16) else { + let v = if w.len() == 4 && w.bytes().all(|b| b.is_ascii_hexdigit()) { u16::from_str_radix(w, 16).ok() } else { None }; + let Some(v) = v else { doc.warnings.push(format!("line {}: bad code word \"{w}\"", n + 1)); continue; }; @@ -538,6 +539,17 @@ mod tests { Cue { start: RATE.tick_of(s), end: RATE.tick_of(e), text: text.into(), ..Default::default() } } + #[test] + fn plus_signed_code_word_is_rejected() { + for bad in ["+942", "-942", "+94", "94G2"] { + let doc = parse(&format!("Scenarist_SCC V1.0\n\n00:00:01:00\t{bad} 9420\n")).unwrap(); + assert_eq!(doc.warnings.len(), 1, "{bad}: {:?}", doc.warnings); + assert!(doc.warnings[0].contains("bad code word")); + } + let doc = parse("Scenarist_SCC V1.0\n\n00:00:01:00\t9420 9420\n").unwrap(); + assert!(doc.warnings.is_empty()); + } + #[test] fn timecode_rejects_signed_fields() { assert_eq!(parse_timecode("00:00:-5:00"), None);