fix: Correct parsing of relative/absolute paths in embed images & fix ImageSize being overwritten (#930)

Signed-off-by: Pecacheu <3608878+Pecacheu@users.noreply.github.com>

crates/services/january/src/requests.rs

@@ -202,6 +202,7 @@ impl Request {
     pub async fn fetch_image_metadata(
         url: &str,
         request: Option<Request>,
+        size: ImageSize,
     ) -> Result<Option<Image>> {
         if let Some(hit) = EMBED_CACHE.get(url).await {
             match hit {
@@ -228,7 +229,7 @@ impl Request {
                     url: url.to_owned(),
                     width,
                     height,
-                    size: ImageSize::Large,
+                    size,
                 }))
             } else {
                 Ok(None)
@@ -332,10 +333,12 @@ impl Request {
                         .map(Embed::Website)
                         .unwrap_or_default()
                 }
-                (mime::IMAGE, _) => Request::fetch_image_metadata(&url, Some(request))
+                (mime::IMAGE, _) => {
-                    .await
+                    Request::fetch_image_metadata(&url, Some(request), ImageSize::Large)
-                    .map(|res| res.map(Embed::Image).unwrap_or_default())
+                        .await
-                    .unwrap_or_default(),
+                        .map(|res| res.map(Embed::Image).unwrap_or_default())
+                        .unwrap_or_default()
+                }
                 (mime::VIDEO, _) => Request::fetch_video_metadata(&url, Some(request))
                     .await
                     .map(|res| res.map(Embed::Video).unwrap_or_default())

crates/services/january/src/website_embed.rs

@@ -6,6 +6,24 @@ use revolt_models::v0::{
     BandcampType, Image, ImageSize, LightspeedType, Special, TwitchType, Video, WebsiteMetadata,
 };
 use scraper::{Html, Selector};
+use url::Url;
+
+/// Convert all URLs to absolute form
+fn url_to_absolute(page_url: &str, url: String) -> String {
+    if url.starts_with("http") && url.contains("://") {
+        //External
+        return url;
+    } else if url.starts_with('/') {
+        //Absolute
+        let page = Url::parse(page_url);
+        if page.is_err() {
+            return "".to_string();
+        }
+        return format!("{}{}", &page.unwrap().origin().unicode_serialization(), url);
+    }
+    //Relative
+    format!("{}/{}", &page_url.trim_end_matches('/'), url)
+}
 
 /// Create website metadata from URL and document
 pub async fn create_website_embed(original_url: &str, document: &str) -> Option<WebsiteMetadata> {
@@ -61,14 +79,7 @@ pub async fn create_website_embed(original_url: &str, document: &str) -> Option<
             .or_else(|| meta.remove("twitter:image"))
             .or_else(|| meta.remove("twitter:image:src"))
             .map(|s| s.trim().to_owned())
-            .map(|mut url| {
+            .map(|url| {
-                // If relative URL, prepend root URL. Also if root URL ends with a slash, remove it.
-                if let Some(ch) = url.chars().next() {
-                    if ch == '/' {
-                        url = format!("{}{}", &original_url.trim_end_matches('/'), url);
-                    }
-                }
-
                 let mut size = ImageSize::Preview;
                 if let Some(card) = meta.remove("twitter:card") {
                     if &card == "summary_large_image" {
@@ -77,7 +88,7 @@ pub async fn create_website_embed(original_url: &str, document: &str) -> Option<
                 }
 
                 Image {
-                    url: url.to_owned(),
+                    url: url_to_absolute(original_url, url),
                     width: meta
                         .remove("og:image:width")
                         .unwrap_or_default()
@@ -96,42 +107,24 @@ pub async fn create_website_embed(original_url: &str, document: &str) -> Option<
             .or_else(|| meta.remove("og:video:url"))
             .or_else(|| meta.remove("og:video:secure_url"))
             .map(|s| s.trim().to_owned())
-            .map(|mut url| {
+            .map(|url| Video {
-                // If relative URL, prepend root URL. Also if root URL ends with a slash, remove it.
+                url: url_to_absolute(original_url, url),
-                if let Some(ch) = url.chars().next() {
+                width: meta
-                    if ch == '/' {
+                    .remove("og:video:width")
-                        url = format!("{}{}", &original_url.trim_end_matches('/'), url);
+                    .unwrap_or_default()
-                    }
+                    .parse()
-                }
+                    .unwrap_or(0),
-
+                height: meta
-                Video {
+                    .remove("og:video:height")
-                    url: url.to_owned(),
+                    .unwrap_or_default()
-                    width: meta
+                    .parse()
-                        .remove("og:video:width")
+                    .unwrap_or(0),
-                        .unwrap_or_default()
-                        .parse()
-                        .unwrap_or(0),
-                    height: meta
-                        .remove("og:video:height")
-                        .unwrap_or_default()
-                        .parse()
-                        .unwrap_or(0),
-                }
             }),
         icon_url: link
             .remove("apple-touch-icon")
             .or_else(|| link.remove("icon"))
             .map(|s| s.trim().to_owned())
-            .map(|mut v| {
+            .map(|url| url_to_absolute(original_url, url)),
-                // If relative URL, prepend root URL.
-                if let Some(ch) = v.chars().next() {
-                    if ch == '/' {
-                        v = format!("{}{}", &original_url.trim_end_matches('/'), v);
-                    }
-                }
-
-                v
-            }),
         colour: meta.remove("theme-color").map(|s| s.trim().to_owned()),
         site_name: meta.remove("og:site_name").map(|s| s.trim().to_owned()),
         url: meta
@@ -147,7 +140,9 @@ pub async fn create_website_embed(original_url: &str, document: &str) -> Option<
     // fetch video size if missing
     if metadata.special.is_none() {
         if let Some(Video { width, height, url }) = &metadata.video {
-            if width == &0 || height == &0 {
+            if url.is_empty() {
+                metadata.video.take();
+            } else if width == &0 || height == &0 {
                 metadata.video =
                     match crate::requests::Request::fetch_video_metadata(url, None).await {
                         Ok(Some(video)) => Some(video),
@@ -168,9 +163,12 @@ pub async fn create_website_embed(original_url: &str, document: &str) -> Option<
             width, height, url, ..
         }) = &metadata.image
         {
-            if width == &0 || height == &0 {
+            if url.is_empty() {
+                metadata.image.take();
+            } else if width == &0 || height == &0 {
+                let size = metadata.image.as_ref().unwrap().size.clone();
                 metadata.image =
-                    match crate::requests::Request::fetch_image_metadata(url, None).await {
+                    match crate::requests::Request::fetch_image_metadata(url, None, size).await {
                         Ok(Some(image)) => Some(image),
                         _ => None,
                     }