From 2976d69d82a2d3d5ca2e920b2ef0464edd4fee1d Mon Sep 17 00:00:00 2001 From: Daniel Shiposha Date: Wed, 11 May 2022 22:45:36 +0000 Subject: [PATCH] Add properties key chars check --- --- a/pallets/common/src/lib.rs +++ b/pallets/common/src/lib.rs @@ -18,7 +18,7 @@ use core::ops::{Deref, DerefMut}; use pallet_evm_coder_substrate::{SubstrateRecorder, WithRecorder}; -use sp_std::{vec::Vec, collections::btree_map::BTreeMap}; +use sp_std::vec::Vec; use pallet_evm::account::CrossAccountId; use frame_support::{ dispatch::{DispatchErrorWithPostInfo, DispatchResultWithPostInfo, Weight, PostDispatchInfo}, @@ -36,7 +36,7 @@ CUSTOM_DATA_LIMIT, CollectionLimits, CustomDataLimit, CreateCollectionData, SponsorshipState, CreateItemExData, SponsoringRateLimit, budget::Budget, COLLECTION_FIELD_LIMIT, CollectionField, PhantomType, Property, Properties, PropertiesPermissionMap, PropertyKey, PropertyPermission, - PropertiesError, PropertyKeyPermission, TokenData, CollectionPropertiesPermissionsVec, + PropertiesError, PropertyKeyPermission, TokenData, TrySet, }; pub use pallet::*; use sp_core::H160; @@ -374,6 +374,9 @@ /// Unable to read array of unbounded keys UnableToReadUnboundedKeys, + + /// Only ASCII letters, digits, and '_', '-' are allowed + InvalidCharacterInPropertyKey, } #[pallet::storage] @@ -676,19 +679,20 @@ meta_update_permission: data.meta_update_permission.unwrap_or_default(), }; - CollectionProperties::::insert( - id, - Properties::from_collection_props_vec(data.properties) - .map_err(|e| -> Error { e.into() })?, - ); + let mut collection_properties = up_data_structs::CollectionProperties::get(); + collection_properties.try_set_from_iter( + data.properties.into_iter() + .map(|p| (p.key, p.value)) + ).map_err(|e| -> Error { e.into() })?; + + CollectionProperties::::insert(id, collection_properties); - let token_props_permissions: PropertiesPermissionMap = data - .token_property_permissions + let mut token_props_permissions = PropertiesPermissionMap::new(); + token_props_permissions.try_set_from_iter( + data.token_property_permissions .into_iter() .map(|property| (property.key, property.permission)) - .collect::>() - .try_into() - .map_err(|_| -> Error { PropertiesError::PropertyLimitReached.into() })?; + ).map_err(|e| -> Error { e.into() })?; CollectionPropertyPermissions::::insert(id, token_props_permissions); @@ -771,7 +775,8 @@ collection.check_is_owner_or_admin(sender)?; CollectionProperties::::try_mutate(collection.id, |properties| { - properties.try_set_property(property.clone()) + let property = property.clone(); + properties.try_set(property.key, property.value) }) .map_err(|e| -> Error { e.into() })?; @@ -799,9 +804,9 @@ ) -> DispatchResult { collection.check_is_owner_or_admin(sender)?; - CollectionProperties::::mutate(collection.id, |properties| { - properties.remove_property(&property_key); - }); + CollectionProperties::::try_mutate(collection.id, |properties| { + properties.remove(&property_key) + }).map_err(|e| -> Error { e.into() })?; Self::deposit_event(Event::CollectionPropertyDeleted( collection.id, @@ -841,7 +846,7 @@ CollectionPropertyPermissions::::try_mutate(collection.id, |permissions| { let property_permission = property_permission.clone(); - permissions.try_insert(property_permission.key, property_permission.permission) + permissions.try_set(property_permission.key, property_permission.permission) }) .map_err(|_| -> Error { PropertiesError::PropertyLimitReached.into() })?; @@ -876,6 +881,19 @@ .collect::, DispatchError>>() } + pub fn check_property_key(key: &PropertyKey) -> Result<(), DispatchError> { + let key_str = sp_std::str::from_utf8(key.as_slice()) + .map_err(|_| >::InvalidCharacterInPropertyKey)?; + + for ch in key_str.chars() { + if !ch.is_ascii_alphanumeric() && ch != '_' && ch != '-' { + return Err(>::InvalidCharacterInPropertyKey.into()); + } + } + + Ok(()) + } + pub fn filter_collection_properties( collection_id: CollectionId, keys: Vec, @@ -885,10 +903,11 @@ let properties = keys .into_iter() .filter_map(|key| { - properties.get_property(&key).map(|value| Property { - key, - value: value.clone(), - }) + properties.get(&key) + .map(|value| Property { + key, + value: value.clone(), + }) }) .collect(); @@ -1222,6 +1241,7 @@ match error { PropertiesError::NoSpaceForProperty => Self::NoSpaceForProperty, PropertiesError::PropertyLimitReached => Self::PropertyLimitReached, + PropertiesError::InvalidCharacterInPropertyKey => Self::InvalidCharacterInPropertyKey, } } } --- a/pallets/nonfungible/src/common.rs +++ b/pallets/nonfungible/src/common.rs @@ -391,10 +391,11 @@ keys.into_iter() .filter_map(|key| { - properties.get_property(&key).map(|value| Property { - key, - value: value.clone(), - }) + properties.get(&key) + .map(|value| Property { + key, + value: value.clone(), + }) }) .collect() } --- a/pallets/nonfungible/src/lib.rs +++ b/pallets/nonfungible/src/lib.rs @@ -21,7 +21,7 @@ use up_data_structs::{ AccessMode, CollectionId, CustomDataLimit, TokenId, CreateCollectionData, CreateNftExData, mapping::TokenAddressMapping, NestingRule, budget::Budget, Property, PropertyPermission, - PropertyKey, PropertyKeyPermission, Properties, + PropertyKey, PropertyKeyPermission, Properties, TrySet, }; use pallet_evm::account::CrossAccountId; use pallet_common::{ @@ -265,7 +265,8 @@ Self::check_token_change_permission(collection, sender, token_id, &property.key)?; >::try_mutate((collection.id, token_id), |properties| { - properties.try_set_property(property.clone()) + let property = property.clone(); + properties.try_set(property.key, property.value) }) .map_err(|e| -> CommonError { e.into() })?; @@ -299,9 +300,9 @@ ) -> DispatchResult { Self::check_token_change_permission(collection, sender, token_id, &property_key)?; - >::mutate((collection.id, token_id), |properties| { - properties.remove_property(&property_key); - }); + >::try_mutate((collection.id, token_id), |properties| { + properties.remove(&property_key) + }).map_err(|e| -> CommonError { e.into() })?; >::deposit_event(CommonEvent::TokenPropertyDeleted( collection.id, @@ -332,7 +333,7 @@ }; let is_property_exists = TokenProperties::::get((collection.id, token_id)) - .get_property(property_key) + .get(property_key) .is_some(); match permission { --- a/primitives/data-structs/src/lib.rs +++ b/primitives/data-structs/src/lib.rs @@ -689,16 +689,82 @@ pub enum PropertiesError { NoSpaceForProperty, PropertyLimitReached, + InvalidCharacterInPropertyKey, +} + +pub trait TrySet: Sized { + type Value; + + fn try_set(&mut self, key: PropertyKey, value: Self::Value) -> Result<(), PropertiesError>; + + fn try_set_from_iter(&mut self, iter: I) -> Result<(), PropertiesError> + where + I: Iterator + { + for (key, value) in iter { + self.try_set(key, value)?; + } + + Ok(()) + } +} + +#[derive(Encode, Decode, TypeInfo, Derivative, Clone, PartialEq, MaxEncodedLen)] +#[derivative(Default(bound = ""))] +pub struct PropertiesMap(BoundedBTreeMap>); + +impl PropertiesMap { + pub fn new() -> Self { + Self(BoundedBTreeMap::new()) + } + + pub fn remove(&mut self, key: &PropertyKey) -> Result, PropertiesError> { + Self::check_property_key(key)?; + + Ok(self.0.remove(key)) + } + + pub fn get(&self, key: &PropertyKey) -> Option<&Value> { + self.0.get(key) + } + + pub fn iter(&self) -> impl Iterator { + self.0.iter() + } + + fn check_property_key(key: &PropertyKey) -> Result<(), PropertiesError> { + let key_str = sp_std::str::from_utf8(key.as_slice()) + .map_err(|_| PropertiesError::InvalidCharacterInPropertyKey)?; + + for ch in key_str.chars() { + if !ch.is_ascii_alphanumeric() && ch != '_' && ch != '-' { + return Err(PropertiesError::InvalidCharacterInPropertyKey); + } + } + + Ok(()) + } +} + +impl TrySet for PropertiesMap { + type Value = Value; + + fn try_set(&mut self, key: PropertyKey, value: Self::Value) -> Result<(), PropertiesError> { + Self::check_property_key(&key)?; + + self.0 + .try_insert(key, value) + .map_err(|_| PropertiesError::PropertyLimitReached)?; + + Ok(()) + } } -pub type PropertiesMap = - BoundedBTreeMap>; -pub type PropertiesPermissionMap = - BoundedBTreeMap>; +pub type PropertiesPermissionMap = PropertiesMap; #[derive(Encode, Decode, TypeInfo, Clone, PartialEq, MaxEncodedLen)] pub struct Properties { - map: PropertiesMap, + map: PropertiesMap, consumed_space: u32, space_limit: u32, } @@ -706,57 +772,47 @@ impl Properties { pub fn new(space_limit: u32) -> Self { Self { - map: BoundedBTreeMap::new(), + map: PropertiesMap::new(), consumed_space: 0, space_limit, } } - pub fn from_collection_props_vec( - data: CollectionPropertiesVec, - ) -> Result { - let mut props = Self::new(MAX_COLLECTION_PROPERTIES_SIZE); + pub fn remove(&mut self, key: &PropertyKey) -> Result, PropertiesError> { + let value = self.map.remove(key)?; - for property in data.into_iter() { - props.try_set_property(property)?; + if let Some(ref value) = value { + let value_len = value.len() as u32; + self.consumed_space -= value_len; } - Ok(props) + Ok(value) } - pub fn try_set_property(&mut self, property: Property) -> Result<(), PropertiesError> { - let value_len = property.value.len(); + pub fn get(&self, key: &PropertyKey) -> Option<&PropertyValue> { + self.map.get(key) + } + pub fn iter(&self) -> impl Iterator { + self.map.iter() + } +} + +impl TrySet for Properties { + type Value = PropertyValue; + + fn try_set(&mut self, key: PropertyKey, value: Self::Value) -> Result<(), PropertiesError> { + let value_len = value.len(); + if self.consumed_space as usize + value_len > self.space_limit as usize { return Err(PropertiesError::NoSpaceForProperty); } - self.map - .try_insert(property.key, property.value) - .map_err(|_| PropertiesError::PropertyLimitReached)?; + self.map.try_set(key, value)?; self.consumed_space += value_len as u32; Ok(()) - } - - pub fn remove_property(&mut self, key: &PropertyKey) { - let property = self.map.get(key); - - if let Some(value) = property { - let value_len = value.len() as u32; - - self.map.remove(key); - self.consumed_space -= value_len; - } - } - - pub fn get_property(&self, key: &PropertyKey) -> Option<&PropertyValue> { - self.map.get(key) - } - - pub fn iter(&self) -> impl Iterator { - self.map.iter() } } -- gitstuff