refactor(rust/signed-doc): Cleanup Builder, make raw_bytes field non-optional (#369)

* wip

* wip

* wip

* wip

* wip

* cleanup

* wip

* wip

* fix clippy

Author: Mr-Leshiy

rust/signed_doc/bins/mk_signed_doc.rs

@@ -67,8 +67,8 @@ impl Cli {
                 let payload = serde_json::to_vec(&json_doc)?;
                 // Start with no signatures.
                 let signed_doc = Builder::new()
-                    .with_decoded_content(payload)
                     .with_json_metadata(metadata)?
+                    .with_decoded_content(payload)?
                     .build();
                 println!(
                     "report {}",

rust/signed_doc/src/builder.rs

@@ -3,13 +3,19 @@ use catalyst_types::{catalyst_id::CatalystId, problem_report::ProblemReport};
 
 use crate::{
     signature::{tbs_data, Signature},
-    CatalystSignedDocument, Content, InnerCatalystSignedDocument, Metadata, Signatures,
-    PROBLEM_REPORT_CTX,
+    CatalystSignedDocument, Content, Metadata, Signatures,
 };
 
 /// Catalyst Signed Document Builder.
 #[derive(Debug)]
-pub struct Builder(InnerCatalystSignedDocument);
+pub struct Builder {
+    /// metadata
+    metadata: Metadata,
+    /// content
+    content: Content,
+    /// signatures
+    signatures: Signatures,
+}
 
 impl Default for Builder {
     fn default() -> Self {
@@ -21,14 +27,11 @@ impl Builder {
     /// Start building a signed document
     #[must_use]
     pub fn new() -> Self {
-        let report = ProblemReport::new(PROBLEM_REPORT_CTX);
-        Self(InnerCatalystSignedDocument {
-            report,
+        Self {
             metadata: Metadata::default(),
             content: Content::default(),
             signatures: Signatures::default(),
-            raw_bytes: None,
-        })
+        }
     }
 
     /// Set document metadata in JSON format
@@ -38,15 +41,21 @@ impl Builder {
     /// - Fails if it is invalid metadata fields JSON object.
     pub fn with_json_metadata(mut self, json: serde_json::Value) -> anyhow::Result<Self> {
         let metadata = serde_json::from_value(json)?;
-        self.0.metadata = Metadata::from_metadata_fields(metadata, &self.0.report);
+        self.metadata = Metadata::from_metadata_fields(metadata, &ProblemReport::new(""));
         Ok(self)
     }
 
     /// Set decoded (original) document content bytes
-    #[must_use]
-    pub fn with_decoded_content(mut self, content: Vec<u8>) -> Self {
-        self.0.content = Content::from_decoded(content);
-        self
+    ///
+    /// # Errors
+    ///  - Compression failure
+    pub fn with_decoded_content(mut self, decoded: Vec<u8>) -> anyhow::Result<Self> {
+        if let Some(encoding) = self.metadata.content_encoding() {
+            self.content = encoding.encode(&decoded)?.into();
+        } else {
+            self.content = decoded.into();
+        }
+        Ok(self)
     }
 
     /// Add a signature to the document
@@ -62,29 +71,49 @@ impl Builder {
         if kid.is_id() {
             anyhow::bail!("Provided kid should be in a uri format, kid: {kid}");
         }
-        let data_to_sign = tbs_data(&kid, &self.0.metadata, &self.0.content)?;
+        let data_to_sign = tbs_data(&kid, &self.metadata, &self.content)?;
         let sign_bytes = sign_fn(data_to_sign);
-        self.0.signatures.push(Signature::new(kid, sign_bytes));
+        self.signatures.push(Signature::new(kid, sign_bytes));
 
         Ok(self)
     }
 
     /// Build a signed document with the collected error report.
     /// Could provide an invalid document.
+    ///
+    /// # Panics
+    ///  Should not panic
     #[must_use]
+    #[allow(
+        clippy::unwrap_used,
+        reason = "At this point all the data MUST be correctly encodable, and the final prepared bytes MUST be correctly decodable as a CatalystSignedDocument object."
+    )]
     pub fn build(self) -> CatalystSignedDocument {
-        self.0.into()
+        let mut e = minicbor::Encoder::new(Vec::new());
+        // COSE_Sign tag
+        // <!https://datatracker.ietf.org/doc/html/rfc8152#page-9>
+        e.tag(minicbor::data::Tag::new(98)).unwrap();
+        e.array(4).unwrap();
+        // protected headers (metadata fields)
+        e.bytes(minicbor::to_vec(&self.metadata).unwrap().as_slice())
+            .unwrap();
+        // empty unprotected headers
+        e.map(0).unwrap();
+        // content
+        e.encode(&self.content).unwrap();
+        // signatures
+        e.encode(self.signatures).unwrap();
+
+        CatalystSignedDocument::try_from(e.into_writer().as_slice()).unwrap()
     }
 }
 
 impl From<&CatalystSignedDocument> for Builder {
     fn from(value: &CatalystSignedDocument) -> Self {
-        Self(InnerCatalystSignedDocument {
+        Self {
             metadata: value.inner.metadata.clone(),
             content: value.inner.content.clone(),
             signatures: value.inner.signatures.clone(),
-            report: value.inner.report.clone(),
-            raw_bytes: None,
-        })
+        }
     }
 }

rust/signed_doc/src/content.rs

@@ -1,78 +1,38 @@
 //! Catalyst Signed Document Content Payload
 
-use anyhow::Context;
-use catalyst_types::problem_report::ProblemReport;
-
-use crate::metadata::ContentEncoding;
-
-/// Decompressed Document Content type bytes.
+/// Document Content bytes (COSE payload).
 #[derive(Debug, Clone, PartialEq, Default)]
-pub struct Content {
-    /// Original Decompressed Document's data bytes
-    data: Option<Vec<u8>>,
-}
+pub struct Content(Vec<u8>);
 
 impl Content {
-    /// Creates a new `Content` value, from the encoded data.
-    /// verifies a Document's content, that it is correctly encoded and it corresponds and
-    /// parsed to the specified type
-    pub(crate) fn from_encoded(
-        mut data: Vec<u8>, content_encoding: Option<ContentEncoding>, report: &ProblemReport,
-    ) -> Self {
-        if let Some(content_encoding) = content_encoding {
-            if let Ok(decoded_data) = content_encoding.decode(&data) {
-                data = decoded_data;
-            } else {
-                report.invalid_value(
-                    "payload",
-                    &hex::encode(&data),
-                    &format!("Invalid Document content, should {content_encoding} encodable"),
-                    "Invalid Document content type.",
-                );
-                return Self::default();
-            }
-        }
-        Self::from_decoded(data)
+    /// Return content bytes.
+    #[must_use]
+    pub fn bytes(&self) -> &[u8] {
+        self.0.as_slice()
     }
 
-    /// Creates a new `Content` value, from the decoded (original) data.
-    pub(crate) fn from_decoded(data: Vec<u8>) -> Self {
-        Self { data: Some(data) }
+    /// Return content byte size.
+    #[must_use]
+    pub fn size(&self) -> usize {
+        self.0.len()
     }
+}
 
-    /// Return an decoded (original) content bytes.
-    ///
-    /// # Errors
-    ///  - Missing Document content
-    pub fn decoded_bytes(&self) -> anyhow::Result<&[u8]> {
-        self.data
-            .as_deref()
-            .ok_or(anyhow::anyhow!("Missing Document content"))
+impl From<Vec<u8>> for Content {
+    fn from(value: Vec<u8>) -> Self {
+        Self(value)
     }
+}
 
-    /// Return an encoded content bytes,
-    /// by the provided `content_encoding` provided field.
-    ///
-    /// # Errors
-    ///  - Missing Document content
-    ///  - Failed to encode content.
-    pub(crate) fn encoded_bytes(
-        &self, content_encoding: Option<ContentEncoding>,
-    ) -> anyhow::Result<Vec<u8>> {
-        let content = self.decoded_bytes()?;
-        if let Some(content_encoding) = content_encoding {
-            content_encoding
-                .encode(content)
-                .context(format!("Failed to encode {content_encoding} content"))
+impl minicbor::Encode<()> for Content {
+    fn encode<W: minicbor::encode::Write>(
+        &self, e: &mut minicbor::Encoder<W>, _ctx: &mut (),
+    ) -> Result<(), minicbor::encode::Error<W::Error>> {
+        if self.0.is_empty() {
+            e.null()?;
         } else {
-            Ok(content.to_vec())
+            e.bytes(self.0.as_slice())?;
         }
-    }
-
-    /// Return content byte size.
-    /// If content is empty returns `0`.
-    #[must_use]
-    pub fn size(&self) -> usize {
-        self.data.as_ref().map(Vec::len).unwrap_or_default()
+        Ok(())
     }
 }

rust/signed_doc/src/lib.rs

@@ -46,7 +46,7 @@ struct InnerCatalystSignedDocument {
     /// raw CBOR bytes of the `CatalystSignedDocument` object.
     /// It is important to keep them to have a consistency what comes from the decoding
     /// process, so we would return the same data again
-    raw_bytes: Option<Vec<u8>>,
+    raw_bytes: Vec<u8>,
 }
 
 /// Keep all the contents private.
@@ -110,12 +110,30 @@ impl CatalystSignedDocument {
         self.inner.metadata.doc_ver()
     }
 
-    /// Return document `Content`.
+    /// Return document content object.
     #[must_use]
-    pub fn doc_content(&self) -> &Content {
+    pub(crate) fn content(&self) -> &Content {
         &self.inner.content
     }
 
+    /// Return document decoded (original/non compressed) content bytes.
+    ///
+    /// # Errors
+    ///  - Decompression failure
+    pub fn decoded_content(&self) -> anyhow::Result<Vec<u8>> {
+        if let Some(encoding) = self.doc_content_encoding() {
+            encoding.decode(self.encoded_content())
+        } else {
+            Ok(self.encoded_content().to_vec())
+        }
+    }
+
+    /// Return document encoded (compressed) content bytes.
+    #[must_use]
+    pub fn encoded_content(&self) -> &[u8] {
+        self.content().bytes()
+    }
+
     /// Return document `ContentType`.
     ///
     /// # Errors
@@ -213,7 +231,7 @@ impl Decode<'_, ()> for CatalystSignedDocument {
         let signatures = Signatures::from_cose_sig_list(&cose_sign.signatures, &report);
 
         let content = if let Some(payload) = cose_sign.payload {
-            Content::from_encoded(payload, metadata.content_encoding(), &report)
+            payload.into()
         } else {
             report.missing_field("COSE Sign Payload", "Missing document content (payload)");
             Content::default()
@@ -224,7 +242,7 @@ impl Decode<'_, ()> for CatalystSignedDocument {
             content,
             signatures,
             report,
-            raw_bytes: Some(cose_bytes.to_vec()),
+            raw_bytes: cose_bytes.to_vec(),
         }
         .into())
     }
@@ -234,33 +252,10 @@ impl<C> Encode<C> for CatalystSignedDocument {
     fn encode<W: minicbor::encode::Write>(
         &self, e: &mut encode::Encoder<W>, _ctx: &mut C,
     ) -> Result<(), encode::Error<W::Error>> {
-        if let Some(raw_bytes) = &self.inner.raw_bytes {
-            e.writer_mut()
-                .write_all(raw_bytes)
-                .map_err(minicbor::encode::Error::write)?;
-        } else {
-            // COSE_Sign tag
-            // <!https://datatracker.ietf.org/doc/html/rfc8152#page-9>
-            e.tag(minicbor::data::Tag::new(98))?;
-            e.array(4)?;
-            // protected headers (metadata fields)
-            e.bytes(
-                minicbor::to_vec(self.doc_meta())
-                    .map_err(minicbor::encode::Error::message)?
-                    .as_slice(),
-            )?;
-            // empty unprotected headers
-            e.map(0)?;
-            // content
-            let content = self
-                .doc_content()
-                .encoded_bytes(self.doc_content_encoding())
-                .map_err(minicbor::encode::Error::message)?;
-            e.bytes(content.as_slice())?;
-            // signatures
-            e.encode(self.signatures())?;
-        }
-
+        let raw_bytes = &self.inner.raw_bytes;
+        e.writer_mut()
+            .write_all(raw_bytes)
+            .map_err(minicbor::encode::Error::write)?;
         Ok(())
     }
 }

rust/signed_doc/src/signature/mod.rs

@@ -116,7 +116,7 @@ pub(crate) fn tbs_data(
         <minicbor::bytes::ByteVec>::from(minicbor::to_vec(metadata)?),
         <minicbor::bytes::ByteVec>::from(protected_header_bytes(kid)?),
         minicbor::bytes::ByteArray::from([]),
-        <minicbor::bytes::ByteVec>::from(content.encoded_bytes(metadata.content_encoding())?),
+        content,
     ))?)
 }
 

rust/signed_doc/src/validator/mod.rs

@@ -321,7 +321,7 @@ where Provider: VerifyingKeyProvider {
         return Ok(false);
     };
 
-    let Ok(tbs_data) = tbs_data(kid, doc.doc_meta(), doc.doc_content()) else {
+    let Ok(tbs_data) = tbs_data(kid, doc.doc_meta(), doc.content()) else {
         doc.report().other(
             "Cannot build a COSE to be signed data",
             "During creating COSE to be signed data",

rust/signed_doc/src/validator/rules/content_encoding.rs

@@ -24,6 +24,17 @@ impl ContentEncodingRule {
                 );
                 return Ok(false);
             }
+            if content_encoding.decode(doc.encoded_content()).is_err() {
+                doc.report().invalid_value(
+                    "payload",
+                    &hex::encode(doc.encoded_content()),
+                    &format!(
+                        "Document content (payload) must decodable by the set content encoding type: {content_encoding}"
+                    ),
+                    "Invalid Document content value",
+                );
+                return Ok(false);
+            }
         } else if !self.optional {
             doc.report().missing_field(
                 "content-encoding",
@@ -54,9 +65,20 @@ mod tests {
                 serde_json::json!({"content-encoding": content_encoding.to_string() }),
             )
             .unwrap()
+            .with_decoded_content(vec![1, 2, 3])
+            .unwrap()
             .build();
         assert!(rule.check(&doc).await.unwrap());
 
+        // empty content (empty bytes) could not be brotli decoded
+        let doc = Builder::new()
+            .with_json_metadata(
+                serde_json::json!({"content-encoding": content_encoding.to_string() }),
+            )
+            .unwrap()
+            .build();
+        assert!(!rule.check(&doc).await.unwrap());
+
         let doc = Builder::new()
             .with_json_metadata(serde_json::json!({}))
             .unwrap()