Cap steps (#1548)

* Fix steps being mistaken for seconds

* Cap steps at `u32::max` seconds

* Fix overflow of steps in Rust

* Prevent overflow of `IntervalKind`

* Prevent overflow in `revlod/mod.rs`

Also replace some `as` with `from` and `try_from` as is recommended to
highlight potential issues.

* Ensure v2 doesn't store overflowing revlog ivls

* Lower steps cap in deck options

Whereas large card intervals are converted to days, revlog intervals use
i32s to store large numbers of seconds.

* Format

pylib/anki/scheduler/v2.py

@@ -755,8 +755,8 @@ limit ?"""
                 card.id,
                 self.col.usn(),
                 ease,
-                ivl,
+                saturated_i32(ivl),
-                lastIvl,
+                saturated_i32(lastIvl),
                 card.factor,
                 card.time_taken(),
                 type,
@@ -888,8 +888,8 @@ limit ?"""
                 card.id,
                 self.col.usn(),
                 ease,
-                -delay or card.ivl,
+                saturated_i32(-delay or card.ivl),
-                card.lastIvl,
+                saturated_i32(card.lastIvl),
                 card.factor,
                 card.time_taken(),
                 type,
@@ -1152,3 +1152,10 @@ and (queue={QUEUE_TYPE_NEW} or (queue={QUEUE_TYPE_REV} and due<=?))""",
     def _is_finished(self) -> bool:
         "Don't use this, it is a stop-gap until this code is refactored."
         return not any((self.newCount, self.revCount, self._immediate_learn_count))
+
+
+def saturated_i32(number: int) -> int:
+    """Avoid problems on the backend by ensuring reasonably sized values."""
+    I32_MIN = -(2 ** 31)
+    I32_MAX = 2 ** 31 - 1
+    return min(max(number, I32_MIN), I32_MAX)

rslib/src/revlog/mod.rs

@@ -78,11 +78,12 @@ impl Default for RevlogReviewKind {
 
 impl RevlogEntry {
     pub(crate) fn interval_secs(&self) -> u32 {
-        (if self.interval > 0 {
+        u32::try_from(if self.interval > 0 {
-            self.interval * 86_400
+            self.interval.saturating_mul(86_400)
         } else {
-            -self.interval
+            self.interval.saturating_mul(-1)
-        }) as u32
+        })
+        .unwrap()
     }
 }
 
@@ -98,9 +99,9 @@ impl Collection {
             cid: card.id,
             usn,
             button_chosen: 0,
-            interval: card.interval as i32,
+            interval: i32::try_from(card.interval).unwrap_or(i32::MAX),
-            last_interval: original.interval as i32,
+            last_interval: i32::try_from(original.interval).unwrap_or(i32::MAX),
-            ease_factor: card.ease_factor as u32,
+            ease_factor: u32::from(card.ease_factor),
             taken_millis: 0,
             review_kind: RevlogReviewKind::Manual,
         };

rslib/src/scheduler/states/interval_kind.rs

@@ -25,14 +25,14 @@ impl IntervalKind {
     pub(crate) fn as_seconds(self) -> u32 {
         match self {
             IntervalKind::InSecs(secs) => secs,
-            IntervalKind::InDays(days) => days * 86_400,
+            IntervalKind::InDays(days) => days.saturating_mul(86_400),
         }
     }
 
     pub(crate) fn as_revlog_interval(self) -> i32 {
         match self {
             IntervalKind::InDays(days) => days as i32,
-            IntervalKind::InSecs(secs) => -(secs as i32),
+            IntervalKind::InSecs(secs) => -i32::try_from(secs).unwrap_or(i32::MAX),
         }
     }
 }

rslib/src/scheduler/states/mod.rs

@@ -109,7 +109,7 @@ impl<'a> StateContext<'a> {
     pub(crate) fn defaults_for_testing() -> Self {
         Self {
             fuzz_factor: None,
-            steps: LearningSteps::new(&[60.0, 600.0]),
+            steps: LearningSteps::new(&[1.0, 10.0]),
             graduating_interval_good: 1,
             graduating_interval_easy: 4,
             initial_ease_factor: 2.5,
@@ -118,7 +118,7 @@ impl<'a> StateContext<'a> {
             interval_multiplier: 1.0,
             maximum_review_interval: 36500,
             leech_threshold: 8,
-            relearn_steps: LearningSteps::new(&[600.0]),
+            relearn_steps: LearningSteps::new(&[10.0]),
             lapse_multiplier: 0.0,
             minimum_lapse_interval: 1,
             in_filtered_deck: false,

rslib/src/scheduler/states/steps.rs

@@ -5,6 +5,7 @@ const DEFAULT_SECS_IF_MISSING: u32 = 60;
 
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub(crate) struct LearningSteps<'a> {
+    /// The steps in minutes.
     steps: &'a [f32],
 }
 
@@ -13,6 +14,7 @@ fn to_secs(v: f32) -> u32 {
 }
 
 impl<'a> LearningSteps<'a> {
+    /// Takes `steps` as minutes.
     pub(crate) fn new(steps: &[f32]) -> LearningSteps<'_> {
         LearningSteps { steps }
     }
@@ -51,11 +53,11 @@ impl<'a> LearningSteps<'a> {
             let next = if self.steps.len() > 1 {
                 self.secs_at_index(idx + 1).unwrap_or(60)
             } else {
-                current * 2
+                current.saturating_mul(2)
             }
             .max(current);
 
-            Some((current + next) / 2)
+            Some(current.saturating_add(next) / 2)
         } else {
             None
         }

ts/deck-options/steps.ts

@@ -49,7 +49,9 @@ function stringToMinutes(text: string): number {
         const [_, num, suffix] = match;
         const unit = suffixToUnit(suffix);
         const seconds = unitSeconds(unit) * parseInt(num, 10);
-        return seconds / 60;
+        // should be representable as negative i32 seconds in a revlog
+        const capped_seconds = Math.min(seconds, 2 ** 31);
+        return capped_seconds / 60;
     } else {
         return 0;
     }