der: add SetOf(Vec)::insert(_ordered); deprecate add

Renames the insertion methods on `SetOf` and `SetOfVec` from `add` to `insert_ordered`, deprecating the previous `add` method. Also adds an `insert` method which pushes onto the underlying `(Array)Vec` then calls `der_sort`. This should be relatively fast since the underlying algorithm is insertion sort and operating on data which is mostly in-order. The name `insert` is more consistent with `BTreeSet`/`HashSet`.
RustCrypto · May 15, 2023 · 604077c · 604077c
1 parent 57b6ca5
commit 604077c
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 37 deletions.
diff --git a/cms/src/content_info.rs b/cms/src/content_info.rs
@@ -61,7 +61,7 @@ impl TryFrom<Certificate> for ContentInfo {
 
     fn try_from(cert: Certificate) -> der::Result<Self> {
         let mut certs = CertificateSet(Default::default());
-        certs.0.add(CertificateChoices::Certificate(cert))?;
+        certs.0.insert(CertificateChoices::Certificate(cert))?;
 
         // include empty CRLs field instead of omitting it to match OpenSSL's behavior
         let sd = SignedData {
@@ -93,7 +93,7 @@ impl TryFrom<PkiPath> for ContentInfo {
     fn try_from(pki_path: PkiPath) -> der::Result<Self> {
         let mut certs = CertificateSet(Default::default());
         for cert in pki_path {
-            certs.0.add(CertificateChoices::Certificate(cert))?;
+            certs.0.insert(CertificateChoices::Certificate(cert))?;
         }
 
         // include empty CRLs field instead of omitting it to match OpenSSL's behavior

diff --git a/der/src/arrayvec.rs b/der/src/arrayvec.rs
@@ -23,14 +23,11 @@ impl<T, const N: usize> ArrayVec<T, N> {
         }
     }
 
-    /// Add an element to this [`ArrayVec`].
-    ///
-    /// Items MUST be added in lexicographical order according to the `Ord`
-    /// impl on `T`.
-    pub fn add(&mut self, element: T) -> Result<()> {
+    /// Push an item into this [`ArrayVec`].
+    pub fn push(&mut self, item: T) -> Result<()> {
         match self.length.checked_add(1) {
             Some(n) if n <= N => {
-                self.elements[self.length] = Some(element);
+                self.elements[self.length] = Some(item);
                 self.length = n;
                 Ok(())
             }
@@ -138,11 +135,11 @@ mod tests {
     #[test]
     fn add() {
         let mut vec = ArrayVec::<u8, 3>::new();
-        vec.add(1).unwrap();
-        vec.add(2).unwrap();
-        vec.add(3).unwrap();
+        vec.push(1).unwrap();
+        vec.push(2).unwrap();
+        vec.push(3).unwrap();
 
-        assert_eq!(vec.add(4).err().unwrap(), ErrorKind::Overlength.into());
+        assert_eq!(vec.push(4).err().unwrap(), ErrorKind::Overlength.into());
         assert_eq!(vec.len(), 3);
     }
 }
diff --git a/der/src/asn1/sequence_of.rs b/der/src/asn1/sequence_of.rs
@@ -30,7 +30,7 @@ impl<T, const N: usize> SequenceOf<T, N> {
 
     /// Add an element to this [`SequenceOf`].
     pub fn add(&mut self, element: T) -> Result<()> {
-        self.inner.add(element)
+        self.inner.push(element)
     }
 
     /// Get an element of this [`SequenceOf`].

diff --git a/der/src/asn1/set_of.rs b/der/src/asn1/set_of.rs
@@ -44,21 +44,36 @@ where
         }
     }
 
-    /// Add an element to this [`SetOf`].
+    /// Add an item to this [`SetOf`].
     ///
     /// 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<()> {
+        self.insert_ordered(new_elem)
+    }
+
+    /// Insert an item into this [`SetOf`].
+    pub fn insert(&mut self, item: T) -> Result<()> {
+        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`.
+    pub fn insert_ordered(&mut self, item: T) -> Result<()> {
         // Ensure set elements are lexicographically ordered
-        if let Some(last_elem) = self.inner.last() {
-            match new_elem.der_cmp(last_elem)? {
+        if let Some(last) = self.inner.last() {
+            match item.der_cmp(last)? {
                 Ordering::Less => return Err(ErrorKind::SetOrdering.into()),
                 Ordering::Equal => return Err(ErrorKind::SetDuplicate.into()),
                 Ordering::Greater => (),
             }
         }
 
-        self.inner.add(new_elem)
+        self.inner.push(item)
     }
 
     /// Get the nth element from this [`SetOf`].
@@ -102,7 +117,7 @@ where
             let mut result = Self::new();
 
             while !reader.is_finished() {
-                result.inner.add(T::decode(reader)?)?;
+                result.inner.push(T::decode(reader)?)?;
             }
 
             der_sort(result.inner.as_mut())?;
@@ -149,7 +164,7 @@ where
         let mut result = SetOf::new();
 
         for elem in arr {
-            result.add(elem)?;
+            result.insert_ordered(elem)?;
         }
 
         Ok(result)
@@ -232,16 +247,9 @@ where
     ///
     /// Items MUST be added in lexicographical order according to the
     /// [`DerOrd`] impl on `T`.
-    pub fn add(&mut self, new_elem: T) -> Result<()> {
-        // Ensure set elements are lexicographically ordered
-        if let Some(last_elem) = self.inner.last() {
-            if new_elem.der_cmp(last_elem)? != Ordering::Greater {
-                return Err(ErrorKind::SetOrdering.into());
-            }
-        }
-
-        self.inner.push(new_elem);
-        Ok(())
+    #[deprecated(since = "0.7.6", note = "use `insert` or `insert_ordered` instead")]
+    pub fn add(&mut self, item: T) -> Result<()> {
+        self.insert_ordered(item)
     }
 
     /// Extend a [`SetOfVec`] using an iterator.
@@ -256,6 +264,30 @@ where
         der_sort(&mut self.inner)
     }
 
+    /// Insert an item into this [`SetOfVec`]. Must be unique.
+    pub fn insert(&mut self, item: T) -> Result<()> {
+        self.inner.push(item);
+        der_sort(&mut self.inner)
+    }
+
+    /// Insert an item into this [`SetOfVec`]. Must be unique.
+    ///
+    /// Items MUST be added in lexicographical order according to the
+    /// [`DerOrd`] impl on `T`.
+    pub fn insert_ordered(&mut self, item: T) -> Result<()> {
+        // Ensure set elements are lexicographically ordered
+        if let Some(last) = self.inner.last() {
+            match item.der_cmp(last)? {
+                Ordering::Less => return Err(ErrorKind::SetOrdering.into()),
+                Ordering::Equal => return Err(ErrorKind::SetDuplicate.into()),
+                Ordering::Greater => (),
+            }
+        }
+
+        self.inner.push(item);
+        Ok(())
+    }
+
     /// Borrow the elements of this [`SetOfVec`] as a slice.
     pub fn as_slice(&self) -> &[T] {
         self.inner.as_slice()

diff --git a/pkcs7/tests/content_tests.rs b/pkcs7/tests/content_tests.rs
@@ -207,7 +207,7 @@ fn create_pkcs7_signed_data() {
             parameters: None,
         };
         let mut digest_algorithms = DigestAlgorithmIdentifiers::new();
-        digest_algorithms.add(digest_algorithm).unwrap();
+        digest_algorithms.insert(digest_algorithm).unwrap();
         digest_algorithms
     };
 
@@ -223,7 +223,7 @@ fn create_pkcs7_signed_data() {
         let cert: x509_cert::Certificate = x509_cert::Certificate::from_pem(cert_pem).unwrap();
         let cert_choice = CertificateChoices::Certificate(cert);
         let mut certs = CertificateSet::new();
-        certs.add(cert_choice).unwrap();
+        certs.insert(cert_choice).unwrap();
         Some(certs)
     };
 

diff --git a/x509-cert/src/builder.rs b/x509-cert/src/builder.rs
@@ -501,7 +501,7 @@ where
     fn finalize(&mut self) -> der::Result<vec::Vec<u8>> {
         self.info
             .attributes
-            .add(self.extension_req.clone().try_into()?)?;
+            .insert(self.extension_req.clone().try_into()?)?;
 
         self.info.to_der()
     }

diff --git a/x509-cert/src/request.rs b/x509-cert/src/request.rs
@@ -117,7 +117,7 @@ impl TryFrom<ExtensionReq> for Attribute {
 
     fn try_from(extension_req: ExtensionReq) -> der::Result<Attribute> {
         let mut values: SetOfVec<AttributeValue> = Default::default();
-        values.add(Any::encode_from(&extension_req.0)?)?;
+        values.insert(Any::encode_from(&extension_req.0)?)?;
 
         Ok(Attribute {
             oid: ExtensionReq::OID,

diff --git a/x509-cert/tests/name.rs b/x509-cert/tests/name.rs
@@ -122,18 +122,18 @@ fn decode_rdn() {
     assert_eq!(utf8a.to_string(), "JOHN SMITH");
 
     let mut from_scratch = RelativeDistinguishedName::default();
-    assert!(from_scratch.0.add(atav1a.clone()).is_ok());
-    assert!(from_scratch.0.add(atav2a.clone()).is_ok());
+    assert!(from_scratch.0.insert(atav1a.clone()).is_ok());
+    assert!(from_scratch.0.insert(atav2a.clone()).is_ok());
     let reencoded = from_scratch.to_der().unwrap();
     assert_eq!(
         reencoded,
         &hex!("311F300A060355040A0C03313233301106035504030C0A4A4F484E20534D495448")
     );
 
     let mut from_scratch2 = RelativeDistinguishedName::default();
-    assert!(from_scratch2.0.add(atav2a.clone()).is_ok());
+    assert!(from_scratch2.0.insert_ordered(atav2a.clone()).is_ok());
     // fails when caller adds items not in DER lexicographical order
-    assert!(from_scratch2.0.add(atav1a.clone()).is_err());
+    assert!(from_scratch2.0.insert_ordered(atav1a.clone()).is_err());
 
     // allow out-of-order RDNs (see: RustCrypto/formats#625)
     assert!(RelativeDistinguishedName::from_der(