From fdbb144cd8e0f5776aac8108a8556f47e1b40acd Mon Sep 17 00:00:00 2001 From: zjean Date: Wed, 4 Mar 2026 15:51:47 +0100 Subject: [PATCH] fix(nextcloud): detect MIME type via magic bytes instead of trusting client header Nextcloud app uploads sent application/octet-stream as Content-Type, causing images to not be recognized. Now both WebDAV PUT and chunked upload paths call refine_content_type() which detects via magic bytes, then extension, then falls back to the client header. Also fixes update_file() which previously hardcoded application/octet-stream. Co-Authored-By: Claude Opus 4.6 --- src/application/ports/file_ports.rs | 7 +- .../services/file_upload_service.rs | 9 +- src/common/mime_detect.rs | 137 +++++++++++++++++- src/common/stubs.rs | 7 +- src/interfaces/nextcloud/uploads_handler.rs | 9 +- src/interfaces/nextcloud/webdav_handler.rs | 9 +- 6 files changed, 163 insertions(+), 15 deletions(-) diff --git a/src/application/ports/file_ports.rs b/src/application/ports/file_ports.rs index 32ea3355..d55ef437 100644 --- a/src/application/ports/file_ports.rs +++ b/src/application/ports/file_ports.rs @@ -63,7 +63,12 @@ pub trait FileUploadUseCase: Send + Sync + 'static { ) -> Result; /// Updates the content of an existing file (for WebDAV) - async fn update_file(&self, path: &str, content: &[u8]) -> Result<(), DomainError>; + async fn update_file( + &self, + path: &str, + content: &[u8], + content_type: &str, + ) -> Result<(), DomainError>; /// Streaming update — spools body to a temp file with incremental hash, /// then atomically replaces the file content via dedup store. diff --git a/src/application/services/file_upload_service.rs b/src/application/services/file_upload_service.rs index 9dd0b8ca..45a3d187 100644 --- a/src/application/services/file_upload_service.rs +++ b/src/application/services/file_upload_service.rs @@ -222,7 +222,12 @@ impl FileUploadUseCase for FileUploadService { /// /// Spools the in-memory `&[u8]` to a temp file with hash-on-write, /// then delegates to the streaming update/create path. - async fn update_file(&self, path: &str, content: &[u8]) -> Result<(), DomainError> { + async fn update_file( + &self, + path: &str, + content: &[u8], + content_type: &str, + ) -> Result<(), DomainError> { // Spool to temp file + hash let temp = tempfile::NamedTempFile::new() .map_err(|e| DomainError::internal_error("FileUpload", format!("temp file: {e}")))?; @@ -235,7 +240,7 @@ impl FileUploadUseCase for FileUploadService { path, temp.path(), content.len() as u64, - "application/octet-stream", + content_type, Some(hash), ) .await diff --git a/src/common/mime_detect.rs b/src/common/mime_detect.rs index 25614a77..dd4399e7 100644 --- a/src/common/mime_detect.rs +++ b/src/common/mime_detect.rs @@ -9,10 +9,16 @@ //! Performance: < 1µs for the `infer` check (reads only header bytes, no allocation). use std::path::Path; +use tokio::io::AsyncReadExt; /// Maximum bytes to read for magic-byte detection. const MAGIC_BYTES_LEN: usize = 8192; +/// Extract the filename component from a `/`-separated path. +pub fn filename_from_path(path: &str) -> &str { + path.rsplit('/').next().unwrap_or(path) +} + /// Refine a claimed MIME type using magic bytes and filename extension. /// /// This is a synchronous function — the caller should already have the first @@ -62,11 +68,12 @@ pub async fn refine_content_type_from_file( return claimed.to_string(); } - // Read first bytes for magic detection - match tokio::fs::read(temp_path).await { - Ok(full) => { - let len = full.len().min(MAGIC_BYTES_LEN); - refine_content_type(&full[..len], filename, claimed) + // Read only the first bytes needed for magic detection (not the whole file). + match tokio::fs::File::open(temp_path).await { + Ok(mut file) => { + let mut buf = vec![0u8; MAGIC_BYTES_LEN]; + let n = file.read(&mut buf).await.unwrap_or(0); + refine_content_type(&buf[..n], filename, claimed) } Err(e) => { tracing::warn!( @@ -83,3 +90,123 @@ pub async fn refine_content_type_from_file( } } } + +#[cfg(test)] +mod tests { + use super::*; + use std::io::Write; + + // ── refine_content_type (sync) ────────────────────────────── + + #[test] + fn specific_claimed_type_is_trusted() { + let result = refine_content_type(b"garbage", "file.txt", "image/png"); + assert_eq!(result, "image/png"); + } + + #[test] + fn octet_stream_triggers_magic_detection_png() { + // PNG magic bytes + let png = b"\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR"; + let result = refine_content_type(png, "noext", "application/octet-stream"); + assert_eq!(result, "image/png"); + } + + #[test] + fn octet_stream_triggers_magic_detection_jpeg() { + let jpeg = b"\xff\xd8\xff\xe0\x00\x10JFIF"; + let result = refine_content_type(jpeg, "noext", "application/octet-stream"); + assert_eq!(result, "image/jpeg"); + } + + #[test] + fn binary_octet_stream_also_triggers_detection() { + let jpeg = b"\xff\xd8\xff\xe0\x00\x10JFIF"; + let result = refine_content_type(jpeg, "noext", "binary/octet-stream"); + assert_eq!(result, "image/jpeg"); + } + + #[test] + fn extension_fallback_when_no_magic_match() { + let result = refine_content_type(b"plain text", "style.css", "application/octet-stream"); + assert_eq!(result, "text/css"); + } + + #[test] + fn falls_back_to_claimed_when_nothing_matches() { + let result = + refine_content_type(b"unknown stuff", "noext", "application/octet-stream"); + assert_eq!(result, "application/octet-stream"); + } + + #[test] + fn empty_claimed_triggers_detection() { + let png = b"\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR"; + let result = refine_content_type(png, "photo.png", ""); + assert_eq!(result, "image/png"); + } + + // ── refine_content_type_from_file (async) ─────────────────── + + #[tokio::test] + async fn from_file_detects_png() { + let mut tmp = tempfile::NamedTempFile::new().unwrap(); + let png = b"\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR"; + tmp.write_all(png).unwrap(); + tmp.flush().unwrap(); + + let result = + refine_content_type_from_file(tmp.path(), "photo", "application/octet-stream").await; + assert_eq!(result, "image/png"); + } + + #[tokio::test] + async fn from_file_falls_back_to_extension() { + let mut tmp = tempfile::NamedTempFile::new().unwrap(); + tmp.write_all(b"not magic").unwrap(); + tmp.flush().unwrap(); + + let result = + refine_content_type_from_file(tmp.path(), "doc.css", "application/octet-stream").await; + assert_eq!(result, "text/css"); + } + + #[tokio::test] + async fn from_file_trusts_specific_claimed() { + let result = refine_content_type_from_file( + Path::new("/nonexistent"), + "file", + "image/webp", + ) + .await; + assert_eq!(result, "image/webp"); + } + + #[tokio::test] + async fn from_file_missing_file_falls_back_to_extension() { + let result = refine_content_type_from_file( + Path::new("/nonexistent/file"), + "photo.jpg", + "application/octet-stream", + ) + .await; + assert_eq!(result, "image/jpeg"); + } + + // ── filename_from_path ────────────────────────────────────── + + #[test] + fn extracts_filename_from_deep_path() { + assert_eq!(filename_from_path("a/b/c/photo.jpg"), "photo.jpg"); + } + + #[test] + fn returns_input_when_no_slash() { + assert_eq!(filename_from_path("photo.jpg"), "photo.jpg"); + } + + #[test] + fn handles_trailing_slash() { + assert_eq!(filename_from_path("a/b/"), ""); + } +} diff --git a/src/common/stubs.rs b/src/common/stubs.rs index e3a137c1..2b3fe0eb 100644 --- a/src/common/stubs.rs +++ b/src/common/stubs.rs @@ -467,7 +467,12 @@ impl FileUploadUseCase for StubFileUploadUseCase { Ok(FileDto::default()) } - async fn update_file(&self, _path: &str, _content: &[u8]) -> Result<(), DomainError> { + async fn update_file( + &self, + _path: &str, + _content: &[u8], + _content_type: &str, + ) -> Result<(), DomainError> { Ok(()) } diff --git a/src/interfaces/nextcloud/uploads_handler.rs b/src/interfaces/nextcloud/uploads_handler.rs index 1da981c2..79edf94a 100644 --- a/src/interfaces/nextcloud/uploads_handler.rs +++ b/src/interfaces/nextcloud/uploads_handler.rs @@ -7,6 +7,7 @@ use std::sync::Arc; use crate::application::ports::file_ports::{FileRetrievalUseCase, FileUploadUseCase}; use crate::common::di::AppState; +use crate::common::mime_detect::{filename_from_path, refine_content_type_from_file}; use crate::interfaces::errors::AppError; use crate::interfaces::middleware::auth::CurrentUser; @@ -135,10 +136,10 @@ async fn handle_assemble( dest_subpath.trim_matches('/') ); - // Detect content type from file extension. - let content_type = mime_guess::from_path(&dest_subpath) - .first_or_octet_stream() - .to_string(); + // Detect content type via magic bytes + extension fallback. + let filename = filename_from_path(&dest_subpath); + let content_type = + refine_content_type_from_file(&temp_path, filename, "application/octet-stream").await; // Check if file exists (update vs create). let existing = file_service.get_file_by_path(&internal_path).await; diff --git a/src/interfaces/nextcloud/webdav_handler.rs b/src/interfaces/nextcloud/webdav_handler.rs index b2b06eaf..2545d464 100644 --- a/src/interfaces/nextcloud/webdav_handler.rs +++ b/src/interfaces/nextcloud/webdav_handler.rs @@ -20,6 +20,7 @@ use crate::application::ports::file_ports::{ use crate::application::ports::inbound::FolderUseCase; use crate::application::ports::trash_ports::TrashUseCase; use crate::common::di::AppState; +use crate::common::mime_detect::{filename_from_path, refine_content_type}; use crate::interfaces::errors::AppError; use crate::interfaces::middleware::auth::CurrentUser; @@ -472,7 +473,7 @@ async fn handle_put( let file_service = &state.applications.file_retrieval_service; let upload_service = &state.applications.file_upload_service; - let content_type = req + let claimed_type = req .headers() .get(header::CONTENT_TYPE) .and_then(|v| v.to_str().ok()) @@ -490,13 +491,17 @@ async fn handle_put( .await .map_err(|e| AppError::bad_request(format!("Failed to read body: {}", e)))?; + // Detect real MIME type via magic bytes + extension, falling back to client header. + let filename = filename_from_path(subpath); + let content_type = refine_content_type(&body_bytes, filename, &claimed_type); + // Check if the file already exists (update vs create). let existing = file_service.get_file_by_path(&internal_path).await; if existing.is_ok() { // Update existing file. upload_service - .update_file(&internal_path, &body_bytes) + .update_file(&internal_path, &body_bytes, &content_type) .await .map_err(|e| AppError::internal_error(format!("Failed to update file: {}", e)))?;