From 4de2e8a8983f96937d3c7d660ec9bd76951b5ebc Mon Sep 17 00:00:00 2001 From: Naim A <227396+naim94a@users.noreply.github.com> Date: Tue, 8 Oct 2019 07:09:40 +0300 Subject: [PATCH] [actix-files] Allow user defined guards for NamedFile (actix#1113) (#1115) * [actix-files] remove request method checks from NamedFile * [actix-files] added custom guard checks to FilesService * [actix-files] modify method check tests (NamedFile -> Files) * [actix-files] add test for custom guards in Files * [actix-files] update changelog --- actix-files/CHANGES.md | 2 ++ actix-files/src/lib.rs | 77 +++++++++++++++++++++++++++++++++++----- actix-files/src/named.rs | 12 +------ 3 files changed, 72 insertions(+), 19 deletions(-) diff --git a/actix-files/CHANGES.md b/actix-files/CHANGES.md index 49ecdbff..2421890d 100644 --- a/actix-files/CHANGES.md +++ b/actix-files/CHANGES.md @@ -6,6 +6,8 @@ * Bump up `percent-encoding` crate version to 2.1 +* Allow user defined request guards for `Files` #1113 + ## [0.1.4] - 2019-07-20 * Allow to disable `Content-Disposition` header #686 diff --git a/actix-files/src/lib.rs b/actix-files/src/lib.rs index c99d3265..7fc3c45c 100644 --- a/actix-files/src/lib.rs +++ b/actix-files/src/lib.rs @@ -15,8 +15,10 @@ use actix_web::dev::{ AppService, HttpServiceFactory, Payload, ResourceDef, ServiceRequest, ServiceResponse, }; +use actix_web::guard::Guard; use actix_web::error::{BlockingError, Error, ErrorInternalServerError}; -use actix_web::http::header::DispositionType; +use actix_web::http::header::{self, DispositionType}; +use actix_web::http::Method; use actix_web::{web, FromRequest, HttpRequest, HttpResponse, Responder}; use bytes::Bytes; use futures::future::{ok, Either, FutureResult}; @@ -235,6 +237,7 @@ pub struct Files { renderer: Rc, mime_override: Option>, file_flags: named::Flags, + guards: Option>>, } impl Clone for Files { @@ -248,6 +251,7 @@ impl Clone for Files { file_flags: self.file_flags, path: self.path.clone(), mime_override: self.mime_override.clone(), + guards: self.guards.clone(), } } } @@ -273,6 +277,7 @@ impl Files { renderer: Rc::new(directory_listing), mime_override: None, file_flags: named::Flags::default(), + guards: None, } } @@ -331,6 +336,15 @@ impl Files { self } + /// Specifies custom guards to use for directory listings and files. + /// + /// Default behaviour allows GET and HEAD. + #[inline] + pub fn use_guards(mut self, guards: G) -> Self { + self.guards = Some(Rc::new(Box::new(guards))); + self + } + /// Disable `Content-Disposition` header. /// /// By default Content-Disposition` header is enabled. @@ -392,6 +406,7 @@ impl NewService for Files { renderer: self.renderer.clone(), mime_override: self.mime_override.clone(), file_flags: self.file_flags, + guards: self.guards.clone(), }; if let Some(ref default) = *self.default.borrow() { @@ -418,6 +433,7 @@ pub struct FilesService { renderer: Rc, mime_override: Option>, file_flags: named::Flags, + guards: Option>>, } impl FilesService { @@ -454,6 +470,25 @@ impl Service for FilesService { fn call(&mut self, req: ServiceRequest) -> Self::Future { // let (req, pl) = req.into_parts(); + let is_method_valid = if let Some(guard) = &self.guards { + // execute user defined guards + (**guard).check(req.head()) + } else { + // default behaviour + match *req.method() { + Method::HEAD | Method::GET => true, + _ => false, + } + }; + + if !is_method_valid { + return Either::A(ok(req.into_response( + actix_web::HttpResponse::MethodNotAllowed() + .header(header::CONTENT_TYPE, "text/plain") + .body("Request did not meet this resource's requirements.") + ))); + } + let real_path = match PathBufWrp::get_pathbuf(req.match_info().path()) { Ok(item) => item, Err(e) => return Either::A(ok(req.error_response(e))), @@ -576,6 +611,7 @@ mod tests { use bytes::BytesMut; use super::*; + use actix_web::guard; use actix_web::http::header::{ self, ContentDisposition, DispositionParam, DispositionType, }; @@ -1010,20 +1046,45 @@ mod tests { } #[test] - fn test_named_file_not_allowed() { - let file = NamedFile::open("Cargo.toml").unwrap(); + fn test_files_not_allowed() { + let mut srv = test::init_service( + App::new().service(Files::new("/", ".")), + ); + let req = TestRequest::default() + .uri("/Cargo.toml") .method(Method::POST) - .to_http_request(); - let resp = file.respond_to(&req).unwrap(); + .to_request(); + + let resp = test::call_service(&mut srv, req); assert_eq!(resp.status(), StatusCode::METHOD_NOT_ALLOWED); - let file = NamedFile::open("Cargo.toml").unwrap(); - let req = TestRequest::default().method(Method::PUT).to_http_request(); - let resp = file.respond_to(&req).unwrap(); + let mut srv = test::init_service( + App::new().service(Files::new("/", ".")), + ); + let req = TestRequest::default().method(Method::PUT).uri("/Cargo.toml").to_request(); + let resp = test::call_service(&mut srv, req); assert_eq!(resp.status(), StatusCode::METHOD_NOT_ALLOWED); } + #[test] + fn test_files_guards() { + let mut srv = test::init_service( + App::new().service( + Files::new("/", ".") + .use_guards(guard::Post()) + ), + ); + + let req = TestRequest::default() + .uri("/Cargo.toml") + .method(Method::POST) + .to_request(); + + let resp = test::call_service(&mut srv, req); + assert_eq!(resp.status(), StatusCode::OK); + } + #[test] fn test_named_file_content_encoding() { let mut srv = test::init_service(App::new().wrap(Compress::default()).service( diff --git a/actix-files/src/named.rs b/actix-files/src/named.rs index f548a7a1..ca1a909a 100644 --- a/actix-files/src/named.rs +++ b/actix-files/src/named.rs @@ -15,7 +15,7 @@ use actix_http::body::SizedStream; use actix_web::http::header::{ self, ContentDisposition, DispositionParam, DispositionType, }; -use actix_web::http::{ContentEncoding, Method, StatusCode}; +use actix_web::http::{ContentEncoding, StatusCode}; use actix_web::middleware::BodyEncoding; use actix_web::{Error, HttpMessage, HttpRequest, HttpResponse, Responder}; @@ -324,16 +324,6 @@ impl Responder for NamedFile { return Ok(resp.streaming(reader)); } - 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 self.flags.contains(Flags::ETAG) { self.etag() } else {