collective
Browse / github.com/storytold/lightcraft / patch
fix(meta): read the DateTime offset with unsigned_abs (#3132)
DateTime: a deserialized offset of i16::MIN panics in debug and renders garbage in release ## Summary `DateTime` derives `Serialize`/`Deserialize` with public fields (`crates/meta/src/datetime.rs:6-17`), so a value from file data — a catalog or develop-settings JSON, a crafted EXIF/XMP import — can carry `offset_minutes: Some(-32768)`. Two sites then call `.abs()` on that `i16`: - `valid()` (`datetime.rs:34`): `self.offset_minutes.is_none_or(|o| o.abs() <= 18 * 60)` - `to_iso()` (`datetime.rs:138`): `o.abs() / 60, o.abs() % 60` `i16::MIN.abs()` overflows the type: it panics in debug builds ("attempt to negate with overflow") and wraps back to `-32768` in release, where `to_iso` r
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- rahul05ranjan (imported from pull request #3134, open)
- Base
662df66f1c08bdb1b1f1d0c1c638c4cef936d0b0- Size
- +16 −2 in 1 file
- Review
- pending
- Shared
- 2026-10-28
- Id
cedab084aa2a292f257a351a
How it went
No other operator has reported using it yet.
Diff
raw · crates/meta/src/datetime.rs
@@ -31,7 +31,7 @@ impl DateTime { // chrono rejects impossible days (leap years and all); the time is checked // by hand so a leap second (`second == 60`) stays acceptable. let day_ok = self.year > 0 && chrono::NaiveDate::from_ymd_opt(i32::from(self.year), u32::from(self.month), u32::from(self.day)).is_some();- let ok = day_ok && self.hour < 24 && self.minute < 60 && self.second < 61 && self.offset_minutes.is_none_or(|o| o.abs() <= 18 * 60);+ let ok = day_ok && self.hour < 24 && self.minute < 60 && self.second < 61 && self.offset_minutes.is_none_or(|o| o.unsigned_abs() <= 18 * 60); ok.then_some(self) } @@ -135,7 +135,9 @@ impl DateTime { } if let Some(o) = self.offset_minutes { let sign = if o < 0 { '-' } else { '+' };- s += &format!("{sign}{:02}:{:02}", o.abs() / 60, o.abs() % 60);+ // `unsigned_abs`: `i16::MIN`'s `.abs()` overflows the type (a debug panic, and a+ // negative offset rendered as `--546:-8` in release).+ s += &format!("{sign}{:02}:{:02}", o.unsigned_abs() / 60, o.unsigned_abs() % 60); } s }@@ -189,6 +191,18 @@ mod tests { assert_eq!(DateTime::parse_offset("+99:00"), None); } + #[test]+ fn an_i16_min_offset_never_overflows_abs() {+ // `DateTime` deserializes from file data with public fields, so an offset of+ // `i16::MIN` reaches `valid` and `to_iso`: `abs()` on it overflows the type.+ let d = DateTime { year: 2024, month: 6, day: 1, hour: 13, minute: 45, second: 59, millis: 0, offset_minutes: Some(-32768) };+ assert!(d.valid().is_none(), "an offset past ±18 h is not a real offset");+ assert_eq!(d.to_iso(), "2024-06-01T13:45:59-546:08");+ let inside = DateTime { year: 2024, month: 6, day: 1, hour: 0, minute: 0, second: 0, millis: 0, offset_minutes: Some(-330) };+ assert!(inside.valid().is_some());+ assert_eq!(inside.to_iso(), "2024-06-01T00:00:00-05:30");+ }+ #[test] fn unicode_offsets_are_rejected_without_panicking() { for offset in ["+1€", "-1€", "+€1", "-€1"] {