From facc43e5ad432e9a227b639da6e47b3e0b9cd819 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sun, 26 Feb 2023 02:36:43 +0000 Subject: [PATCH] rename Tempfile -> TempFile --- actix-multipart/CHANGES.md | 3 +- actix-multipart/Cargo.toml | 14 +++-- actix-multipart/src/form/bytes.rs | 2 +- actix-multipart/src/form/mod.rs | 46 ++++++++-------- actix-multipart/src/form/tempfile.rs | 80 ++++++++++++++-------------- actix-multipart/src/form/text.rs | 20 +++---- 6 files changed, 86 insertions(+), 79 deletions(-) diff --git a/actix-multipart/CHANGES.md b/actix-multipart/CHANGES.md index 537110802..53134db4f 100644 --- a/actix-multipart/CHANGES.md +++ b/actix-multipart/CHANGES.md @@ -3,13 +3,12 @@ ## Unreleased - 2022-xx-xx - Added `MultipartForm` typed data extractor. [#2883] -[#2880]: https://github.com/actix/actix-web/pull/2880 [#2883]: https://github.com/actix/actix-web/pull/2883 ## 0.5.0 - 2023-01-21 +- `Field::content_type()` now returns `Option<&mime::Mime>`. [#2885] - Minimum supported Rust version (MSRV) is now 1.59 due to transitive `time` dependency. -- `Field::content_type()` now returns `Option<&mime::Mime>` [#2885] [#2885]: https://github.com/actix/actix-web/pull/2885 diff --git a/actix-multipart/Cargo.toml b/actix-multipart/Cargo.toml index 0a10cb935..fc84559a8 100644 --- a/actix-multipart/Cargo.toml +++ b/actix-multipart/Cargo.toml @@ -1,7 +1,10 @@ [package] name = "actix-multipart" version = "0.5.0" -authors = ["Nikolay Kim "] +authors = [ + "Nikolay Kim ", + "Jacob Halsey ", +] description = "Multipart form support for Actix Web" keywords = ["http", "web", "framework", "async", "futures"] homepage = "https://actix.rs" @@ -30,6 +33,7 @@ actix-web = { version = "4", default-features = false } bytes = "1" derive_more = "0.99.5" futures-core = { version = "0.3.17", default-features = false, features = ["alloc"] } +futures-util = { version = "0.3.17", default-features = false, features = ["alloc"] } httparse = "1.3" local-waker = "0.1" log = "0.4" @@ -39,15 +43,15 @@ serde = "1" serde_json = "1" serde_plain = "1" # TODO(MSRV 1.60): replace with dep: prefix -tempfile-dep = { package = "tempfile", version = "3.3.0", optional = true } -tokio = { version = "1.13.1", features = ["sync"] } +tempfile-dep = { package = "tempfile", version = "3.4", optional = true } +tokio = { version = "1.18.5", features = ["sync"] } [dev-dependencies] actix-http = "3" actix-multipart-rfc7578 = "0.10" actix-rt = "2.2" -actix-test = "0.1.0" -awc = "3.0.1" +actix-test = "0.1" +awc = "3" futures-util = { version = "0.3.17", default-features = false, features = ["alloc"] } tokio = { version = "1.18.5", features = ["sync"] } tokio-stream = "0.1" diff --git a/actix-multipart/src/form/bytes.rs b/actix-multipart/src/form/bytes.rs index 5df50a9a0..7d64fffce 100644 --- a/actix-multipart/src/form/bytes.rs +++ b/actix-multipart/src/form/bytes.rs @@ -33,7 +33,7 @@ impl<'t> FieldReader<'t> for Bytes { limits: &'t mut Limits, ) -> Self::Future { Box::pin(async move { - let mut buf = BytesMut::new(); + let mut buf = BytesMut::with_capacity(131_072); while let Some(chunk) = field.try_next().await? { limits.try_consume_limits(chunk.len(), true)?; diff --git a/actix-multipart/src/form/mod.rs b/actix-multipart/src/form/mod.rs index c94a457f1..b0285d97e 100644 --- a/actix-multipart/src/form/mod.rs +++ b/actix-multipart/src/form/mod.rs @@ -10,7 +10,7 @@ use std::{ use actix_web::{dev, error::PayloadError, web, Error, FromRequest, HttpRequest}; use derive_more::{Deref, DerefMut}; use futures_core::future::LocalBoxFuture; -use futures_util::{TryFutureExt, TryStreamExt as _}; +use futures_util::{TryFutureExt as _, TryStreamExt as _}; use crate::{Field, Multipart, MultipartError}; @@ -80,7 +80,7 @@ where DuplicateField::Deny => { return Box::pin(ready(Err(MultipartError::DuplicateField( - field.name().to_string(), + field.name().to_owned(), )))) } @@ -89,7 +89,7 @@ where } Box::pin(async move { - let field_name = field.name().to_string(); + let field_name = field.name().to_owned(); let t = T::read_field(req, field, limits).await?; state.insert(field_name, Box::new(t)); Ok(()) @@ -117,11 +117,11 @@ where Box::pin(async move { // Note: Vec GroupReader always allows duplicates - let field_name = field.name().to_string(); + let field_name = field.name().to_owned(); let vec = state .entry(field_name) - .or_insert_with(|| Box::new(Vec::::new())) + .or_insert_with(|| Box::>::default()) .downcast_mut::>() .unwrap(); @@ -159,15 +159,16 @@ where DuplicateField::Deny => { return Box::pin(ready(Err(MultipartError::DuplicateField( - field.name().to_string(), + field.name().to_owned(), )))) } DuplicateField::Replace => {} } } + Box::pin(async move { - let field_name = field.name().to_string(); + let field_name = field.name().to_owned(); let t = T::read_field(req, field, limits).await?; state.insert(field_name, Box::new(t)); Ok(()) @@ -182,8 +183,9 @@ where } } -/// Trait that allows a type to be used in the [`struct@MultipartForm`] extractor. You should use -/// the [`macro@MultipartForm`] to implement this for your struct. +/// Trait that allows a type to be used in the [`struct@MultipartForm`] extractor. +/// +/// You should use the [`macro@MultipartForm`] macro to derive this for your struct. pub trait MultipartCollect: Sized { /// An optional limit in bytes to be applied a given field name. Note this limit will be shared /// across all fields sharing the same name. @@ -205,13 +207,13 @@ pub trait MultipartCollect: Sized { #[doc(hidden)] pub enum DuplicateField { - /// Additional fields are not processed + /// Additional fields are not processed. Ignore, - /// An error will be raised + /// An error will be raised. Deny, - /// All fields will be processed, the last one will replace all previous + /// All fields will be processed, the last one will replace all previous. Replace, } @@ -270,10 +272,10 @@ impl Limits { /// Typed `multipart/form-data` extractor. /// /// To extract typed data from a multipart stream, the inner type `T` must implement the -/// [`MultipartCollect`] trait, you should use the [`macro@MultipartForm`] macro to derive this +/// [`MultipartCollect`] trait. You should use the [`macro@MultipartForm`] macro to derive this /// for your struct. /// -/// Use [`MultipartFormConfig`] to configure extraction options. +/// Add a [`MultipartFormConfig`] to your app data to configure extraction. #[derive(Deref, DerefMut)] pub struct MultipartForm(pub T); @@ -338,6 +340,8 @@ type MultipartFormErrorHandler = Option Error + Send + Sync>>; /// [`struct@MultipartForm`] extractor configuration. +/// +/// Add to your app data to have it picked up by [`struct@MultipartForm`] extractors. #[derive(Clone)] pub struct MultipartFormConfig { total_limit: usize, @@ -346,19 +350,19 @@ pub struct MultipartFormConfig { } impl MultipartFormConfig { - /// Set maximum accepted payload size for the entire form. By default this limit is 50MiB. + /// Sets maximum accepted payload size for the entire form. By default this limit is 50MiB. pub fn total_limit(mut self, total_limit: usize) -> Self { self.total_limit = total_limit; self } - /// Set maximum accepted data that will be read into memory. By default this limit is 2MiB. + /// Sets maximum accepted data that will be read into memory. By default this limit is 2MiB. pub fn memory_limit(mut self, memory_limit: usize) -> Self { self.memory_limit = memory_limit; self } - /// Set custom error handler. + /// Sets custom error handler. pub fn error_handler(mut self, f: F) -> Self where F: Fn(MultipartError, &HttpRequest) -> Error + Send + Sync + 'static, @@ -367,7 +371,7 @@ impl MultipartFormConfig { self } - /// Extract payload config from app data. Check both `T` and `Data`, in that order, and fall + /// Extracts payload config from app data. Check both `T` and `Data`, in that order, and fall /// back to the default payload config. fn from_req(req: &HttpRequest) -> &Self { req.app_data::() @@ -384,7 +388,7 @@ const DEFAULT_CONFIG: MultipartFormConfig = MultipartFormConfig { impl Default for MultipartFormConfig { fn default() -> Self { - DEFAULT_CONFIG.clone() + DEFAULT_CONFIG } } @@ -397,7 +401,7 @@ mod tests { use awc::{Client, ClientResponse}; use super::MultipartForm; - use crate::form::{bytes::Bytes, tempfile::Tempfile, text::Text, MultipartFormConfig}; + use crate::form::{bytes::Bytes, tempfile::TempFile, text::Text, MultipartFormConfig}; pub async fn send_form( srv: &TestServer, @@ -611,7 +615,7 @@ mod tests { #[derive(MultipartForm)] struct TestFileUploadLimits { - field: Tempfile, + field: TempFile, } async fn test_upload_limits_memory( diff --git a/actix-multipart/src/form/tempfile.rs b/actix-multipart/src/form/tempfile.rs index 52966e5fa..3c637e717 100644 --- a/actix-multipart/src/form/tempfile.rs +++ b/actix-multipart/src/form/tempfile.rs @@ -16,13 +16,13 @@ use tokio::io::AsyncWriteExt; use super::FieldErrorHandler; use crate::{ - form::{tempfile::TempfileError::FileIo, FieldReader, Limits}, + form::{FieldReader, Limits}, Field, MultipartError, }; /// Write the field to a temporary file on disk. #[derive(Debug)] -pub struct Tempfile { +pub struct TempFile { /// The temporary file on disk. pub file: NamedTempFile, @@ -36,7 +36,7 @@ pub struct Tempfile { pub size: usize, } -impl<'t> FieldReader<'t> for Tempfile { +impl<'t> FieldReader<'t> for TempFile { type Future = LocalBoxFuture<'t, Result>; fn read_field( @@ -45,34 +45,31 @@ impl<'t> FieldReader<'t> for Tempfile { limits: &'t mut Limits, ) -> Self::Future { Box::pin(async move { - let config = TempfileConfig::from_req(req); + let config = TempFileConfig::from_req(req); let field_name = field.name().to_owned(); let mut size = 0; - let file = config - .create_tempfile() - .map_err(|err| config.map_error(req, &field_name, FileIo(err)))?; + let file = config.create_tempfile().map_err(|err| { + config.map_error(req, &field_name, TempFileError::FileIo(err)) + })?; - let mut file_async = tokio::fs::File::from_std( - file.reopen() - .map_err(|err| config.map_error(req, &field_name, FileIo(err)))?, - ); + let mut file_async = tokio::fs::File::from_std(file.reopen().map_err(|err| { + config.map_error(req, &field_name, TempFileError::FileIo(err)) + })?); while let Some(chunk) = field.try_next().await? { limits.try_consume_limits(chunk.len(), false)?; size += chunk.len(); - file_async - .write_all(chunk.as_ref()) - .await - .map_err(|err| config.map_error(req, &field_name, FileIo(err)))?; + file_async.write_all(chunk.as_ref()).await.map_err(|err| { + config.map_error(req, &field_name, TempFileError::FileIo(err)) + })?; } - file_async - .flush() - .await - .map_err(|err| config.map_error(req, &field_name, FileIo(err)))?; + file_async.flush().await.map_err(|err| { + config.map_error(req, &field_name, TempFileError::FileIo(err)) + })?; - Ok(Tempfile { + Ok(TempFile { file, content_type: field.content_type().map(ToOwned::to_owned), file_name: field @@ -87,28 +84,28 @@ impl<'t> FieldReader<'t> for Tempfile { #[derive(Debug, Display, Error)] #[non_exhaustive] -pub enum TempfileError { +pub enum TempFileError { /// File I/O Error #[display(fmt = "File I/O error: {}", _0)] FileIo(std::io::Error), } -impl ResponseError for TempfileError { +impl ResponseError for TempFileError { fn status_code(&self) -> StatusCode { StatusCode::INTERNAL_SERVER_ERROR } } -/// Configuration for the [`Tempfile`] field reader. +/// Configuration for the [`TempFile`] field reader. #[derive(Clone)] -pub struct TempfileConfig { - err_handler: FieldErrorHandler, +pub struct TempFileConfig { + err_handler: FieldErrorHandler, directory: Option, } -impl TempfileConfig { +impl TempFileConfig { fn create_tempfile(&self) -> io::Result { - if let Some(dir) = self.directory.as_deref() { + if let Some(ref dir) = self.directory { NamedTempFile::new_in(dir) } else { NamedTempFile::new() @@ -116,21 +113,17 @@ impl TempfileConfig { } } -const DEFAULT_CONFIG: TempfileConfig = TempfileConfig { - err_handler: None, - directory: None, -}; - -impl TempfileConfig { +impl TempFileConfig { + /// Sets custom error handler. pub fn error_handler(mut self, f: F) -> Self where - F: Fn(TempfileError, &HttpRequest) -> Error + Send + Sync + 'static, + F: Fn(TempFileError, &HttpRequest) -> Error + Send + Sync + 'static, { self.err_handler = Some(Arc::new(f)); self } - /// Extract payload config from app data. Check both `T` and `Data`, in that order, and fall + /// Extracts payload config from app data. Check both `T` and `Data`, in that order, and fall /// back to the default payload config. fn from_req(req: &HttpRequest) -> &Self { req.app_data::() @@ -142,10 +135,10 @@ impl TempfileConfig { &self, req: &HttpRequest, field_name: &str, - err: TempfileError, + err: TempFileError, ) -> MultipartError { - let source = if let Some(err_handler) = self.err_handler.as_ref() { - (*err_handler)(err, req) + let source = if let Some(ref err_handler) = self.err_handler { + (err_handler)(err, req) } else { err.into() }; @@ -165,7 +158,12 @@ impl TempfileConfig { } } -impl Default for TempfileConfig { +const DEFAULT_CONFIG: TempFileConfig = TempFileConfig { + err_handler: None, + directory: None, +}; + +impl Default for TempFileConfig { fn default() -> Self { DEFAULT_CONFIG } @@ -178,11 +176,11 @@ mod tests { use actix_multipart_rfc7578::client::multipart; use actix_web::{http::StatusCode, web, App, HttpResponse, Responder}; - use crate::form::{tempfile::Tempfile, tests::send_form, MultipartForm}; + use crate::form::{tempfile::TempFile, tests::send_form, MultipartForm}; #[derive(MultipartForm)] struct FileForm { - file: Tempfile, + file: TempFile, } async fn test_file_route(form: MultipartForm) -> impl Responder { diff --git a/actix-multipart/src/form/text.rs b/actix-multipart/src/form/text.rs index bccf66d98..83e211524 100644 --- a/actix-multipart/src/form/text.rs +++ b/actix-multipart/src/form/text.rs @@ -21,6 +21,7 @@ use crate::{ pub struct Text(pub T); impl Text { + /// Unwraps into inner value. pub fn into_inner(self) -> T { self.0 } @@ -41,7 +42,7 @@ where let valid = if let Some(mime) = field.content_type() { mime.subtype() == mime::PLAIN || mime.suffix() == Some(mime::PLAIN) } else { - // https://www.rfc-editor.org/rfc/rfc7578#section-4.4 + // https://datatracker.ietf.org/doc/html/rfc7578#section-4.4 // content type defaults to text/plain, so None should be considered valid true }; @@ -100,12 +101,8 @@ pub struct TextConfig { validate_content_type: bool, } -const DEFAULT_CONFIG: TextConfig = TextConfig { - err_handler: None, - validate_content_type: true, -}; - impl TextConfig { + /// Sets custom error handler. pub fn error_handler(mut self, f: F) -> Self where F: Fn(TextError, &HttpRequest) -> Error + Send + Sync + 'static, @@ -114,7 +111,7 @@ impl TextConfig { self } - /// Extract payload config from app data. Check both `T` and `Data`, in that order, and fall + /// Extracts payload config from app data. Check both `T` and `Data`, in that order, and fall /// back to the default payload config. fn from_req(req: &HttpRequest) -> &Self { req.app_data::() @@ -123,8 +120,8 @@ impl TextConfig { } fn map_error(&self, req: &HttpRequest, err: TextError) -> Error { - if let Some(err_handler) = self.err_handler.as_ref() { - (*err_handler)(err, req) + if let Some(ref err_handler) = self.err_handler { + (err_handler)(err, req) } else { err.into() } @@ -140,6 +137,11 @@ impl TextConfig { } } +const DEFAULT_CONFIG: TextConfig = TextConfig { + err_handler: None, + validate_content_type: true, +}; + impl Default for TextConfig { fn default() -> Self { DEFAULT_CONFIG