From d30027ac5b13d517645492055100ab76632a62f9 Mon Sep 17 00:00:00 2001 From: Douman Date: Mon, 25 Mar 2019 23:02:37 +0300 Subject: [PATCH] Remove StaticFilesConfig (#731) * Remove StaticFilesConfig * Applying comments * Impl Clone for Files --- actix-files/Cargo.toml | 1 + actix-files/src/config.rs | 67 ------------ actix-files/src/lib.rs | 221 ++++++++++++++++++-------------------- actix-files/src/named.rs | 126 ++++++++++------------ 4 files changed, 162 insertions(+), 253 deletions(-) delete mode 100644 actix-files/src/config.rs diff --git a/actix-files/Cargo.toml b/actix-files/Cargo.toml index aa93ac225..65faa5e8c 100644 --- a/actix-files/Cargo.toml +++ b/actix-files/Cargo.toml @@ -22,6 +22,7 @@ actix-web = { path=".." } actix-http = { git = "https://github.com/actix/actix-http.git" } actix-service = "0.3.3" +bitflags = "1" bytes = "0.4" futures = "0.1" derive_more = "0.14" diff --git a/actix-files/src/config.rs b/actix-files/src/config.rs deleted file mode 100644 index 7ad65ae79..000000000 --- a/actix-files/src/config.rs +++ /dev/null @@ -1,67 +0,0 @@ -use actix_web::http::{header::DispositionType, Method}; -use mime; - -/// Describes `StaticFiles` configiration -/// -/// To configure actix's static resources you need -/// to define own configiration type and implement any method -/// you wish to customize. -/// As trait implements reasonable defaults for Actix. -/// -/// ## Example -/// -/// ```rust -/// use actix_web::http::header::DispositionType; -/// use actix_files::{StaticFileConfig, NamedFile}; -/// -/// #[derive(Default)] -/// struct MyConfig; -/// -/// impl StaticFileConfig for MyConfig { -/// fn content_disposition_map(typ: mime::Name) -> DispositionType { -/// DispositionType::Attachment -/// } -/// } -/// -/// let file = NamedFile::open_with_config("foo.txt", MyConfig); -/// ``` -pub trait StaticFileConfig: Default { - /// Describes mapping for mime type to content disposition header - /// - /// By default `IMAGE`, `TEXT` and `VIDEO` are mapped to Inline. - /// Others are mapped to Attachment - fn content_disposition_map(typ: mime::Name) -> DispositionType { - match typ { - mime::IMAGE | mime::TEXT | mime::VIDEO => DispositionType::Inline, - _ => DispositionType::Attachment, - } - } - - /// Describes whether Actix should attempt to calculate `ETag` - /// - /// Defaults to `true` - fn is_use_etag() -> bool { - true - } - - /// Describes whether Actix should use last modified date of file. - /// - /// Defaults to `true` - fn is_use_last_modifier() -> bool { - true - } - - /// Describes allowed methods to access static resources. - /// - /// By default all methods are allowed - fn is_method_allowed(_method: &Method) -> bool { - true - } -} - -/// Default content disposition as described in -/// [StaticFileConfig](trait.StaticFileConfig.html) -#[derive(Default)] -pub struct DefaultConfig; - -impl StaticFileConfig for DefaultConfig {} diff --git a/actix-files/src/lib.rs b/actix-files/src/lib.rs index b92400099..31ff4cdaf 100644 --- a/actix-files/src/lib.rs +++ b/actix-files/src/lib.rs @@ -3,7 +3,6 @@ use std::cell::RefCell; use std::fmt::Write; use std::fs::{DirEntry, File}; use std::io::{Read, Seek}; -use std::marker::PhantomData; use std::path::{Path, PathBuf}; use std::rc::Rc; use std::{cmp, io}; @@ -22,15 +21,14 @@ use actix_web::dev::{ }; use actix_web::error::{BlockingError, Error, ErrorInternalServerError}; use actix_web::{web, FromRequest, HttpRequest, HttpResponse, Responder}; +use actix_web::http::header::{DispositionType}; use futures::future::{ok, FutureResult}; -mod config; mod error; mod named; mod range; use self::error::{FilesError, UriSegmentError}; -pub use crate::config::{DefaultConfig, StaticFileConfig}; pub use crate::named::NamedFile; pub use crate::range::HttpRange; @@ -211,6 +209,8 @@ fn directory_listing( )) } +type MimeOverride = Fn(&mime::Name) -> DispositionType; + /// Static files handling /// /// `Files` service must be registered with `App::service()` method. @@ -224,16 +224,34 @@ fn directory_listing( /// .service(fs::Files::new("/static", ".")); /// } /// ``` -pub struct Files { +pub struct Files { path: String, directory: PathBuf, index: Option, show_index: bool, default: Rc>>>>, renderer: Rc, + mime_override: Option>, _chunk_size: usize, _follow_symlinks: bool, - _cd_map: PhantomData, + file_flags: named::Flags, +} + +impl Clone for Files { + fn clone(&self) -> Self { + Self { + directory: self.directory.clone(), + index: self.index.clone(), + show_index: self.show_index, + default: self.default.clone(), + renderer: self.renderer.clone(), + _chunk_size: self._chunk_size, + _follow_symlinks: self._follow_symlinks, + file_flags: self.file_flags, + path: self.path.clone(), + mime_override: self.mime_override.clone(), + } + } } impl Files { @@ -243,15 +261,6 @@ impl Files { /// By default pool with 5x threads of available cpus is used. /// Pool size can be changed by setting ACTIX_CPU_POOL environment variable. pub fn new>(path: &str, dir: T) -> Files { - Self::with_config(path, dir, DefaultConfig) - } -} - -impl Files { - /// Create new `Files` instance for specified base directory. - /// - /// Identical with `new` but allows to specify configiration to use. - pub fn with_config>(path: &str, dir: T, _: C) -> Files { let dir = dir.into().canonicalize().unwrap_or_else(|_| PathBuf::new()); if !dir.is_dir() { log::error!("Specified path is not a directory"); @@ -264,9 +273,10 @@ impl Files { show_index: false, default: Rc::new(RefCell::new(None)), renderer: Rc::new(directory_listing), + mime_override: None, _chunk_size: 0, _follow_symlinks: false, - _cd_map: PhantomData, + file_flags: named::Flags::default(), } } @@ -289,20 +299,44 @@ impl Files { self } + /// Specifies mime override callback + pub fn mime_override(mut self, f: F) -> Self where F: Fn(&mime::Name) -> DispositionType + 'static { + self.mime_override = Some(Rc::new(f)); + self + } + /// Set index file /// /// Shows specific index file for directory "/" instead of /// showing files listing. - pub fn index_file>(mut self, index: T) -> Files { + pub fn index_file>(mut self, index: T) -> Self { self.index = Some(index.into()); self } + + #[inline] + ///Specifies whether to use ETag or not. + /// + ///Default is true. + pub fn use_etag(mut self, value: bool) -> Self { + self.file_flags.set(named::Flags::ETAG, value); + self + } + + #[inline] + ///Specifies whether to use Last-Modified or not. + /// + ///Default is true. + pub fn use_last_modified(mut self, value: bool) -> Self { + self.file_flags.set(named::Flags::LAST_MD, value); + self + } + } -impl HttpServiceFactory

for Files +impl

HttpServiceFactory

for Files

where P: 'static, - C: StaticFileConfig + 'static, { fn register(self, config: &mut ServiceConfig

) { if self.default.borrow().is_none() { @@ -317,40 +351,20 @@ where } } -impl NewService for Files { +impl

NewService for Files

{ type Request = ServiceRequest

; type Response = ServiceResponse; type Error = Error; - type Service = FilesService; + type Service = Self; type InitError = (); type Future = FutureResult; fn new_service(&self, _: &()) -> Self::Future { - ok(FilesService { - directory: self.directory.clone(), - index: self.index.clone(), - show_index: self.show_index, - default: self.default.clone(), - renderer: self.renderer.clone(), - _chunk_size: self._chunk_size, - _follow_symlinks: self._follow_symlinks, - _cd_map: self._cd_map, - }) + ok(self.clone()) } } -pub struct FilesService { - directory: PathBuf, - index: Option, - show_index: bool, - default: Rc>>>>, - renderer: Rc, - _chunk_size: usize, - _follow_symlinks: bool, - _cd_map: PhantomData, -} - -impl Service for FilesService { +impl

Service for Files

{ type Request = ServiceRequest

; type Response = ServiceResponse; type Error = Error; @@ -378,10 +392,18 @@ impl Service for FilesService { if let Some(ref redir_index) = self.index { let path = path.join(redir_index); - match NamedFile::open_with_config(path, C::default()) { - Ok(named_file) => match named_file.respond_to(&req) { - Ok(item) => ok(ServiceResponse::new(req.clone(), item)), - Err(e) => ok(ServiceResponse::from_err(e, req.clone())), + match NamedFile::open(path) { + Ok(mut named_file) => { + if let Some(ref mime_override) = self.mime_override { + let new_disposition = mime_override(&named_file.content_type.type_()); + named_file.content_disposition.disposition = new_disposition; + } + + named_file.flags = self.file_flags; + match named_file.respond_to(&req) { + Ok(item) => ok(ServiceResponse::new(req.clone(), item)), + Err(e) => ok(ServiceResponse::from_err(e, req.clone())), + } }, Err(e) => ok(ServiceResponse::from_err(e, req.clone())), } @@ -399,10 +421,18 @@ impl Service for FilesService { )) } } else { - match NamedFile::open_with_config(path, C::default()) { - Ok(named_file) => match named_file.respond_to(&req) { - Ok(item) => ok(ServiceResponse::new(req.clone(), item)), - Err(e) => ok(ServiceResponse::from_err(e, req.clone())), + match NamedFile::open(path) { + Ok(mut named_file) => { + if let Some(ref mime_override) = self.mime_override { + let new_disposition = mime_override(&named_file.content_type.type_()); + named_file.content_disposition.disposition = new_disposition; + } + + named_file.flags = self.file_flags; + match named_file.respond_to(&req) { + Ok(item) => ok(ServiceResponse::new(req.clone(), item)), + Err(e) => ok(ServiceResponse::from_err(e, req.clone())), + } }, Err(e) => ok(ServiceResponse::from_err(e, req.clone())), } @@ -606,53 +636,6 @@ mod tests { ); } - #[derive(Default)] - pub struct AllAttachmentConfig; - impl StaticFileConfig for AllAttachmentConfig { - fn content_disposition_map(_typ: mime::Name) -> DispositionType { - DispositionType::Attachment - } - } - - #[derive(Default)] - pub struct AllInlineConfig; - impl StaticFileConfig for AllInlineConfig { - fn content_disposition_map(_typ: mime::Name) -> DispositionType { - DispositionType::Inline - } - } - - #[test] - fn test_named_file_image_attachment_and_custom_config() { - let file = - NamedFile::open_with_config("tests/test.png", AllAttachmentConfig).unwrap(); - - let req = TestRequest::default().to_http_request(); - let resp = file.respond_to(&req).unwrap(); - assert_eq!( - resp.headers().get(header::CONTENT_TYPE).unwrap(), - "image/png" - ); - assert_eq!( - resp.headers().get(header::CONTENT_DISPOSITION).unwrap(), - "attachment; filename=\"test.png\"" - ); - - let file = - NamedFile::open_with_config("tests/test.png", AllInlineConfig).unwrap(); - - let req = TestRequest::default().to_http_request(); - let resp = file.respond_to(&req).unwrap(); - assert_eq!( - resp.headers().get(header::CONTENT_TYPE).unwrap(), - "image/png" - ); - assert_eq!( - resp.headers().get(header::CONTENT_DISPOSITION).unwrap(), - "inline; filename=\"test.png\"" - ); - } - #[test] fn test_named_file_binary() { let mut file = NamedFile::open("tests/test.binary").unwrap(); @@ -702,6 +685,25 @@ mod tests { assert_eq!(resp.status(), StatusCode::NOT_FOUND); } + #[test] + fn test_mime_override() { + fn all_attachment(_: &mime::Name) -> DispositionType { + DispositionType::Attachment + } + + let mut srv = test::init_service( + App::new().service(Files::new("/", ".").mime_override(all_attachment).index_file("Cargo.toml")), + ); + + let request = TestRequest::get().uri("/").to_request(); + let response = test::call_success(&mut srv, request); + assert_eq!(response.status(), StatusCode::OK); + + let content_disposition = response.headers().get(header::CONTENT_DISPOSITION).expect("To have CONTENT_DISPOSITION"); + let content_disposition = content_disposition.to_str().expect("Convert CONTENT_DISPOSITION to str"); + assert_eq!(content_disposition, "attachment; filename=\"Cargo.toml\""); + } + #[test] fn test_named_file_ranges_status_code() { let mut srv = test::init_service( @@ -860,21 +862,10 @@ mod tests { assert_eq!(bytes.freeze(), data); } - #[derive(Default)] - pub struct OnlyMethodHeadConfig; - impl StaticFileConfig for OnlyMethodHeadConfig { - fn is_method_allowed(method: &Method) -> bool { - match *method { - Method::HEAD => true, - _ => false, - } - } - } - #[test] fn test_named_file_not_allowed() { let file = - NamedFile::open_with_config("Cargo.toml", OnlyMethodHeadConfig).unwrap(); + NamedFile::open("Cargo.toml").unwrap(); let req = TestRequest::default() .method(Method::POST) .to_http_request(); @@ -882,16 +873,10 @@ mod tests { assert_eq!(resp.status(), StatusCode::METHOD_NOT_ALLOWED); let file = - NamedFile::open_with_config("Cargo.toml", OnlyMethodHeadConfig).unwrap(); + NamedFile::open("Cargo.toml").unwrap(); let req = TestRequest::default().method(Method::PUT).to_http_request(); let resp = file.respond_to(&req).unwrap(); assert_eq!(resp.status(), StatusCode::METHOD_NOT_ALLOWED); - - let file = - NamedFile::open_with_config("Cargo.toml", OnlyMethodHeadConfig).unwrap(); - let req = TestRequest::default().method(Method::GET).to_http_request(); - let resp = file.respond_to(&req).unwrap(); - assert_eq!(resp.status(), StatusCode::METHOD_NOT_ALLOWED); } // #[test] @@ -910,9 +895,9 @@ mod tests { // } #[test] - fn test_named_file_any_method() { + fn test_named_file_allowed_method() { let req = TestRequest::default() - .method(Method::POST) + .method(Method::GET) .to_http_request(); let file = NamedFile::open("Cargo.toml").unwrap(); let resp = file.respond_to(&req).unwrap(); diff --git a/actix-files/src/named.rs b/actix-files/src/named.rs index 2bfa30675..d2bf25691 100644 --- a/actix-files/src/named.rs +++ b/actix-files/src/named.rs @@ -1,6 +1,5 @@ use std::fs::{File, Metadata}; use std::io; -use std::marker::PhantomData; use std::ops::{Deref, DerefMut}; use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; @@ -8,20 +7,33 @@ use std::time::{SystemTime, UNIX_EPOCH}; #[cfg(unix)] use std::os::unix::fs::MetadataExt; +use bitflags::bitflags; use mime; use mime_guess::guess_mime_type; -use actix_web::http::header::{self, ContentDisposition, DispositionParam}; +use actix_web::http::header::{self, DispositionType, ContentDisposition, DispositionParam}; use actix_web::http::{ContentEncoding, Method, StatusCode}; use actix_web::{Error, HttpMessage, HttpRequest, HttpResponse, Responder}; -use crate::config::{DefaultConfig, StaticFileConfig}; use crate::range::HttpRange; use crate::ChunkedReadFile; +bitflags! { + pub(crate) struct Flags: u32 { + const ETAG = 0b00000001; + const LAST_MD = 0b00000010; + } +} + +impl Default for Flags { + fn default() -> Self { + Flags::all() + } +} + /// A file with an associated name. #[derive(Debug)] -pub struct NamedFile { +pub struct NamedFile { path: PathBuf, file: File, pub(crate) content_type: mime::Mime, @@ -30,7 +42,7 @@ pub struct NamedFile { modified: Option, encoding: Option, pub(crate) status_code: StatusCode, - _cd_map: PhantomData, + pub(crate) flags: Flags, } impl NamedFile { @@ -55,49 +67,6 @@ impl NamedFile { /// } /// ``` pub fn from_file>(file: File, path: P) -> io::Result { - Self::from_file_with_config(file, path, DefaultConfig) - } - - /// Attempts to open a file in read-only mode. - /// - /// # Examples - /// - /// ```rust - /// use actix_files::NamedFile; - /// - /// let file = NamedFile::open("foo.txt"); - /// ``` - pub fn open>(path: P) -> io::Result { - Self::open_with_config(path, DefaultConfig) - } -} - -impl NamedFile { - /// Creates an instance from a previously opened file using the provided configuration. - /// - /// The given `path` need not exist and is only used to determine the `ContentType` and - /// `ContentDisposition` headers. - /// - /// # Examples - /// - /// ```rust - /// use actix_files::{DefaultConfig, NamedFile}; - /// use std::io::{self, Write}; - /// use std::env; - /// use std::fs::File; - /// - /// fn main() -> io::Result<()> { - /// let mut file = File::create("foo.txt")?; - /// file.write_all(b"Hello, world!")?; - /// let named_file = NamedFile::from_file_with_config(file, "bar.txt", DefaultConfig)?; - /// Ok(()) - /// } - /// ``` - pub fn from_file_with_config>( - file: File, - path: P, - _: C, - ) -> io::Result> { let path = path.as_ref().to_path_buf(); // Get the name of the file and use it to construct default Content-Type @@ -114,7 +83,10 @@ impl NamedFile { }; let ct = guess_mime_type(&path); - let disposition_type = C::content_disposition_map(ct.type_()); + let disposition_type = match ct.type_() { + mime::IMAGE | mime::TEXT | mime::VIDEO => DispositionType::Inline, + _ => DispositionType::Attachment, + }; let cd = ContentDisposition { disposition: disposition_type, parameters: vec![DispositionParam::Filename(filename.into_owned())], @@ -134,24 +106,21 @@ impl NamedFile { modified, encoding, status_code: StatusCode::OK, - _cd_map: PhantomData, + flags: Flags::default(), }) } - /// Attempts to open a file in read-only mode using provided configuration. + /// Attempts to open a file in read-only mode. /// /// # Examples /// /// ```rust - /// use actix_files::{DefaultConfig, NamedFile}; + /// use actix_files::NamedFile; /// - /// let file = NamedFile::open_with_config("foo.txt", DefaultConfig); + /// let file = NamedFile::open("foo.txt"); /// ``` - pub fn open_with_config>( - path: P, - config: C, - ) -> io::Result> { - Self::from_file_with_config(File::open(&path)?, path, config) + pub fn open>(path: P) -> io::Result { + Self::from_file(File::open(&path)?, path) } /// Returns reference to the underlying `File` object. @@ -213,6 +182,24 @@ impl NamedFile { self } + #[inline] + ///Specifies whether to use ETag or not. + /// + ///Default is true. + pub fn use_etag(mut self, value: bool) -> Self { + self.flags.set(Flags::ETAG, value); + self + } + + #[inline] + ///Specifies whether to use Last-Modified or not. + /// + ///Default is true. + pub fn use_last_modified(mut self, value: bool) -> Self { + self.flags.set(Flags::LAST_MD, value); + self + } + pub(crate) fn etag(&self) -> Option { // This etag format is similar to Apache's. self.modified.as_ref().map(|mtime| { @@ -245,7 +232,7 @@ impl NamedFile { } } -impl Deref for NamedFile { +impl Deref for NamedFile { type Target = File; fn deref(&self) -> &File { @@ -253,7 +240,7 @@ impl Deref for NamedFile { } } -impl DerefMut for NamedFile { +impl DerefMut for NamedFile { fn deref_mut(&mut self) -> &mut File { &mut self.file } @@ -294,7 +281,7 @@ fn none_match(etag: Option<&header::EntityTag>, req: &HttpRequest) -> bool { } } -impl Responder for NamedFile { +impl Responder for NamedFile { type Error = Error; type Future = Result; @@ -320,15 +307,18 @@ impl Responder for NamedFile { return Ok(resp.streaming(reader)); } - if !C::is_method_allowed(req.method()) { - return Ok(HttpResponse::MethodNotAllowed() - .header(header::CONTENT_TYPE, "text/plain") - .header(header::ALLOW, "GET, HEAD") - .body("This resource only supports GET and HEAD.")); + match req.method() { + &Method::HEAD | &Method::GET => (), + _ => { + return Ok(HttpResponse::MethodNotAllowed() + .header(header::CONTENT_TYPE, "text/plain") + .header(header::ALLOW, "GET, HEAD") + .body("This resource only supports GET and HEAD.")); + } } - let etag = if C::is_use_etag() { self.etag() } else { None }; - let last_modified = if C::is_use_last_modifier() { + let etag = if self.flags.contains(Flags::ETAG) { self.etag() } else { None }; + let last_modified = if self.flags.contains(Flags::LAST_MD) { self.last_modified() } else { None