fix: prevent panic when merging a notetype with more fields

rslib/src/notetype/merge.rs

@@ -25,11 +25,20 @@ impl Notetype {
         for (index, field) in other.fields.iter().enumerate() {
             match self.find_field(field) {
                 Some(i) if i == index => (),
-                Some(i) => self.fields.swap(i, index),
+                // `other` may have more fields than us (e.g. a corrupt notetype
+                // with a field matching one of ours twice). Only re-align when
+                // `index` is a real slot in our list; otherwise the field is
+                // already present, so leave it in place rather than panicking.
+                Some(i) => {
+                    if index < self.fields.len() {
+                        self.fields.swap(i, index);
+                    }
+                }
                 None => {
                     let mut missing = field.clone();
                     missing.ord.take();
-                    self.fields.insert(index, missing);
+                    // clamp so a longer `other` appends instead of panicking
+                    self.fields.insert(index.min(self.fields.len()), missing);
                 }
             }
         }
@@ -47,11 +56,18 @@ impl Notetype {
         for (index, template) in other.templates.iter().enumerate() {
             match self.find_template(template) {
                 Some(i) if i == index => (),
-                Some(i) => self.templates.swap(i, index),
+                // see merge_fields: only re-align when `index` is a real slot;
+                // a longer `other` would otherwise go out of bounds
+                Some(i) => {
+                    if index < self.templates.len() {
+                        self.templates.swap(i, index);
+                    }
+                }
                 None => {
                     let mut missing = template.clone();
                     missing.ord.take();
-                    self.templates.insert(index, missing);
+                    self.templates
+                        .insert(index.min(self.templates.len()), missing);
                 }
             }
         }
@@ -91,6 +107,8 @@ impl CardTemplate {
 
 #[cfg(test)]
 mod test {
+    use std::collections::HashSet;
+
     use itertools::assert_equal;
 
     use super::*;
@@ -160,6 +178,47 @@ mod test {
         assert_equal(basic.template_names(), std::iter::once("Card 1"));
     }
 
+    #[test]
+    fn merge_longer_other_with_duplicate_field() {
+        // A malformed incoming notetype (e.g. from a corrupt deck) can have more
+        // fields than ours, including a field that matches one of ours a second
+        // time. Merging it must not panic. See issue #4345.
+        let mut basic = stock::basic(&I18n::template_only());
+        let mut other = basic.clone();
+        other.add_field("Extra1");
+        other.add_field("Extra2");
+        // duplicate the first field so it matches `basic`'s first field again,
+        // at an index beyond `basic`'s length
+        let duplicate = other.fields[0].clone();
+        other.fields.push(duplicate);
+
+        basic.merge(&other);
+
+        // every distinct incoming field is present, with no duplicates
+        let names: HashSet<&str> = basic.field_names().map(String::as_str).collect();
+        assert_eq!(names, HashSet::from(["Front", "Back", "Extra1", "Extra2"]));
+        assert_eq!(basic.fields.len(), 4);
+    }
+
+    #[test]
+    fn merge_longer_other_with_duplicate_template() {
+        let mut basic = stock::basic_forward_reverse(&I18n::template_only());
+        let mut other = basic.clone();
+        other.add_template("Extra1", "", "");
+        other.add_template("Extra2", "", "");
+        let duplicate = other.templates[0].clone();
+        other.templates.push(duplicate);
+
+        basic.merge(&other);
+
+        let names: HashSet<&str> = basic.template_names().map(String::as_str).collect();
+        assert_eq!(
+            names,
+            HashSet::from(["Card 1", "Card 2", "Extra1", "Extra2"])
+        );
+        assert_eq!(basic.templates.len(), 4);
+    }
+
     #[test]
     fn align_template_order() {
         let mut basic_rev = stock::basic_forward_reverse(&I18n::template_only());