From e2728a70812412aade9322f6ad832731978a4240 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= <76261501+zecakeh@users.noreply.github.com> Date: Sun, 11 Apr 2021 10:15:37 +0200 Subject: [PATCH] identifiers: Make MxcUri less strict --- ruma-client-api/src/r0/media/get_content.rs | 10 +- .../src/r0/media/get_content_as_filename.rs | 13 +- .../src/r0/media/get_content_thumbnail.rs | 15 +- ruma-identifiers-validation/src/mxc_uri.rs | 6 +- ruma-identifiers/CHANGELOG.md | 7 + ruma-identifiers/src/mxc_uri.rs | 130 ++++++++++++------ 6 files changed, 117 insertions(+), 64 deletions(-) diff --git a/ruma-client-api/src/r0/media/get_content.rs b/ruma-client-api/src/r0/media/get_content.rs index 0812d47f..3182c476 100644 --- a/ruma-client-api/src/r0/media/get_content.rs +++ b/ruma-client-api/src/r0/media/get_content.rs @@ -1,7 +1,7 @@ //! [GET /_matrix/media/r0/download/{serverName}/{mediaId}](https://matrix.org/docs/spec/client_server/r0.6.0#get-matrix-media-r0-download-servername-mediaid) use ruma_api::ruma_api; -use ruma_identifiers::{MxcUri, ServerName}; +use ruma_identifiers::{Error, MxcUri, ServerName}; ruma_api! { metadata: { @@ -59,8 +59,12 @@ impl<'a> Request<'a> { } /// Creates a new `Request` with the given url. - pub fn from_url(url: &'a MxcUri) -> Self { - Self { media_id: url.media_id(), server_name: url.server_name(), allow_remote: true } + pub fn from_url(url: &'a MxcUri) -> Result { + if let Some((server_name, media_id)) = url.parts() { + Ok(Self { media_id, server_name, allow_remote: true }) + } else { + Err(Error::InvalidMxcUri) + } } } diff --git a/ruma-client-api/src/r0/media/get_content_as_filename.rs b/ruma-client-api/src/r0/media/get_content_as_filename.rs index ee60102d..cb135db6 100644 --- a/ruma-client-api/src/r0/media/get_content_as_filename.rs +++ b/ruma-client-api/src/r0/media/get_content_as_filename.rs @@ -1,7 +1,7 @@ //! [GET /_matrix/media/r0/download/{serverName}/{mediaId}/{fileName}](https://matrix.org/docs/spec/client_server/r0.6.0#get-matrix-media-r0-download-servername-mediaid-filename) use ruma_api::ruma_api; -use ruma_identifiers::{MxcUri, ServerName}; +use ruma_identifiers::{Error, MxcUri, ServerName}; ruma_api! { metadata: { @@ -64,12 +64,11 @@ impl<'a> Request<'a> { } /// Creates a new `Request` with the given url and filename. - pub fn from_url(url: &'a MxcUri, filename: &'a str) -> Self { - Self { - media_id: url.media_id(), - server_name: url.server_name(), - filename, - allow_remote: true, + pub fn from_url(url: &'a MxcUri, filename: &'a str) -> Result { + if let Some((server_name, media_id)) = url.parts() { + Ok(Self { media_id, server_name, filename, allow_remote: true }) + } else { + Err(Error::InvalidMxcUri) } } } diff --git a/ruma-client-api/src/r0/media/get_content_thumbnail.rs b/ruma-client-api/src/r0/media/get_content_thumbnail.rs index 2c31be9a..0ac94fac 100644 --- a/ruma-client-api/src/r0/media/get_content_thumbnail.rs +++ b/ruma-client-api/src/r0/media/get_content_thumbnail.rs @@ -2,7 +2,7 @@ use js_int::UInt; use ruma_api::ruma_api; -use ruma_identifiers::{MxcUri, ServerName}; +use ruma_identifiers::{Error, MxcUri, ServerName}; use ruma_serde::StringEnum; /// The desired resizing method. @@ -83,14 +83,11 @@ impl<'a> Request<'a> { /// Creates a new `Request` with the given url, desired thumbnail width and /// desired thumbnail height. - pub fn from_url(url: &'a MxcUri, width: UInt, height: UInt) -> Self { - Self { - media_id: url.media_id(), - server_name: url.server_name(), - method: None, - width, - height, - allow_remote: true, + pub fn from_url(url: &'a MxcUri, width: UInt, height: UInt) -> Result { + if let Some((server_name, media_id)) = url.parts() { + Ok(Self { media_id, server_name, method: None, width, height, allow_remote: true }) + } else { + Err(Error::InvalidMxcUri) } } } diff --git a/ruma-identifiers-validation/src/mxc_uri.rs b/ruma-identifiers-validation/src/mxc_uri.rs index 14d25451..6c4f024a 100644 --- a/ruma-identifiers-validation/src/mxc_uri.rs +++ b/ruma-identifiers-validation/src/mxc_uri.rs @@ -1,8 +1,10 @@ +use std::num::NonZeroU8; + use crate::{server_name, Error}; const PROTOCOL: &str = "mxc://"; -pub fn validate(uri: &str) -> Result<(&str, &str), Error> { +pub fn validate(uri: &str) -> Result { let uri = match uri.strip_prefix(PROTOCOL) { Some(uri) => uri, None => return Err(Error::InvalidMxcUri), @@ -20,7 +22,7 @@ pub fn validate(uri: &str) -> Result<(&str, &str), Error> { media_id.bytes().all(|b| matches!(b, b'0'..=b'9' | b'a'..=b'z' | b'A'..=b'Z' | b'-' )); if media_id_is_valid && server_name::validate(server_name).is_ok() { - Ok((media_id, server_name)) + Ok(NonZeroU8::new((index + 6) as u8).unwrap()) } else { Err(Error::InvalidMxcUri) } diff --git a/ruma-identifiers/CHANGELOG.md b/ruma-identifiers/CHANGELOG.md index d3d19e2a..1b2381bc 100644 --- a/ruma-identifiers/CHANGELOG.md +++ b/ruma-identifiers/CHANGELOG.md @@ -1,5 +1,12 @@ # [unreleased] +Breaking changes: + +* Make `MxcUri` accept any string + * `TryFrom` trait methods were changed to `From` trait + * Use `MxcUri::is_valid` to make sure it is spec-compliant + * `MxcUri::{media_id, server_name}` return `Some({value})` only if the URI is spec-compliant + # 0.18.1 Improvements: diff --git a/ruma-identifiers/src/mxc_uri.rs b/ruma-identifiers/src/mxc_uri.rs index c90173ef..262586a4 100644 --- a/ruma-identifiers/src/mxc_uri.rs +++ b/ruma-identifiers/src/mxc_uri.rs @@ -1,75 +1,81 @@ -//! Matrix-spec compliant mxc:// urls. +//! A URI that should be a Matrix-spec compliant MXC URI. -use std::{convert::TryFrom, fmt, str::FromStr}; +use std::{convert::TryFrom, fmt, num::NonZeroU8}; use ruma_identifiers_validation::mxc_uri::validate; -use crate::{Error, ServerName, ServerNameBox}; +use crate::ServerName; -/// Matrix-spec compliant mxc:// urls. +/// A URI that should be a Matrix-spec compliant MXC URI. #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct MxcUri { - server_name: ServerNameBox, - media_id: Box, + full_uri: Box, + slash_idx: Option, } impl MxcUri { - /// Returns the media ID of this mxc://. - pub fn media_id(&self) -> &str { - &self.media_id + /// If this is a valid MXC URI, returns the media ID. + pub fn media_id(&self) -> Option<&str> { + self.parts().map(|(_, s)| s) } - /// Returns the server name of this mxc://. - pub fn server_name(&self) -> &ServerName { - &self.server_name + /// If this is a valid MXC URI, returns the server name. + pub fn server_name(&self) -> Option<&ServerName> { + self.parts().map(|(s, _)| s) } - fn mxc_uri_fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_fmt(format_args!("mxc://{}/{}", self.server_name, self.media_id)) + /// If this is a valid MXC URI, returns a `(server_name, media_id)` tuple. + pub fn parts(&self) -> Option<(&ServerName, &str)> { + self.slash_idx.map(|idx| { + ( + <&ServerName>::try_from(&self.full_uri[6..idx.get() as usize]).unwrap(), + &self.full_uri[idx.get() as usize + 1..], + ) + }) + } + + /// Returns if this is a spec-compliant MXC URI. + pub fn is_valid(&self) -> bool { + self.slash_idx.is_some() + } + + /// Create a string slice from this MXC URI. + pub fn as_str(&self) -> &str { + &self.full_uri } } impl fmt::Debug for MxcUri { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.mxc_uri_fmt(f) + f.write_str(&self.full_uri) } } impl fmt::Display for MxcUri { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.mxc_uri_fmt(f) + f.write_str(&self.full_uri) } } -fn try_from(uri: S) -> Result +fn from(uri: S) -> MxcUri where S: AsRef + Into>, { - let (media_id, server_name) = validate(uri.as_ref())?; - Ok(MxcUri { media_id: media_id.into(), server_name: ::try_from(server_name)? }) -} - -impl FromStr for MxcUri { - type Err = crate::Error; - - fn from_str(uri: &str) -> Result { - try_from(uri) + match validate(uri.as_ref()) { + Ok(idx) => MxcUri { full_uri: uri.into(), slash_idx: Some(idx) }, + Err(_) => MxcUri { full_uri: uri.into(), slash_idx: None }, } } -impl TryFrom<&str> for MxcUri { - type Error = crate::Error; - - fn try_from(s: &str) -> Result { - try_from(s) +impl From<&str> for MxcUri { + fn from(s: &str) -> Self { + from(s) } } -impl TryFrom for MxcUri { - type Error = crate::Error; - - fn try_from(s: String) -> Result { - try_from(s) +impl From for MxcUri { + fn from(s: String) -> Self { + from(s) } } @@ -79,10 +85,7 @@ impl<'de> serde::Deserialize<'de> for MxcUri { where D: serde::Deserializer<'de>, { - crate::deserialize_id( - deserializer, - "Content location represented as a Matrix Content (MXC) URI", - ) + String::deserialize(deserializer).map(Into::into) } } @@ -100,20 +103,61 @@ impl serde::Serialize for MxcUri { mod tests { use std::convert::TryFrom; + use crate::ServerName; + use super::MxcUri; #[test] fn parse_mxc_uri() { - assert!(::try_from("mxc://127.0.0.1/asd32asdfasdsd").is_ok()); + let mxc = MxcUri::from("mxc://127.0.0.1/asd32asdfasdsd"); + + assert!(mxc.is_valid()); + assert_eq!( + mxc.parts(), + Some(( + <&ServerName>::try_from("127.0.0.1").expect("Failed to create ServerName"), + "asd32asdfasdsd" + )) + ); } #[test] fn parse_mxc_uri_without_media_id() { - assert!(!::try_from("mxc://127.0.0.1").is_ok()); + let mxc = MxcUri::from("mxc://127.0.0.1"); + + assert!(!mxc.is_valid()); + assert_eq!(mxc.parts(), None); } #[test] fn parse_mxc_uri_without_protocol() { - assert!(!::try_from("127.0.0.1/asd32asdfasdsd").is_ok()); + assert!(!MxcUri::from("127.0.0.1/asd32asdfasdsd").is_valid()); + } + + #[cfg(feature = "serde")] + #[test] + fn serialize_mxc_uri() { + assert_eq!( + serde_json::to_string(&MxcUri::from("mxc://server/1234id")) + .expect("Failed to convert MxcUri to JSON."), + r#""mxc://server/1234id""# + ); + } + + #[cfg(feature = "serde")] + #[test] + fn deserialize_mxc_uri() { + let mxc = serde_json::from_str::(r#""mxc://server/1234id""#) + .expect("Failed to convert JSON to MxcUri"); + + assert_eq!(mxc.as_str(), "mxc://server/1234id"); + assert!(mxc.is_valid()); + assert_eq!( + mxc.parts(), + Some(( + <&ServerName>::try_from("server").expect("Failed to create ServerName"), + "1234id" + )) + ); } }