collective
Browse / github.com/storytold/artcraft / patch
fix(url-utils): extract extensions only from file path segments
fix(url-utils): extract extensions only from file path segments ## Summary Extract suffixes only from the final file path segment of hierarchical URLs. Directory URLs, opaque URLs and empty suffixes must not manufacture a filename extension. Preserve existing media suffix filtering. Route the deprecated downloader through the same corrected extractor: it is still called by completed Sora and Fal generation handlers. This follow-up completes the same underlying fix within this PR. ## Verification - Original extractor: three regression failures, 32 passing controls. - Legacy downloader follow-up: two regression failures, 37 passing controls before the am
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- rudycelekli (imported from pull request #2070, open)
- Base
b5614ad333839649d8f4e779d955b89b3c22f376- Size
- +84 −8 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
f1aaf17587e900bd7d6bcf92
How it went
No other operator has reported using it yet.
Diff
raw · crates/lib/url_utils/src/download_extension/extract_download_extension_from_url.rs crates/lib/url_utils/src/extension/extract_extension_from_url.rs
@@ -1,4 +1,5 @@ use crate::download_extension::DownloadExtension;+use crate::extension::extract_extension_from_url::{extract_extension_from_url, ExtractExtensions}; use url::Url; /// This should be used for naming files on the local file system based on their URL.@@ -11,11 +12,8 @@ pub fn extract_download_extension_from_url_str(url: &str) -> Option<DownloadExte /// This should be used for naming files on the local file system based on their URL. #[deprecated(note = "Use `extract_extension_from_url` in `url_utils::extension::extract_extension_from_url` instead")] pub fn extract_download_extension_from_url(url: &Url) -> Option<DownloadExtension> {- let path = url.path();- let extension = std::path::Path::new(path)- .extension()- .and_then(|ext| ext.to_str())?;- DownloadExtension::try_from_str(extension)+ let extension = extract_extension_from_url(url, &ExtractExtensions::All)?;+ DownloadExtension::try_from_str(extension.without_period()) } #[cfg(test)]@@ -30,6 +28,42 @@ mod tests { assert_eq!(maybe_extension, Some(DownloadExtension::Jpg)); } + mod final_path_segment {+ use super::*;++ #[test]+ fn directory_path_does_not_infer_an_extension() {+ assert_eq!(+ extract_download_extension_from_url_str("https://example.com/assets.png/"),+ None,+ );+ }++ #[test]+ fn opaque_url_does_not_infer_an_extension() {+ assert_eq!(+ extract_download_extension_from_url_str("data:text/plain,file.png"),+ None,+ );+ }++ #[test]+ fn mixed_case_supported_filename_with_query_is_preserved() {+ assert_eq!(+ extract_download_extension_from_url_str("https://example.com/clip.MP4?token=abc"),+ Some(DownloadExtension::Mp4),+ );+ }++ #[test]+ fn unknown_extension_remains_rejected() {+ assert_eq!(+ extract_download_extension_from_url_str("https://example.com/file.xyz"),+ None,+ );+ }+ }+ #[test] fn difficult_url() { let url = "https://videos.youtube.com/foo/bar/files/00000000-f19a-1275-a092-caa60b38f8d2%2Fraw?se=2025-11-21T22%3A26%3A53Z&sp=r&sv=2024-08-09&sr=b&skoid=8cfff87a-01fc-27c1-9091-32999dcd1389&sktid=aa8ccaf6-e6da-484e-ab11-9c7496c21c53&skt=2025-11-17T21%3A09%3A08Z&ske=2025-10-23T21%3A14%3A08Z&sks=b&skv=2024-08-04&sig=T2U6%2Brw9WJdJagBkCL4/GlFr72KhZGP3QPlobIO7JlU%3D&ac=oaisdsorprnorthcentralus";@@ -61,10 +61,11 @@ pub fn extract_extension_from_url_str(url: &str, accept: &ExtractExtensions) -> /// Extracts a file extension from a parsed `Url`, filtered by `accept`. pub fn extract_extension_from_url(url: &Url, accept: &ExtractExtensions) -> Option<Extension> {- let path = url.path();- let raw_ext = std::path::Path::new(path)+ let filename = url.path_segments()?.next_back()?;+ let raw_ext = std::path::Path::new(filename) .extension()- .and_then(|ext| ext.to_str())?;+ .and_then(|ext| ext.to_str())+ .filter(|ext| !ext.is_empty())?; let lower = raw_ext.to_lowercase(); let candidate = Extension::new(&lower);@@ -104,6 +105,47 @@ fn slice_contains(extensions: &[Extension], candidate: &Extension) -> bool { mod tests { use super::*; + mod final_path_segment {+ use super::*;++ #[test]+ fn directory_url_has_no_file_extension() {+ for accept in [ExtractExtensions::All, ExtractExtensions::KnownImage] {+ let ext = extract_extension_from_url_str("https://example.com/assets.png/", &accept);+ assert_eq!(ext.map(|ext| ext.without_period().to_string()), None);+ }+ }++ #[test]+ fn empty_extension_is_not_a_filename_suffix() {+ let ext = extract_extension_from_url_str(+ "https://example.com/image.", &ExtractExtensions::All,+ );+ assert_eq!(ext.map(|ext| ext.without_period().to_string()), None);+ }++ #[test]+ fn opaque_url_is_not_a_file_path() {+ let ext = extract_extension_from_url_str(+ "data:application/example.png", &ExtractExtensions::All,+ );+ assert_eq!(ext.map(|ext| ext.without_period().to_string()), None);+ }++ #[test]+ fn parent_directory_extension_does_not_replace_the_file_extension() {+ let ext = extract_extension_from_url_str(+ "https://example.com/assets.png/reference.MP4?sig=value#preview",+ &ExtractExtensions::KnownVideo,+ );+ assert_eq!(ext.unwrap().without_period(), "mp4");+ let ext = extract_extension_from_url_str(+ "https://example.com/assets.png/reference", &ExtractExtensions::All,+ );+ assert_eq!(ext.map(|ext| ext.without_period().to_string()), None);+ }+ }+ #[test] fn all_extracts_any_extension() { let ext = extract_extension_from_url_str("https://example.com/file.png", &ExtractExtensions::All);