identifiers: Make MxcUri less strict

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 {
+    pub fn from_url(url: &'a MxcUri) -> Result<Self, Error> {
-        Self { media_id: url.media_id(), server_name: url.server_name(), allow_remote: true }
+        if let Some((server_name, media_id)) = url.parts() {
+            Ok(Self { media_id, server_name, allow_remote: true })
+        } else {
+            Err(Error::InvalidMxcUri)
+        }
     }
 }
 

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 {
+    pub fn from_url(url: &'a MxcUri, filename: &'a str) -> Result<Self, Error> {
-        Self {
+        if let Some((server_name, media_id)) = url.parts() {
-            media_id: url.media_id(),
+            Ok(Self { media_id, server_name, filename, allow_remote: true })
-            server_name: url.server_name(),
+        } else {
-            filename,
+            Err(Error::InvalidMxcUri)
-            allow_remote: true,
         }
     }
 }

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 {
+    pub fn from_url(url: &'a MxcUri, width: UInt, height: UInt) -> Result<Self, Error> {
-        Self {
+        if let Some((server_name, media_id)) = url.parts() {
-            media_id: url.media_id(),
+            Ok(Self { media_id, server_name, method: None, width, height, allow_remote: true })
-            server_name: url.server_name(),
+        } else {
-            method: None,
+            Err(Error::InvalidMxcUri)
-            width,
-            height,
-            allow_remote: true,
         }
     }
 }

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<NonZeroU8, Error> {
     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)
     }

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:

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,
+    full_uri: Box<str>,
-    media_id: Box<str>,
+    slash_idx: Option<NonZeroU8>,
 }
 
 impl MxcUri {
-    /// Returns the media ID of this mxc://.
+    /// If this is a valid MXC URI, returns the media ID.
-    pub fn media_id(&self) -> &str {
+    pub fn media_id(&self) -> Option<&str> {
-        &self.media_id
+        self.parts().map(|(_, s)| s)
     }
 
-    /// Returns the server name of this mxc://.
+    /// If this is a valid MXC URI, returns the server name.
-    pub fn server_name(&self) -> &ServerName {
+    pub fn server_name(&self) -> Option<&ServerName> {
-        &self.server_name
+        self.parts().map(|(s, _)| s)
     }
 
-    fn mxc_uri_fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+    /// If this is a valid MXC URI, returns a `(server_name, media_id)` tuple.
-        f.write_fmt(format_args!("mxc://{}/{}", self.server_name, self.media_id))
+    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<S>(uri: S) -> Result<MxcUri, Error>
+fn from<S>(uri: S) -> MxcUri
 where
     S: AsRef<str> + Into<Box<str>>,
 {
-    let (media_id, server_name) = validate(uri.as_ref())?;
+    match validate(uri.as_ref()) {
-    Ok(MxcUri { media_id: media_id.into(), server_name: <ServerNameBox>::try_from(server_name)? })
+        Ok(idx) => MxcUri { full_uri: uri.into(), slash_idx: Some(idx) },
-}
+        Err(_) => MxcUri { full_uri: uri.into(), slash_idx: None },
-
-impl FromStr for MxcUri {
-    type Err = crate::Error;
-
-    fn from_str(uri: &str) -> Result<Self, Self::Err> {
-        try_from(uri)
     }
 }
 
-impl TryFrom<&str> for MxcUri {
+impl From<&str> for MxcUri {
-    type Error = crate::Error;
+    fn from(s: &str) -> Self {
-
+        from(s)
-    fn try_from(s: &str) -> Result<Self, Self::Error> {
-        try_from(s)
     }
 }
 
-impl TryFrom<String> for MxcUri {
+impl From<String> for MxcUri {
-    type Error = crate::Error;
+    fn from(s: String) -> Self {
-
+        from(s)
-    fn try_from(s: String) -> Result<Self, Self::Error> {
-        try_from(s)
     }
 }
 
@@ -79,10 +85,7 @@ impl<'de> serde::Deserialize<'de> for MxcUri {
     where
         D: serde::Deserializer<'de>,
     {
-        crate::deserialize_id(
+        String::deserialize(deserializer).map(Into::into)
-            deserializer,
-            "Content location represented as a Matrix Content (MXC) URI",
-        )
     }
 }
 
@@ -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!(<MxcUri>::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!(!<MxcUri>::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!(!<MxcUri>::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::<MxcUri>(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"
+            ))
+        );
     }
 }