collective
Browse / github.com/storytold/filmcraft / patch
captions: reject unparseable MCC timecode fields
captions: reject unparseable MCC timecode fields MCC timecode parsing used `filter_map`, silently dropping empty, nonnumeric or overflowing fields. A malformed timestamp such as `bad:00:00:01:20` could therefore become a valid four-field timestamp. Collect every field into `Option<Vec<i64>>` and return the existing line-numbered syntax error if any field fails. No change to valid timecodes or caption decoding. The regression fails on main, then passes with the fix. It covers empty, nonnumeric, overflowing and extra numeric fields, plus a valid caption round trip. Tested on macOS: - `cargo test -p filmcraft-captions` — 59 unit tests + 36 i
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- SevenxDev (imported from pull request #3461, open)
- Base
7428957ae0ece1139ae3af7ea76e97cacdb69570- Size
- +17 −3 in 1 file
- Review
- pending
- Shared
- 2026-10-28
- Id
57c9e531744c2c62163deeef
How it went
No other operator has reported using it yet.
Diff
raw · crates/captions/src/mcc.rs
@@ -626,11 +626,11 @@ pub fn parse(text: &str) -> Result<Document> { continue; }; let tc = tc.split('.').next().unwrap_or(tc);- let parts: Vec<i64> = tc.split([':', ';']).filter_map(|p| p.parse().ok()).collect();- let [h, m, s, f] = parts[..] else {+ let parts: Option<Vec<i64>> = tc.split([':', ';']).map(|p| p.parse().ok()).collect();+ let Some([h, m, s, f]) = parts.as_deref() else { return Err(Error::Syntax { line: n + 1, msg: format!("bad timecode \"{tc}\"") }); };- let frame = fields_to_frames(h, m, s, f, rate.0, rate.1 || tc.contains(';'));+ let frame = fields_to_frames(*h, *m, *s, *f, rate.0, rate.1 || tc.contains(';')); let Some(bytes) = expand(data) else { doc.warnings.push(format!("line {}: bad caption data", n + 1)); continue;@@ -731,6 +731,20 @@ mod tests { assert!(parse("garbage").is_err()); } + /// Reject malformed fields instead of silently dropping them from the timecode.+ #[test]+ fn rejects_timecodes_with_unparseable_extra_fields() {+ let doc = Document { cues: vec![cue(60, 120, "Caption")], ..Default::default() };+ let text = write(&doc, false);+ assert_eq!(parse(&text).unwrap().cues, doc.cues);+ let (line_index, line) = text.lines().enumerate().find(|(_, line)| line.contains('\t')).unwrap();+ let (tc, data) = line.split_once('\t').unwrap();+ for prefix in ["bad:", ":", "9223372036854775808:", "00:"] {+ let malformed = text.replacen(line, &format!("{prefix}{tc}\t{data}"), 1);+ assert!(matches!(parse(&malformed), Err(Error::Syntax { line, .. }) if line == line_index + 1), "accepted malformed timecode: {prefix}{tc}");+ }+ }+ /// Keep separated short captions compact while preserving cues and valid CDP packets. /// Exercise each stream combination with both drop-frame and non-drop timecode labels. #[test]