der: have SetOf(Vec)::insert check for duplicates (#2217)

Previously they would detect duplicates at sorting time, which would
both leave the set not completely sorted, and containing duplicates,
both of which are disallowed.

This adds an eager initial check the given item is not a duplicate prior
to the actual state mutation.

Closes #2123

Author: tarcieri

der/src/asn1/set_of.rs

@@ -46,23 +46,22 @@ where
 
     /// Add an item to this [`SetOf`].
     ///
-    /// Items MUST be added in lexicographical order according to the
-    /// [`DerOrd`] impl on `T`.
+    /// Items MUST be added in lexicographical order according to the [`DerOrd`] impl on `T`.
     #[deprecated(since = "0.7.6", note = "use `insert` or `insert_ordered` instead")]
     pub fn add(&mut self, new_elem: T) -> Result<(), Error> {
         self.insert_ordered(new_elem)
     }
 
     /// Insert an item into this [`SetOf`].
     pub fn insert(&mut self, item: T) -> Result<(), Error> {
+        check_duplicate(&item, self.iter())?;
         self.inner.push(item)?;
         der_sort(self.inner.as_mut())
     }
 
     /// Insert an item into this [`SetOf`].
     ///
-    /// Items MUST be added in lexicographical order according to the
-    /// [`DerOrd`] impl on `T`.
+    /// Items MUST be added in lexicographical order according to the [`DerOrd`] impl on `T`.
     pub fn insert_ordered(&mut self, item: T) -> Result<(), Error> {
         // Ensure set elements are lexicographically ordered
         if let Some(last) = self.inner.last() {
@@ -266,6 +265,7 @@ where
 
     /// Insert an item into this [`SetOfVec`]. Must be unique.
     pub fn insert(&mut self, item: T) -> Result<(), Error> {
+        check_duplicate(&item, self.iter())?;
         self.inner.push(item);
         der_sort(&mut self.inner)
     }
@@ -417,7 +417,7 @@ where
     }
 }
 
-// Implement by hand because the derive would create invalid values.
+// Implement by hand because custom derive would create invalid values.
 // Use the conversion from Vec to create a valid value.
 #[cfg(feature = "arbitrary")]
 impl<'a, T> arbitrary::Arbitrary<'a> for SetOfVec<T>
@@ -434,6 +434,24 @@ where
     }
 }
 
+/// Check if the given item is a duplicate, given an iterator over sorted items (which we can
+/// short-circuit once we hit `Ordering::Less`.
+fn check_duplicate<'a, T, I>(item: &T, iter: I) -> Result<(), Error>
+where
+    T: DerOrd + 'a,
+    I: Iterator<Item = &'a T>,
+{
+    for item2 in iter {
+        match item.der_cmp(item2)? {
+            Ordering::Less => return Ok(()), // all remaining items are greater
+            Ordering::Equal => return Err(ErrorKind::SetDuplicate.into()),
+            Ordering::Greater => continue,
+        }
+    }
+
+    Ok(())
+}
+
 /// Ensure set elements are lexicographically ordered using [`DerOrd`].
 fn check_der_ordering<T: DerOrd>(a: &T, b: &T) -> Result<(), Error> {
     match a.der_cmp(b)? {
@@ -480,6 +498,21 @@ mod tests {
     use super::SetOfVec;
     use crate::{DerOrd, ErrorKind};
 
+    #[test]
+    fn setof_insert() {
+        let mut setof = SetOf::<u8, 10>::new();
+        setof.insert(42).unwrap();
+        assert_eq!(setof.len(), 1);
+        assert_eq!(*setof.iter().next().unwrap(), 42);
+
+        // Ensure duplicates are disallowed
+        assert_eq!(
+            setof.insert(42).unwrap_err().kind(),
+            ErrorKind::SetDuplicate
+        );
+        assert_eq!(setof.len(), 1);
+    }
+
     #[test]
     fn setof_tryfrom_array() {
         let arr = [3u16, 2, 1, 65535, 0];
@@ -505,6 +538,22 @@ mod tests {
         assert_eq!(set1.der_cmp(&set2), Ok(Ordering::Greater));
     }
 
+    #[cfg(feature = "alloc")]
+    #[test]
+    fn setofvec_insert() {
+        let mut setof = SetOfVec::new();
+        setof.insert(42).unwrap();
+        assert_eq!(setof.len(), 1);
+        assert_eq!(*setof.iter().next().unwrap(), 42);
+
+        // Ensure duplicates are disallowed
+        assert_eq!(
+            setof.insert(42).unwrap_err().kind(),
+            ErrorKind::SetDuplicate
+        );
+        assert_eq!(setof.len(), 1);
+    }
+
     #[cfg(feature = "alloc")]
     #[test]
     fn setofvec_tryfrom_array() {