Remove StaticFilesConfig (#731)

* Remove StaticFilesConfig

* Applying comments

* Impl Clone for Files<S>

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"

actix-files/src/config.rs

@@ -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 {}

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<S, C = DefaultConfig> {
+pub struct Files<S> {
     path: String,
     directory: PathBuf,
     index: Option<String>,
     show_index: bool,
     default: Rc<RefCell<Option<Rc<HttpNewService<S>>>>>,
     renderer: Rc<DirectoryRenderer>,
+    mime_override: Option<Rc<MimeOverride>>,
     _chunk_size: usize,
     _follow_symlinks: bool,
-    _cd_map: PhantomData<C>,
+    file_flags: named::Flags,
+}
+
+impl<S> Clone for Files<S> {
+    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<S: 'static> Files<S> {
@@ -243,15 +261,6 @@ impl<S: 'static> Files<S> {
     /// 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<T: Into<PathBuf>>(path: &str, dir: T) -> Files<S> {
-        Self::with_config(path, dir, DefaultConfig)
-    }
-}
-
-impl<S: 'static, C: StaticFileConfig> Files<S, C> {
-    /// Create new `Files` instance for specified base directory.
-    ///
-    /// Identical with `new` but allows to specify configiration to use.
-    pub fn with_config<T: Into<PathBuf>>(path: &str, dir: T, _: C) -> Files<S, C> {
         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<S: 'static, C: StaticFileConfig> Files<S, C> {
             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<S: 'static, C: StaticFileConfig> Files<S, C> {
         self
     }
 
+    /// Specifies mime override callback
+    pub fn mime_override<F>(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<T: Into<String>>(mut self, index: T) -> Files<S, C> {
+    pub fn index_file<T: Into<String>>(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
     }
 
-impl<P, C> HttpServiceFactory<P> for Files<P, C>
+    #[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<P> HttpServiceFactory<P> for Files<P>
 where
     P: 'static,
-    C: StaticFileConfig + 'static,
 {
     fn register(self, config: &mut ServiceConfig<P>) {
         if self.default.borrow().is_none() {
@@ -317,40 +351,20 @@ where
     }
 }
 
-impl<P, C: StaticFileConfig + 'static> NewService for Files<P, C> {
+impl<P> NewService for Files<P> {
     type Request = ServiceRequest<P>;
     type Response = ServiceResponse;
     type Error = Error;
-    type Service = FilesService<P, C>;
+    type Service = Self;
     type InitError = ();
     type Future = FutureResult<Self::Service, Self::InitError>;
 
     fn new_service(&self, _: &()) -> Self::Future {
-        ok(FilesService {
+        ok(self.clone())
-            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,
-        })
     }
 }
 
-pub struct FilesService<S, C = DefaultConfig> {
+impl<P> Service for Files<P> {
-    directory: PathBuf,
-    index: Option<String>,
-    show_index: bool,
-    default: Rc<RefCell<Option<Rc<HttpNewService<S>>>>>,
-    renderer: Rc<DirectoryRenderer>,
-    _chunk_size: usize,
-    _follow_symlinks: bool,
-    _cd_map: PhantomData<C>,
-}
-
-impl<P, C: StaticFileConfig> Service for FilesService<P, C> {
     type Request = ServiceRequest<P>;
     type Response = ServiceResponse;
     type Error = Error;
@@ -378,10 +392,18 @@ impl<P, C: StaticFileConfig> Service for FilesService<P, C> {
             if let Some(ref redir_index) = self.index {
                 let path = path.join(redir_index);
 
-                match NamedFile::open_with_config(path, C::default()) {
+                match NamedFile::open(path) {
-                    Ok(named_file) => match named_file.respond_to(&req) {
+                    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<P, C: StaticFileConfig> Service for FilesService<P, C> {
                 ))
             }
         } else {
-            match NamedFile::open_with_config(path, C::default()) {
+            match NamedFile::open(path) {
-                Ok(named_file) => match named_file.respond_to(&req) {
+                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();

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<C = DefaultConfig> {
+pub struct NamedFile {
     path: PathBuf,
     file: File,
     pub(crate) content_type: mime::Mime,
@@ -30,7 +42,7 @@ pub struct NamedFile<C = DefaultConfig> {
     modified: Option<SystemTime>,
     encoding: Option<ContentEncoding>,
     pub(crate) status_code: StatusCode,
-    _cd_map: PhantomData<C>,
+    pub(crate) flags: Flags,
 }
 
 impl NamedFile {
@@ -55,49 +67,6 @@ impl NamedFile {
     /// }
     /// ```
     pub fn from_file<P: AsRef<Path>>(file: File, path: P) -> io::Result<NamedFile> {
-        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<P: AsRef<Path>>(path: P) -> io::Result<NamedFile> {
-        Self::open_with_config(path, DefaultConfig)
-    }
-}
-
-impl<C: StaticFileConfig> NamedFile<C> {
-    /// 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<P: AsRef<Path>>(
-        file: File,
-        path: P,
-        _: C,
-    ) -> io::Result<NamedFile<C>> {
         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<C: StaticFileConfig> NamedFile<C> {
             };
 
             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<C: StaticFileConfig> NamedFile<C> {
             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<P: AsRef<Path>>(
+    pub fn open<P: AsRef<Path>>(path: P) -> io::Result<NamedFile> {
-        path: P,
+        Self::from_file(File::open(&path)?, path)
-        config: C,
-    ) -> io::Result<NamedFile<C>> {
-        Self::from_file_with_config(File::open(&path)?, path, config)
     }
 
     /// Returns reference to the underlying `File` object.
@@ -213,6 +182,24 @@ impl<C: StaticFileConfig> NamedFile<C> {
         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<header::EntityTag> {
         // This etag format is similar to Apache's.
         self.modified.as_ref().map(|mtime| {
@@ -245,7 +232,7 @@ impl<C: StaticFileConfig> NamedFile<C> {
     }
 }
 
-impl<C> Deref for NamedFile<C> {
+impl Deref for NamedFile {
     type Target = File;
 
     fn deref(&self) -> &File {
@@ -253,7 +240,7 @@ impl<C> Deref for NamedFile<C> {
     }
 }
 
-impl<C> DerefMut for NamedFile<C> {
+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<C: StaticFileConfig> Responder for NamedFile<C> {
+impl Responder for NamedFile {
     type Error = Error;
     type Future = Result<HttpResponse, Error>;
 
@@ -320,15 +307,18 @@ impl<C: StaticFileConfig> Responder for NamedFile<C> {
             return Ok(resp.streaming(reader));
         }
 
-        if !C::is_method_allowed(req.method()) {
+        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 etag = if self.flags.contains(Flags::ETAG) { self.etag() } else { None };
-        let last_modified = if C::is_use_last_modifier() {
+        let last_modified = if self.flags.contains(Flags::LAST_MD) {
             self.last_modified()
         } else {
             None