From f5ed5155ff0c9a34916f3bc6b3183f24fe4b1aa2 Mon Sep 17 00:00:00 2001 From: no30bit Date: Fri, 20 Jun 2025 04:43:53 +0300 Subject: [PATCH 1/7] impl deserialize --- rust/signed_doc/src/builder.rs | 5 +- rust/signed_doc/src/metadata/mod.rs | 96 +++++++++++++------ .../src/metadata/supported_field.rs | 69 +++++++++---- 3 files changed, 119 insertions(+), 51 deletions(-) diff --git a/rust/signed_doc/src/builder.rs b/rust/signed_doc/src/builder.rs index 96f41edde2e..09ae53563f1 100644 --- a/rust/signed_doc/src/builder.rs +++ b/rust/signed_doc/src/builder.rs @@ -1,5 +1,5 @@ //! Catalyst Signed Document Builder. -use catalyst_types::{catalyst_id::CatalystId, problem_report::ProblemReport}; +use catalyst_types::catalyst_id::CatalystId; use crate::{ signature::{tbs_data, Signature}, @@ -40,8 +40,7 @@ impl Builder { /// # Errors /// - Fails if it is invalid metadata fields JSON object. pub fn with_json_metadata(mut self, json: serde_json::Value) -> anyhow::Result { - let metadata = serde_json::from_value(json)?; - self.metadata = Metadata::from_metadata_fields(metadata, &ProblemReport::new("")); + self.metadata = serde_json::from_value(json)?; Ok(self) } diff --git a/rust/signed_doc/src/metadata/mod.rs b/rust/signed_doc/src/metadata/mod.rs index 7fc2ffb80e2..3feb01c0546 100644 --- a/rust/signed_doc/src/metadata/mod.rs +++ b/rust/signed_doc/src/metadata/mod.rs @@ -63,41 +63,69 @@ const CATEGORY_ID_KEY: &str = "category_id"; #[derive(Clone, Debug, PartialEq, Default)] pub struct Metadata(BTreeMap); +/// Implements [`serde::de::Visitor`], so that [`Metadata`] can implement [`serde::Deserialize`]. +struct MetadataDeserializeContext; + +impl<'de> serde::de::Visitor<'de> for MetadataDeserializeContext { + type Value = Vec; + + fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.write_str("Catalyst Signed Document metadata key-value pairs") + } + + fn visit_map>(self, mut d: A) -> Result { + let mut res = Vec::with_capacity(d.size_hint().unwrap_or(0)); + while let Some(k) = d.next_key::()? { + let v = d.next_value_seed(k)?; + res.push(v); + } + Ok(res) + } +} + +impl<'de> serde::Deserialize<'de> for Metadata { + fn deserialize>(d: D) -> Result { + let fields = d.deserialize_map(MetadataDeserializeContext)?; + let report = ProblemReport::new("Metadata deserialization"); + let res = Self::from_metadata_fields(fields, &report); + if report.is_problematic() { + let mut err = anyhow::anyhow!("Invalid metadata"); + if let Ok(report_json) = serde_json::to_string(&report) { + err = err.context(report_json); + } + Err(serde::de::Error::custom(err)) + } else { + Ok(res) + } + } +} + /// An actual representation of all metadata fields. // TODO: this is maintained as an implementation of `serde` and `coset` for `Metadata` // and should be removed in case `serde` and `coset` are deprecated completely. -#[derive(Clone, Debug, PartialEq, serde::Deserialize, Default)] +#[derive(Clone, Debug, PartialEq, Default)] pub(crate) struct InnerMetadata { /// Document Type, list of `UUIDv4`. - #[serde(rename = "type")] doc_type: Option, /// Document ID `UUIDv7`. id: Option, /// Document Version `UUIDv7`. ver: Option, /// Document Payload Content Type. - #[serde(rename = "content-type")] content_type: Option, /// Document Payload Content Encoding. - #[serde(rename = "content-encoding")] content_encoding: Option, /// Reference to the latest document. - #[serde(rename = "ref", skip_serializing_if = "Option::is_none")] doc_ref: Option, /// Reference to the document template. - #[serde(skip_serializing_if = "Option::is_none")] template: Option, /// Reference to the document reply. - #[serde(skip_serializing_if = "Option::is_none")] reply: Option, /// Reference to the document section. - #[serde(skip_serializing_if = "Option::is_none")] section: Option
, /// Reference to the document collaborators. Collaborator type is TBD. - #[serde(default = "Vec::new", skip_serializing_if = "Vec::is_empty")] collabs: Vec, /// Reference to the parameters document. - #[serde(skip_serializing_if = "Option::is_none")] parameters: Option, } @@ -233,30 +261,38 @@ impl Metadata { } /// Build `Metadata` object from the metadata fields, doing all necessary validation. - pub(crate) fn from_metadata_fields(metadata: InnerMetadata, report: &ProblemReport) -> Self { - if metadata.doc_type.is_none() { - report.missing_field("type", "Missing type field in COSE protected header"); + pub(crate) fn from_metadata_fields(fields: I, report: &ProblemReport) -> Self + where + I: IntoIterator, + { + const REPORT_CONTEXT: &str = "Metadata creation"; + + let mut metadata = Metadata(BTreeMap::new()); + for v in fields { + let k = v.discriminant(); + if metadata.0.insert(k, v).is_some() { + report.duplicate_field( + &k.to_string(), + "Duplicate metadata fields are not allowed", + REPORT_CONTEXT, + ); + } } - if metadata.id.is_none() { - report.missing_field("id", "Missing id field in COSE protected header"); + + if metadata.doc_type().is_err() { + report.missing_field("type", REPORT_CONTEXT); } - if metadata.ver.is_none() { - report.missing_field("ver", "Missing ver field in COSE protected header"); + if metadata.doc_id().is_err() { + report.missing_field("id", REPORT_CONTEXT); } - - if metadata.content_type.is_none() { - report.missing_field( - "content type", - "Missing content_type field in COSE protected header", - ); + if metadata.doc_ver().is_err() { + report.missing_field("ver", REPORT_CONTEXT); + } + if metadata.content_type().is_err() { + report.missing_field("content-type", REPORT_CONTEXT); } - Self( - metadata - .into_iter() - .map(|field| (field.discriminant(), field)) - .collect(), - ) + metadata } /// Converting COSE Protected Header to Metadata. @@ -264,7 +300,7 @@ impl Metadata { protected: &coset::ProtectedHeader, context: &mut DecodeContext, ) -> Self { let metadata = InnerMetadata::from_protected_header(protected, context); - Self::from_metadata_fields(metadata, context.report) + Self::from_metadata_fields(metadata.into_iter(), context.report) } } diff --git a/rust/signed_doc/src/metadata/supported_field.rs b/rust/signed_doc/src/metadata/supported_field.rs index 757eba35a89..ebde71f0b10 100644 --- a/rust/signed_doc/src/metadata/supported_field.rs +++ b/rust/signed_doc/src/metadata/supported_field.rs @@ -5,6 +5,7 @@ use std::fmt::{self, Display}; use std::{cmp, convert::Infallible}; use catalyst_types::uuid::UuidV7; +use serde::Deserialize; use strum::{EnumDiscriminants, EnumTryAs, IntoDiscriminant as _}; use crate::{ @@ -13,7 +14,8 @@ use crate::{ }; /// COSE label. May be either a signed integer or a string. -#[derive(Copy, Clone, Eq, PartialEq)] +#[derive(Copy, Clone, Eq, PartialEq, serde::Deserialize)] +#[serde(untagged, expecting = "8bit unsigned integer or text")] enum Label<'a> { /// Integer label. /// @@ -44,12 +46,10 @@ impl<'a, C> minicbor::Decode<'a, C> for Label<'a> { match d.datatype()? { minicbor::data::Type::U8 => d.u8().map(Self::U8), minicbor::data::Type::String => d.str().map(Self::Str), - _ => { - Err(minicbor::decode::Error::message( - "Datatype is neither 8bit signed integer nor text", - ) - .at(d.position())) - }, + _ => Err(minicbor::decode::Error::message( + "Datatype is neither 8bit unsigned integer nor text", + ) + .at(d.position())), } } } @@ -125,7 +125,7 @@ impl SupportedLabel { /// Try to convert from an arbitrary COSE [`Label`]. fn from_cose(label: Label<'_>) -> Option { match label { - Label::U8(3) => Some(Self::ContentType), + Label::U8(3) | Label::Str("content-type") => Some(Self::ContentType), Label::Str("id") => Some(Self::Id), Label::Str("ref") => Some(Self::Ref), Label::Str("ver") => Some(Self::Ver), @@ -137,7 +137,7 @@ impl SupportedLabel { Label::Str("parameters" | "brand_id" | "campaign_id" | "category_id") => { Some(Self::Parameters) }, - Label::Str("Content-Encoding") => Some(Self::ContentEncoding), + Label::Str("content-encoding") => Some(Self::ContentEncoding), _ => None, } } @@ -155,7 +155,7 @@ impl SupportedLabel { Self::Section => Label::Str("section"), Self::Template => Label::Str("template"), Self::Parameters => Label::Str("parameters"), - Self::ContentEncoding => Label::Str("Content-Encoding"), + Self::ContentEncoding => Label::Str("content-encoding"), } } } @@ -166,6 +166,41 @@ impl Display for SupportedLabel { } } +impl<'de> serde::Deserialize<'de> for SupportedLabel { + fn deserialize>(d: D) -> Result { + let l = Label::deserialize(d)?; + Self::from_cose(l).ok_or_else(|| { + serde::de::Error::custom(format!("Not a supported metadata label ({l})")) + }) + } +} + +impl<'de> serde::de::DeserializeSeed<'de> for SupportedLabel { + type Value = SupportedField; + + fn deserialize>(self, d: D) -> Result { + match self { + SupportedLabel::ContentType => { + Deserialize::deserialize(d).map(SupportedField::ContentType) + }, + SupportedLabel::Id => Deserialize::deserialize(d).map(SupportedField::Id), + SupportedLabel::Ref => Deserialize::deserialize(d).map(SupportedField::Ref), + SupportedLabel::Ver => Deserialize::deserialize(d).map(SupportedField::Ver), + SupportedLabel::Type => Deserialize::deserialize(d).map(SupportedField::Type), + SupportedLabel::Reply => Deserialize::deserialize(d).map(SupportedField::Reply), + SupportedLabel::Collabs => Deserialize::deserialize(d).map(SupportedField::Collabs), + SupportedLabel::Section => Deserialize::deserialize(d).map(SupportedField::Section), + SupportedLabel::Template => Deserialize::deserialize(d).map(SupportedField::Template), + SupportedLabel::Parameters => { + Deserialize::deserialize(d).map(SupportedField::Parameters) + }, + SupportedLabel::ContentEncoding => { + Deserialize::deserialize(d).map(SupportedField::ContentEncoding) + }, + } + } +} + impl minicbor::Decode<'_, crate::decode_context::DecodeContext<'_>> for SupportedField { #[allow(clippy::todo, reason = "Not migrated to `minicbor` yet.")] fn decode( @@ -195,15 +230,13 @@ impl minicbor::Decode<'_, crate::decode_context::DecodeContext<'_>> for Supporte let field = match key { SupportedLabel::ContentType => todo!(), - SupportedLabel::Id => { - d.decode_with(&mut catalyst_types::uuid::CborContext::Tagged) - .map(Self::Id) - }, + SupportedLabel::Id => d + .decode_with(&mut catalyst_types::uuid::CborContext::Tagged) + .map(Self::Id), SupportedLabel::Ref => todo!(), - SupportedLabel::Ver => { - d.decode_with(&mut catalyst_types::uuid::CborContext::Tagged) - .map(Self::Ver) - }, + SupportedLabel::Ver => d + .decode_with(&mut catalyst_types::uuid::CborContext::Tagged) + .map(Self::Ver), SupportedLabel::Type => d.decode_with(ctx).map(Self::Type), SupportedLabel::Reply => todo!(), SupportedLabel::Collabs => todo!(), From fc34fa37d0bc96e137cef1322229a6cda50b4484 Mon Sep 17 00:00:00 2001 From: no30bit Date: Fri, 20 Jun 2025 06:31:10 +0300 Subject: [PATCH 2/7] refactor from_metadata_fields & remove InnerMetadata --- rust/signed_doc/src/builder.rs | 4 +- rust/signed_doc/src/metadata/mod.rs | 194 +++++++----------- .../src/metadata/supported_field.rs | 36 ++-- 3 files changed, 98 insertions(+), 136 deletions(-) diff --git a/rust/signed_doc/src/builder.rs b/rust/signed_doc/src/builder.rs index 09ae53563f1..ba699f4b40c 100644 --- a/rust/signed_doc/src/builder.rs +++ b/rust/signed_doc/src/builder.rs @@ -1,5 +1,5 @@ //! Catalyst Signed Document Builder. -use catalyst_types::catalyst_id::CatalystId; +use catalyst_types::{catalyst_id::CatalystId, problem_report::ProblemReport}; use crate::{ signature::{tbs_data, Signature}, @@ -40,7 +40,7 @@ impl Builder { /// # Errors /// - Fails if it is invalid metadata fields JSON object. pub fn with_json_metadata(mut self, json: serde_json::Value) -> anyhow::Result { - self.metadata = serde_json::from_value(json)?; + self.metadata = Metadata::from_json(json, &ProblemReport::new("")); Ok(self) } diff --git a/rust/signed_doc/src/metadata/mod.rs b/rust/signed_doc/src/metadata/mod.rs index 3feb01c0546..26957fe5784 100644 --- a/rust/signed_doc/src/metadata/mod.rs +++ b/rust/signed_doc/src/metadata/mod.rs @@ -63,93 +63,6 @@ const CATEGORY_ID_KEY: &str = "category_id"; #[derive(Clone, Debug, PartialEq, Default)] pub struct Metadata(BTreeMap); -/// Implements [`serde::de::Visitor`], so that [`Metadata`] can implement [`serde::Deserialize`]. -struct MetadataDeserializeContext; - -impl<'de> serde::de::Visitor<'de> for MetadataDeserializeContext { - type Value = Vec; - - fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - f.write_str("Catalyst Signed Document metadata key-value pairs") - } - - fn visit_map>(self, mut d: A) -> Result { - let mut res = Vec::with_capacity(d.size_hint().unwrap_or(0)); - while let Some(k) = d.next_key::()? { - let v = d.next_value_seed(k)?; - res.push(v); - } - Ok(res) - } -} - -impl<'de> serde::Deserialize<'de> for Metadata { - fn deserialize>(d: D) -> Result { - let fields = d.deserialize_map(MetadataDeserializeContext)?; - let report = ProblemReport::new("Metadata deserialization"); - let res = Self::from_metadata_fields(fields, &report); - if report.is_problematic() { - let mut err = anyhow::anyhow!("Invalid metadata"); - if let Ok(report_json) = serde_json::to_string(&report) { - err = err.context(report_json); - } - Err(serde::de::Error::custom(err)) - } else { - Ok(res) - } - } -} - -/// An actual representation of all metadata fields. -// TODO: this is maintained as an implementation of `serde` and `coset` for `Metadata` -// and should be removed in case `serde` and `coset` are deprecated completely. -#[derive(Clone, Debug, PartialEq, Default)] -pub(crate) struct InnerMetadata { - /// Document Type, list of `UUIDv4`. - doc_type: Option, - /// Document ID `UUIDv7`. - id: Option, - /// Document Version `UUIDv7`. - ver: Option, - /// Document Payload Content Type. - content_type: Option, - /// Document Payload Content Encoding. - content_encoding: Option, - /// Reference to the latest document. - doc_ref: Option, - /// Reference to the document template. - template: Option, - /// Reference to the document reply. - reply: Option, - /// Reference to the document section. - section: Option
, - /// Reference to the document collaborators. Collaborator type is TBD. - collabs: Vec, - /// Reference to the parameters document. - parameters: Option, -} - -impl InnerMetadata { - /// Converts into an iterator over present fields fields. - fn into_iter(self) -> impl Iterator { - [ - self.doc_type.map(SupportedField::Type), - self.id.map(SupportedField::Id), - self.ver.map(SupportedField::Ver), - self.content_type.map(SupportedField::ContentType), - self.content_encoding.map(SupportedField::ContentEncoding), - self.doc_ref.map(SupportedField::Ref), - self.template.map(SupportedField::Template), - self.reply.map(SupportedField::Reply), - self.section.map(SupportedField::Section), - (!self.collabs.is_empty()).then_some(SupportedField::Collabs(self.collabs)), - self.parameters.map(SupportedField::Parameters), - ] - .into_iter() - .flatten() - } -} - impl Metadata { /// Return Document Type `DocType` - a list of `UUIDv4`. /// @@ -261,11 +174,8 @@ impl Metadata { } /// Build `Metadata` object from the metadata fields, doing all necessary validation. - pub(crate) fn from_metadata_fields(fields: I, report: &ProblemReport) -> Self - where - I: IntoIterator, - { - const REPORT_CONTEXT: &str = "Metadata creation"; + pub(crate) fn from_fields(fields: Vec, report: &ProblemReport) -> Self { + const REPORT_CONTEXT: &str = "Metadata building"; let mut metadata = Metadata(BTreeMap::new()); for v in fields { @@ -295,16 +205,21 @@ impl Metadata { metadata } - /// Converting COSE Protected Header to Metadata. - pub(crate) fn from_protected_header( - protected: &coset::ProtectedHeader, context: &mut DecodeContext, - ) -> Self { - let metadata = InnerMetadata::from_protected_header(protected, context); - Self::from_metadata_fields(metadata.into_iter(), context.report) + /// Build `Metadata` object from the metadata fields, doing all necessary validation. + pub(crate) fn from_json(fields: serde_json::Value, report: &ProblemReport) -> Self { + let fields = serde::Deserializer::deserialize_map(fields, MetadataDeserializeContext) + .inspect_err(|err| { + report.other( + &format!("Unable to deserialize json: {err}"), + "Metadata building from json", + ); + }) + .unwrap_or_default(); + Self::from_fields(fields, report) } } -impl InnerMetadata { +impl Metadata { /// Converting COSE Protected Header to Metadata fields, collecting decoding report /// issues. #[allow( @@ -318,11 +233,11 @@ impl InnerMetadata { /// header. const COSE_DECODING_CONTEXT: &str = "COSE Protected Header to Metadata"; - let mut metadata = Self::default(); + let mut metadata_fields = vec![]; if let Some(value) = protected.header.content_type.as_ref() { match ContentType::try_from(value) { - Ok(ct) => metadata.content_type = Some(ct), + Ok(ct) => metadata_fields.push(SupportedField::ContentType(ct)), Err(e) => { context.report.conversion_error( "COSE protected header content type", @@ -339,7 +254,7 @@ impl InnerMetadata { |key| matches!(key, coset::Label::Text(label) if label.eq_ignore_ascii_case(CONTENT_ENCODING_KEY)), ) { match ContentEncoding::try_from(value) { - Ok(ce) => metadata.content_encoding = Some(ce), + Ok(ce) => metadata_fields.push(SupportedField::ContentEncoding(ce)), Err(e) => { context.report.conversion_error( "COSE protected header content encoding", @@ -351,7 +266,7 @@ impl InnerMetadata { } } - metadata.doc_type = cose_protected_header_find( + if let Some(value) = cose_protected_header_find( protected, |key| matches!(key, coset::Label::Text(label) if label.eq_ignore_ascii_case(TYPE_KEY)), ) @@ -361,48 +276,64 @@ impl InnerMetadata { context, ) .ok() - }); + }) { + metadata_fields.push(SupportedField::Type(value)); + } - metadata.id = decode_document_field_from_protected_header::( + if let Some(value) = decode_document_field_from_protected_header::( protected, ID_KEY, COSE_DECODING_CONTEXT, context.report, ) - .map(|v| v.0); + .map(|v| v.0) + { + metadata_fields.push(SupportedField::Id(value)); + } - metadata.ver = decode_document_field_from_protected_header::( + if let Some(value) = decode_document_field_from_protected_header::( protected, VER_KEY, COSE_DECODING_CONTEXT, context.report, ) - .map(|v| v.0); + .map(|v| v.0) + { + metadata_fields.push(SupportedField::Ver(value)); + } - metadata.doc_ref = decode_document_field_from_protected_header( + if let Some(value) = decode_document_field_from_protected_header( protected, REF_KEY, COSE_DECODING_CONTEXT, context.report, - ); - metadata.template = decode_document_field_from_protected_header( + ) { + metadata_fields.push(SupportedField::Ref(value)); + } + if let Some(value) = decode_document_field_from_protected_header( protected, TEMPLATE_KEY, COSE_DECODING_CONTEXT, context.report, - ); - metadata.reply = decode_document_field_from_protected_header( + ) { + metadata_fields.push(SupportedField::Template(value)); + } + if let Some(value) = decode_document_field_from_protected_header( protected, REPLY_KEY, COSE_DECODING_CONTEXT, context.report, - ); - metadata.section = decode_document_field_from_protected_header( + ) { + metadata_fields.push(SupportedField::Reply(value)); + } + if let Some(value) = decode_document_field_from_protected_header( protected, SECTION_KEY, COSE_DECODING_CONTEXT, context.report, - ); + ) { + metadata_fields.push(SupportedField::Section(value)); + } // process `parameters` field and all its aliases let (parameters, has_multiple_fields) = [ @@ -428,7 +359,9 @@ impl InnerMetadata { "Validation of parameters field aliases" ); } - metadata.parameters = parameters; + if let Some(value) = parameters { + metadata_fields.push(SupportedField::Parameters(value)); + } if let Some(cbor_doc_collabs) = cose_protected_header_find(protected, |key| { key == &coset::Label::Text(COLLABS_KEY.to_string()) @@ -452,7 +385,9 @@ impl InnerMetadata { }, } } - metadata.collabs = c; + if !c.is_empty() { + metadata_fields.push(SupportedField::Collabs(c)); + } } else { context.report.conversion_error( "CBOR COSE protected header collaborators", @@ -463,7 +398,7 @@ impl InnerMetadata { }; } - metadata + Self::from_fields(metadata_fields, context.report) } } @@ -596,3 +531,24 @@ impl minicbor::Decode<'_, crate::decode_context::DecodeContext<'_>> for Metadata first_err.map_or(Ok(Self(metadata_map)), Err) } } + +/// Implements [`serde::de::Visitor`], so that [`Metadata`] can be +/// deserialized by [`serde::Deserializer::deserialize_map`]. +struct MetadataDeserializeContext; + +impl<'de> serde::de::Visitor<'de> for MetadataDeserializeContext { + type Value = Vec; + + fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.write_str("Catalyst Signed Document metadata key-value pairs") + } + + fn visit_map>(self, mut d: A) -> Result { + let mut res = Vec::with_capacity(d.size_hint().unwrap_or(0)); + while let Some(k) = d.next_key::()? { + let v = d.next_value_seed(k)?; + res.push(v); + } + Ok(res) + } +} diff --git a/rust/signed_doc/src/metadata/supported_field.rs b/rust/signed_doc/src/metadata/supported_field.rs index ebde71f0b10..d994ffe96ac 100644 --- a/rust/signed_doc/src/metadata/supported_field.rs +++ b/rust/signed_doc/src/metadata/supported_field.rs @@ -1,6 +1,9 @@ //! Catalyst Signed Document unified metadata field. -use std::fmt::{self, Display}; +use std::{ + borrow::Cow, + fmt::{self, Display}, +}; #[cfg(test)] use std::{cmp, convert::Infallible}; @@ -14,8 +17,7 @@ use crate::{ }; /// COSE label. May be either a signed integer or a string. -#[derive(Copy, Clone, Eq, PartialEq, serde::Deserialize)] -#[serde(untagged, expecting = "8bit unsigned integer or text")] +#[derive(Copy, Clone, Eq, PartialEq)] enum Label<'a> { /// Integer label. /// @@ -46,10 +48,12 @@ impl<'a, C> minicbor::Decode<'a, C> for Label<'a> { match d.datatype()? { minicbor::data::Type::U8 => d.u8().map(Self::U8), minicbor::data::Type::String => d.str().map(Self::Str), - _ => Err(minicbor::decode::Error::message( - "Datatype is neither 8bit unsigned integer nor text", - ) - .at(d.position())), + _ => { + Err(minicbor::decode::Error::message( + "Datatype is neither 8bit unsigned integer nor text", + ) + .at(d.position())) + }, } } } @@ -168,8 +172,8 @@ impl Display for SupportedLabel { impl<'de> serde::Deserialize<'de> for SupportedLabel { fn deserialize>(d: D) -> Result { - let l = Label::deserialize(d)?; - Self::from_cose(l).ok_or_else(|| { + let l = Cow::deserialize(d)?; + Self::from_cose(Label::Str(&l)).ok_or_else(|| { serde::de::Error::custom(format!("Not a supported metadata label ({l})")) }) } @@ -230,13 +234,15 @@ impl minicbor::Decode<'_, crate::decode_context::DecodeContext<'_>> for Supporte let field = match key { SupportedLabel::ContentType => todo!(), - SupportedLabel::Id => d - .decode_with(&mut catalyst_types::uuid::CborContext::Tagged) - .map(Self::Id), + SupportedLabel::Id => { + d.decode_with(&mut catalyst_types::uuid::CborContext::Tagged) + .map(Self::Id) + }, SupportedLabel::Ref => todo!(), - SupportedLabel::Ver => d - .decode_with(&mut catalyst_types::uuid::CborContext::Tagged) - .map(Self::Ver), + SupportedLabel::Ver => { + d.decode_with(&mut catalyst_types::uuid::CborContext::Tagged) + .map(Self::Ver) + }, SupportedLabel::Type => d.decode_with(ctx).map(Self::Type), SupportedLabel::Reply => todo!(), SupportedLabel::Collabs => todo!(), From 53f21f997f30de4027ea81e591519219f21d0cac Mon Sep 17 00:00:00 2001 From: no30bit Date: Fri, 20 Jun 2025 06:47:02 +0300 Subject: [PATCH 3/7] rename visitor --- rust/signed_doc/src/metadata/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rust/signed_doc/src/metadata/mod.rs b/rust/signed_doc/src/metadata/mod.rs index 26957fe5784..4c7b46527e8 100644 --- a/rust/signed_doc/src/metadata/mod.rs +++ b/rust/signed_doc/src/metadata/mod.rs @@ -207,7 +207,7 @@ impl Metadata { /// Build `Metadata` object from the metadata fields, doing all necessary validation. pub(crate) fn from_json(fields: serde_json::Value, report: &ProblemReport) -> Self { - let fields = serde::Deserializer::deserialize_map(fields, MetadataDeserializeContext) + let fields = serde::Deserializer::deserialize_map(fields, MetadataDeserializeVisitor) .inspect_err(|err| { report.other( &format!("Unable to deserialize json: {err}"), @@ -534,9 +534,9 @@ impl minicbor::Decode<'_, crate::decode_context::DecodeContext<'_>> for Metadata /// Implements [`serde::de::Visitor`], so that [`Metadata`] can be /// deserialized by [`serde::Deserializer::deserialize_map`]. -struct MetadataDeserializeContext; +struct MetadataDeserializeVisitor; -impl<'de> serde::de::Visitor<'de> for MetadataDeserializeContext { +impl<'de> serde::de::Visitor<'de> for MetadataDeserializeVisitor { type Value = Vec; fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { From 5e233bc0ba2173253eaee7dc93943a44a4905ac0 Mon Sep 17 00:00:00 2001 From: no30bit Date: Fri, 20 Jun 2025 12:39:53 +0300 Subject: [PATCH 4/7] handle aliasing --- .../src/metadata/supported_field.rs | 61 +++++++++++++++---- 1 file changed, 50 insertions(+), 11 deletions(-) diff --git a/rust/signed_doc/src/metadata/supported_field.rs b/rust/signed_doc/src/metadata/supported_field.rs index d994ffe96ac..bb76e194f0a 100644 --- a/rust/signed_doc/src/metadata/supported_field.rs +++ b/rust/signed_doc/src/metadata/supported_field.rs @@ -126,10 +126,41 @@ pub enum SupportedField { } impl SupportedLabel { + /// If [`Label`] is an alias to `self`, return canonical [`Self::to_label`]. + /// Otherwise, returns [`Label`] as is. + fn resolve_aliases(self, label: Label<'_>) -> Label<'_> { + match (self, label) { + // Text version alias to COAP assigned integer. + (Self::ContentType, Label::Str("content-type")) => Self::ContentType.to_label(), + // Legacy names of the parameters field. + (Self::Parameters, Label::Str("brand_id" | "campaign_id" | "category_id")) => { + Self::Parameters.to_label() + }, + // Ignore case to support (e.g. to support "Content-Encoding" alias). + (Self::ContentEncoding, Label::Str(s)) + if s.eq_ignore_ascii_case("content-encoding") => + { + Self::ContentEncoding.to_label() + }, + // Other fields don't have aliases. + _ => label, + } + } + /// Try to convert from an arbitrary COSE [`Label`]. - fn from_cose(label: Label<'_>) -> Option { + /// This will resolve `Self` variant aliases (but only the explicitly allowed ones). + fn from_label_aliased(label: Label<'_>, allowed_aliasing: &[Self]) -> Option { + let label = allowed_aliasing + .iter() + .fold(label, |l, allowed| allowed.resolve_aliases(l)); + Self::from_label(label) + } + + /// Try to convert from an arbitrary COSE [`Label`]. + /// This doesn't allow any aliases. + fn from_label(label: Label<'_>) -> Option { match label { - Label::U8(3) | Label::Str("content-type") => Some(Self::ContentType), + Label::U8(3) => Some(Self::ContentType), Label::Str("id") => Some(Self::Id), Label::Str("ref") => Some(Self::Ref), Label::Str("ver") => Some(Self::Ver), @@ -138,16 +169,14 @@ impl SupportedLabel { Label::Str("collabs") => Some(Self::Collabs), Label::Str("section") => Some(Self::Section), Label::Str("template") => Some(Self::Template), - Label::Str("parameters" | "brand_id" | "campaign_id" | "category_id") => { - Some(Self::Parameters) - }, + Label::Str("parameters") => Some(Self::Parameters), Label::Str("content-encoding") => Some(Self::ContentEncoding), _ => None, } } /// Convert to the corresponding COSE [`Label`]. - fn to_cose(self) -> Label<'static> { + fn to_label(self) -> Label<'static> { match self { Self::ContentType => Label::U8(3), Self::Id => Label::Str("id"), @@ -166,14 +195,21 @@ impl SupportedLabel { impl Display for SupportedLabel { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - Display::fmt(&self.to_cose(), f) + Display::fmt(&self.to_label(), f) } } impl<'de> serde::Deserialize<'de> for SupportedLabel { fn deserialize>(d: D) -> Result { + /// Allow aliases for `content-type`, `content-encoding` and `parameters`. + const ALLOWED_ALIASING: &[SupportedLabel] = &[ + SupportedLabel::ContentType, + SupportedLabel::ContentEncoding, + SupportedLabel::Parameters, + ]; + let l = Cow::deserialize(d)?; - Self::from_cose(Label::Str(&l)).ok_or_else(|| { + Self::from_label_aliased(Label::Str(&l), ALLOWED_ALIASING).ok_or_else(|| { serde::de::Error::custom(format!("Not a supported metadata label ({l})")) }) } @@ -211,10 +247,13 @@ impl minicbor::Decode<'_, crate::decode_context::DecodeContext<'_>> for Supporte d: &mut minicbor::Decoder<'_>, ctx: &mut crate::decode_context::DecodeContext<'_>, ) -> Result { const REPORT_CONTEXT: &str = "Metadata field decoding"; + /// Allow aliases for `content-encoding` and `parameters` only. + const ALLOWED_ALIASING: &[SupportedLabel] = + &[SupportedLabel::ContentEncoding, SupportedLabel::Parameters]; let label_pos = d.position(); let label = Label::decode(d, &mut ())?; - let Some(key) = SupportedLabel::from_cose(label) else { + let Some(key) = SupportedLabel::from_label_aliased(label, ALLOWED_ALIASING) else { let value_start = d.position(); d.skip()?; let value_end = d.position(); @@ -260,7 +299,7 @@ impl minicbor::Encode<()> for SupportedField { fn encode( &self, e: &mut minicbor::Encoder, ctx: &mut (), ) -> Result<(), minicbor::encode::Error> { - let key = self.discriminant().to_cose(); + let key = self.discriminant().to_label(); e.encode(key)?; match self { @@ -334,7 +373,7 @@ mod tests { let mut cose_ord = SupportedLabel::VARIANTS.to_vec(); // Sorting by the corresponding COSE labels. - cose_ord.sort_unstable_by(|lhs, rhs| lhs.to_cose().rfc8949_cmp(&rhs.to_cose()).unwrap()); + cose_ord.sort_unstable_by(|lhs, rhs| lhs.to_label().rfc8949_cmp(&rhs.to_label()).unwrap()); assert_eq!(enum_ord, cose_ord); } From f8bca5b23d03a9426868948c8e5c8f3079b08523 Mon Sep 17 00:00:00 2001 From: no30bit Date: Fri, 20 Jun 2025 15:05:55 +0300 Subject: [PATCH 5/7] kebab-case only deserialization for SupportedLabel --- .../src/metadata/supported_field.rs | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/rust/signed_doc/src/metadata/supported_field.rs b/rust/signed_doc/src/metadata/supported_field.rs index d994ffe96ac..122ca4299c5 100644 --- a/rust/signed_doc/src/metadata/supported_field.rs +++ b/rust/signed_doc/src/metadata/supported_field.rs @@ -1,9 +1,6 @@ //! Catalyst Signed Document unified metadata field. -use std::{ - borrow::Cow, - fmt::{self, Display}, -}; +use std::fmt::{self, Display}; #[cfg(test)] use std::{cmp, convert::Infallible}; @@ -92,7 +89,8 @@ impl Display for Label<'_> { #[derive(Clone, Debug, PartialEq, EnumDiscriminants, EnumTryAs)] #[strum_discriminants( name(SupportedLabel), - derive(Ord, PartialOrd), + derive(Ord, PartialOrd, serde::Deserialize), + serde(rename_all = "kebab-case"), cfg_attr(test, derive(strum::VariantArray)) )] #[non_exhaustive] @@ -129,7 +127,7 @@ impl SupportedLabel { /// Try to convert from an arbitrary COSE [`Label`]. fn from_cose(label: Label<'_>) -> Option { match label { - Label::U8(3) | Label::Str("content-type") => Some(Self::ContentType), + Label::U8(3) => Some(Self::ContentType), Label::Str("id") => Some(Self::Id), Label::Str("ref") => Some(Self::Ref), Label::Str("ver") => Some(Self::Ver), @@ -170,15 +168,6 @@ impl Display for SupportedLabel { } } -impl<'de> serde::Deserialize<'de> for SupportedLabel { - fn deserialize>(d: D) -> Result { - let l = Cow::deserialize(d)?; - Self::from_cose(Label::Str(&l)).ok_or_else(|| { - serde::de::Error::custom(format!("Not a supported metadata label ({l})")) - }) - } -} - impl<'de> serde::de::DeserializeSeed<'de> for SupportedLabel { type Value = SupportedField; From c209f25a400da5c034c0fc90d6a2ffdeb11ca145 Mon Sep 17 00:00:00 2001 From: no30bit Date: Fri, 20 Jun 2025 15:13:44 +0300 Subject: [PATCH 6/7] cleanup --- .../src/metadata/supported_field.rs | 77 ++++++------------- 1 file changed, 22 insertions(+), 55 deletions(-) diff --git a/rust/signed_doc/src/metadata/supported_field.rs b/rust/signed_doc/src/metadata/supported_field.rs index b8380398805..65f90297610 100644 --- a/rust/signed_doc/src/metadata/supported_field.rs +++ b/rust/signed_doc/src/metadata/supported_field.rs @@ -45,12 +45,10 @@ impl<'a, C> minicbor::Decode<'a, C> for Label<'a> { match d.datatype()? { minicbor::data::Type::U8 => d.u8().map(Self::U8), minicbor::data::Type::String => d.str().map(Self::Str), - _ => { - Err(minicbor::decode::Error::message( - "Datatype is neither 8bit unsigned integer nor text", - ) - .at(d.position())) - }, + _ => Err(minicbor::decode::Error::message( + "Datatype is neither 8bit unsigned integer nor text", + ) + .at(d.position())), } } } @@ -124,39 +122,9 @@ pub enum SupportedField { } impl SupportedLabel { - /// If [`Label`] is an alias to `self`, return canonical [`Self::to_label`]. - /// Otherwise, returns [`Label`] as is. - fn resolve_aliases(self, label: Label<'_>) -> Label<'_> { - match (self, label) { - // Text version alias to COAP assigned integer. - (Self::ContentType, Label::Str("content-type")) => Self::ContentType.to_label(), - // Legacy names of the parameters field. - (Self::Parameters, Label::Str("brand_id" | "campaign_id" | "category_id")) => { - Self::Parameters.to_label() - }, - // Ignore case to support (e.g. to support "Content-Encoding" alias). - (Self::ContentEncoding, Label::Str(s)) - if s.eq_ignore_ascii_case("content-encoding") => - { - Self::ContentEncoding.to_label() - }, - // Other fields don't have aliases. - _ => label, - } - } - - /// Try to convert from an arbitrary COSE [`Label`]. - /// This will resolve `Self` variant aliases (but only the explicitly allowed ones). - fn from_label_aliased(label: Label<'_>, allowed_aliasing: &[Self]) -> Option { - let label = allowed_aliasing - .iter() - .fold(label, |l, allowed| allowed.resolve_aliases(l)); - Self::from_label(label) - } - /// Try to convert from an arbitrary COSE [`Label`]. /// This doesn't allow any aliases. - fn from_label(label: Label<'_>) -> Option { + fn from_cose(label: Label<'_>) -> Option { match label { Label::U8(3) => Some(Self::ContentType), Label::Str("id") => Some(Self::Id), @@ -167,14 +135,18 @@ impl SupportedLabel { Label::Str("collabs") => Some(Self::Collabs), Label::Str("section") => Some(Self::Section), Label::Str("template") => Some(Self::Template), - Label::Str("parameters") => Some(Self::Parameters), - Label::Str("content-encoding") => Some(Self::ContentEncoding), + Label::Str("parameters" | "brand_id" | "campaign_id" | "category_id") => { + Some(Self::Parameters) + }, + Label::Str(s) if s.eq_ignore_ascii_case("content-encoding") => { + Some(Self::ContentEncoding) + }, _ => None, } } /// Convert to the corresponding COSE [`Label`]. - fn to_label(self) -> Label<'static> { + fn to_cose(self) -> Label<'static> { match self { Self::ContentType => Label::U8(3), Self::Id => Label::Str("id"), @@ -193,7 +165,7 @@ impl SupportedLabel { impl Display for SupportedLabel { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - Display::fmt(&self.to_label(), f) + Display::fmt(&self.to_cose(), f) } } @@ -229,13 +201,10 @@ impl minicbor::Decode<'_, crate::decode_context::DecodeContext<'_>> for Supporte d: &mut minicbor::Decoder<'_>, ctx: &mut crate::decode_context::DecodeContext<'_>, ) -> Result { const REPORT_CONTEXT: &str = "Metadata field decoding"; - /// Allow aliases for `content-encoding` and `parameters` only. - const ALLOWED_ALIASING: &[SupportedLabel] = - &[SupportedLabel::ContentEncoding, SupportedLabel::Parameters]; let label_pos = d.position(); let label = Label::decode(d, &mut ())?; - let Some(key) = SupportedLabel::from_label_aliased(label, ALLOWED_ALIASING) else { + let Some(key) = SupportedLabel::from_cose(label) else { let value_start = d.position(); d.skip()?; let value_end = d.position(); @@ -255,15 +224,13 @@ impl minicbor::Decode<'_, crate::decode_context::DecodeContext<'_>> for Supporte let field = match key { SupportedLabel::ContentType => todo!(), - SupportedLabel::Id => { - d.decode_with(&mut catalyst_types::uuid::CborContext::Tagged) - .map(Self::Id) - }, + SupportedLabel::Id => d + .decode_with(&mut catalyst_types::uuid::CborContext::Tagged) + .map(Self::Id), SupportedLabel::Ref => todo!(), - SupportedLabel::Ver => { - d.decode_with(&mut catalyst_types::uuid::CborContext::Tagged) - .map(Self::Ver) - }, + SupportedLabel::Ver => d + .decode_with(&mut catalyst_types::uuid::CborContext::Tagged) + .map(Self::Ver), SupportedLabel::Type => d.decode_with(ctx).map(Self::Type), SupportedLabel::Reply => todo!(), SupportedLabel::Collabs => todo!(), @@ -281,7 +248,7 @@ impl minicbor::Encode<()> for SupportedField { fn encode( &self, e: &mut minicbor::Encoder, ctx: &mut (), ) -> Result<(), minicbor::encode::Error> { - let key = self.discriminant().to_label(); + let key = self.discriminant().to_cose(); e.encode(key)?; match self { @@ -355,7 +322,7 @@ mod tests { let mut cose_ord = SupportedLabel::VARIANTS.to_vec(); // Sorting by the corresponding COSE labels. - cose_ord.sort_unstable_by(|lhs, rhs| lhs.to_label().rfc8949_cmp(&rhs.to_label()).unwrap()); + cose_ord.sort_unstable_by(|lhs, rhs| lhs.to_cose().rfc8949_cmp(&rhs.to_cose()).unwrap()); assert_eq!(enum_ord, cose_ord); } From d75a26e91c4d33691c795ebb9815ebf90ef2c65d Mon Sep 17 00:00:00 2001 From: no30bit Date: Fri, 20 Jun 2025 15:14:21 +0300 Subject: [PATCH 7/7] fmt --- .../src/metadata/supported_field.rs | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/rust/signed_doc/src/metadata/supported_field.rs b/rust/signed_doc/src/metadata/supported_field.rs index 65f90297610..69f1c589cb4 100644 --- a/rust/signed_doc/src/metadata/supported_field.rs +++ b/rust/signed_doc/src/metadata/supported_field.rs @@ -45,10 +45,12 @@ impl<'a, C> minicbor::Decode<'a, C> for Label<'a> { match d.datatype()? { minicbor::data::Type::U8 => d.u8().map(Self::U8), minicbor::data::Type::String => d.str().map(Self::Str), - _ => Err(minicbor::decode::Error::message( - "Datatype is neither 8bit unsigned integer nor text", - ) - .at(d.position())), + _ => { + Err(minicbor::decode::Error::message( + "Datatype is neither 8bit unsigned integer nor text", + ) + .at(d.position())) + }, } } } @@ -224,13 +226,15 @@ impl minicbor::Decode<'_, crate::decode_context::DecodeContext<'_>> for Supporte let field = match key { SupportedLabel::ContentType => todo!(), - SupportedLabel::Id => d - .decode_with(&mut catalyst_types::uuid::CborContext::Tagged) - .map(Self::Id), + SupportedLabel::Id => { + d.decode_with(&mut catalyst_types::uuid::CborContext::Tagged) + .map(Self::Id) + }, SupportedLabel::Ref => todo!(), - SupportedLabel::Ver => d - .decode_with(&mut catalyst_types::uuid::CborContext::Tagged) - .map(Self::Ver), + SupportedLabel::Ver => { + d.decode_with(&mut catalyst_types::uuid::CborContext::Tagged) + .map(Self::Ver) + }, SupportedLabel::Type => d.decode_with(ctx).map(Self::Type), SupportedLabel::Reply => todo!(), SupportedLabel::Collabs => todo!(),