fix(media): Preserve remote preview dimensions without decoding images

src/service/media/mod.rs

@@ -36,8 +36,6 @@ pub struct FileMeta {
 
 pub struct Service {
 	url_preview_mutex: MutexMap<String, ()>,
-	#[cfg(feature = "url_preview")]
-	url_preview_image_semaphore: Arc<tokio::sync::Semaphore>,
 	pub(super) db: Data,
 	services: Services,
 }
@@ -64,8 +62,6 @@ impl crate::Service for Service {
 	fn build(args: crate::Args<'_>) -> Result<Arc<Self>> {
 		Ok(Arc::new(Self {
 			url_preview_mutex: MutexMap::new(),
-			#[cfg(feature = "url_preview")]
-			url_preview_image_semaphore: Arc::new(tokio::sync::Semaphore::new(2)),
 			db: Data::new(args.db),
 			services: Services {
 				server: args.server.clone(),

src/service/media/preview.rs

@@ -144,12 +144,6 @@ pub async fn download_image(
 			return Err!(Request(Unknown("Image download returned HTTP {}", response.status())));
 		}
 
-		let permit = self
-			.url_preview_image_semaphore
-			.clone()
-			.acquire_owned()
-			.await
-			.map_err(|error| err!(Request(Unknown("Image decoder unavailable: {error}"))))?;
 		let image = response
 			.limit_read(
 				self.services
@@ -161,31 +155,30 @@ pub async fn download_image(
 			)
 			.await?;
 
+		let needs_dimensions =
+			preview_data.image_width.is_none() || preview_data.image_height.is_none();
 		let (image, content_type, width, height) = self
 			.services
 			.server
 			.runtime()
 			.spawn_blocking(move || -> Result<_> {
-				// Keep the slot until decoding finishes, even if the request is
-				// cancelled.
-				let _permit = permit;
-				let mut reader =
+				let reader =
 					ImageReader::new(std::io::Cursor::new(&image)).with_guessed_format()?;
-				let mut limits = image::Limits::default();
-				limits.max_image_width = Some(8192);
-				limits.max_image_height = Some(8192);
-				limits.max_alloc = Some(64 * 1024 * 1024);
-				reader.limits(limits);
 				let content_type = reader
 					.format()
 					.ok_or_else(|| {
 						err!(Request(Unknown("Downloaded file is not a supported image")))
 					})?
 					.to_mime_type();
-				let decoded = reader.decode().map_err(|error| {
-					err!(Request(Unknown("Failed to decode preview image: {error}")))
-				})?;
-				Ok((image, content_type, decoded.width(), decoded.height()))
+				let (width, height) = if needs_dimensions {
+					let (width, height) = reader.into_dimensions().map_err(|error| {
+						err!(Request(Unknown("Failed to read preview image dimensions: {error}")))
+					})?;
+					(Some(width), Some(height))
+				} else {
+					(None, None)
+				};
+				Ok((image, content_type, width, height))
 			})
 			.await??;
 
@@ -198,8 +191,8 @@ pub async fn download_image(
 			.await?;
 
 		preview_data.image = Some(mxc.to_string());
-		preview_data.image_width = Some(width);
-		preview_data.image_height = Some(height);
+		preview_data.image_width = preview_data.image_width.or(width);
+		preview_data.image_height = preview_data.image_height.or(height);
 
 		Ok(preview_data)
 	}

src/service/media/tests.rs

@@ -108,8 +108,6 @@ async fn preview_images_store_mime_type_and_dimensions() {
 						&server.url,
 						Some(UrlPreviewData {
 							title: Some("Example preview".into()),
-							image_width: Some(99),
-							image_height: Some(99),
 							..UrlPreviewData::default()
 						}),
 					)
@@ -126,6 +124,34 @@ async fn preview_images_store_mime_type_and_dimensions() {
 		}
 	}
 
+	#[tokio::test]
+	async fn preview_images_preserve_remote_dimensions() {
+		let (_directory, services) = services().await;
+		let server = ImageServer::new(
+			&services,
+			StatusCode::OK,
+			Some("image/png"),
+			image(ImageFormat::Png),
+		)
+		.await;
+		for (width, height) in [(Some(99), Some(98)), (Some(99), None), (None, Some(98))] {
+			let preview = services
+				.media
+				.download_image(
+					&server.url,
+					Some(UrlPreviewData {
+						image_width: width,
+						image_height: height,
+						..UrlPreviewData::default()
+					}),
+				)
+				.await
+				.unwrap();
+			assert_eq!(preview.image_width, width.or(Some(2)));
+			assert_eq!(preview.image_height, height.or(Some(3)));
+		}
+	}
+
 	#[tokio::test]
 	async fn preview_images_reject_failed_or_invalid_downloads_before_storage() {
 		let (_directory, services) = services().await;
@@ -138,7 +164,6 @@ async fn preview_images_reject_failed_or_invalid_downloads_before_storage() {
 			(StatusCode::OK, b"<html>Access denied</html>".to_vec()),
 			(StatusCode::OK, Vec::new()),
 			(StatusCode::OK, png[..33].to_vec()),
-			(StatusCode::OK, png[..png.len() - 20].to_vec()),
 			(StatusCode::OK, vec![0; 4097]),
 		];
 		for (status, bytes) in cases {
@@ -156,22 +181,34 @@ async fn preview_images_reject_failed_or_invalid_downloads_before_storage() {
 	}
 
 	#[tokio::test]
-	async fn preview_images_reject_excessive_decoded_dimensions_and_memory() {
+	async fn preview_images_read_dimensions_without_decoding_pixels() {
 		let (_directory, services) = services().await;
+		let mut png = image(ImageFormat::Png);
+		png.truncate(png.len() - 20);
+		assert!(image::load_from_memory(&png).is_err());
+		let server = ImageServer::new(&services, StatusCode::OK, Some("image/png"), png).await;
+		let preview = services
+			.media
+			.download_image(&server.url, None)
+			.await
+			.unwrap();
+		assert_eq!((preview.image_width, preview.image_height), (Some(2), Some(3)));
+
 		for (width, height) in [(8193_u16, 1_u16), (5000, 5000)] {
 			let mut bytes = image(ImageFormat::Gif);
 			bytes[6..8].copy_from_slice(&width.to_le_bytes());
 			bytes[8..10].copy_from_slice(&height.to_le_bytes());
 			let server =
 				ImageServer::new(&services, StatusCode::OK, Some("image/gif"), bytes).await;
-			assert!(
-				services
-					.media
-					.download_image(&server.url, None)
-					.await
-					.is_err()
+			let preview = services
+				.media
+				.download_image(&server.url, None)
+				.await
+				.unwrap();
+			assert_eq!(
+				(preview.image_width, preview.image_height),
+				(Some(u32::from(width)), Some(u32::from(height))),
 			);
-			assert!(services.media.get_all_mxcs().await.unwrap().is_empty());
 		}
 	}
 }