fix(multipart): optional content-disposition for non-form-data reques…

…ts (#3416)

actix-multipart-derive/src/lib.rs

@@ -138,7 +138,7 @@ struct ParsedField<'t> {
 /// `#[multipart(duplicate_field = "<behavior>")]` attribute:
 ///
 /// - "ignore": (default) Extra fields are ignored. I.e., the first one is persisted.
-/// - "deny": A `MultipartError::UnsupportedField` error response is returned.
+/// - "deny": A `MultipartError::UnknownField` error response is returned.
 /// - "replace": Each field is processed, but only the last one is persisted.
 ///
 /// Note that `Vec` fields will ignore this option.
@@ -229,7 +229,7 @@ pub fn impl_multipart_form(input: proc_macro::TokenStream) -> proc_macro::TokenS
     // Return value when a field name is not supported by the form
     let unknown_field_result = if attrs.deny_unknown_fields {
         quote!(::std::result::Result::Err(
-            ::actix_multipart::MultipartError::UnsupportedField(field.name().to_string())
+            ::actix_multipart::MultipartError::UnknownField(field.name().unwrap().to_string())
         ))
     } else {
         quote!(::std::result::Result::Ok(()))
@@ -292,7 +292,7 @@ pub fn impl_multipart_form(input: proc_macro::TokenStream) -> proc_macro::TokenS
                 limits: &'t mut ::actix_multipart::form::Limits,
                 state: &'t mut ::actix_multipart::form::State,
             ) -> ::std::pin::Pin<::std::boxed::Box<dyn ::std::future::Future<Output = ::std::result::Result<(), ::actix_multipart::MultipartError>> + 't>> {
-                match field.name() {
+                match field.name().unwrap() {
                     #handle_field_impl
                     _ => return ::std::boxed::Box::pin(::std::future::ready(#unknown_field_result)),
                 }

actix-multipart/CHANGES.md

@@ -2,6 +2,15 @@
 
 ## Unreleased
 
+- Add `MultipartError::ContentTypeIncompatible` variant.
+- Add `MultipartError::ContentDispositionNameMissing` variant.
+- Rename `MultipartError::{NoContentDisposition => ContentDispositionMissing}` variant.
+- Rename `MultipartError::{NoContentType => ContentTypeMissing}` variant.
+- Rename `MultipartError::{ParseContentType => ContentTypeParse}` variant.
+- Rename `MultipartError::{Boundary => BoundaryMissing}` variant.
+- Rename `MultipartError::{UnsupportedField => UnknownField}` variant.
+- Remove top-level re-exports of `test` utilities.
+
 ## 0.6.2
 
 - Add testing utilities under new module `test`.

actix-multipart/Cargo.toml

@@ -63,7 +63,9 @@ actix-multipart-rfc7578 = "0.10"
 actix-rt = "2.2"
 actix-test = "0.1"
 actix-web = "4"
+assert_matches = "1"
 awc = "3"
+env_logger = "0.11"
 futures-util = { version = "0.3.17", default-features = false, features = ["alloc"] }
 multer = "3"
 tokio = { version = "1.24.2", features = ["sync"] }

actix-multipart/README.md

@@ -54,15 +54,15 @@ async fn main() -> std::io::Result<()> {
 }
 ```
 
-<!-- cargo-rdme end -->
-
-[More available in the examples repo &rarr;](https://github.com/actix/examples/tree/master/forms/multipart)
-
-Curl request :
+cURL request:
 
-```bash
+```sh
 curl -v --request POST \
   --url http://localhost:8080/videos \
   -F 'json={"name": "Cargo.lock"};type=application/json' \
   -F file=@./Cargo.lock
 ```
+
+<!-- cargo-rdme end -->
+
+[More available in the examples repo &rarr;](https://github.com/actix/examples/tree/master/forms/multipart)

actix-multipart/examples/form.rs

@@ -0,0 +1,36 @@
+use actix_multipart::form::{json::Json as MpJson, tempfile::TempFile, MultipartForm};
+use actix_web::{middleware::Logger, post, App, HttpServer, Responder};
+use serde::Deserialize;
+
+#[derive(Debug, Deserialize)]
+struct Metadata {
+    name: String,
+}
+
+#[derive(Debug, MultipartForm)]
+struct UploadForm {
+    #[multipart(limit = "100MB")]
+    file: TempFile,
+    json: MpJson<Metadata>,
+}
+
+#[post("/videos")]
+async fn post_video(MultipartForm(form): MultipartForm<UploadForm>) -> impl Responder {
+    format!(
+        "Uploaded file {}, with size: {}\ntemporary file ({}) was deleted\n",
+        form.json.name,
+        form.file.size,
+        form.file.file.path().display(),
+    )
+}
+
+#[actix_web::main]
+async fn main() -> std::io::Result<()> {
+    env_logger::init_from_env(env_logger::Env::new().default_filter_or("info"));
+
+    HttpServer::new(move || App::new().service(post_video).wrap(Logger::default()))
+        .workers(2)
+        .bind(("127.0.0.1", 8080))?
+        .run()
+        .await
+}

actix-multipart/src/error.rs

@@ -11,77 +11,95 @@ use derive_more::{Display, Error, From};
 #[derive(Debug, Display, From, Error)]
 #[non_exhaustive]
 pub enum MultipartError {
-    /// Content-Disposition header is not found or is not equal to "form-data".
-    ///
-    /// According to [RFC 7578 §4.2](https://datatracker.ietf.org/doc/html/rfc7578#section-4.2) a
-    /// Content-Disposition header must always be present and equal to "form-data".
-    #[display(fmt = "No Content-Disposition `form-data` header")]
-    NoContentDisposition,
+    /// Could not find Content-Type header.
+    #[display(fmt = "Could not find Content-Type header")]
+    ContentTypeMissing,
 
-    /// Content-Type header is not found
-    #[display(fmt = "No Content-Type header found")]
-    NoContentType,
+    /// Could not parse Content-Type header.
+    #[display(fmt = "Could not parse Content-Type header")]
+    ContentTypeParse,
 
-    /// Can not parse Content-Type header
-    #[display(fmt = "Can not parse Content-Type header")]
-    ParseContentType,
+    /// Parsed Content-Type did not have "multipart" top-level media type.
+    ///
+    /// Also raised when extracting a [`MultipartForm`] from a request that does not have the
+    /// "multipart/form-data" media type.
+    ///
+    /// [`MultipartForm`]: struct@crate::form::MultipartForm
+    #[display(fmt = "Parsed Content-Type did not have "multipart" top-level media type")]
+    ContentTypeIncompatible,
 
-    /// Multipart boundary is not found
+    /// Multipart boundary is not found.
     #[display(fmt = "Multipart boundary is not found")]
-    Boundary,
+    BoundaryMissing,
+
+    /// Content-Disposition header was not found or not of disposition type "form-data" when parsing
+    /// a "form-data" field.
+    ///
+    /// As per [RFC 7578 §4.2], a "multipart/form-data" field's Content-Disposition header must
+    /// always be present and have a disposition type of "form-data".
+    ///
+    /// [RFC 7578 §4.2]: https://datatracker.ietf.org/doc/html/rfc7578#section-4.2
+    #[display(fmt = "Content-Disposition header was not found when parsing a \"form-data\" field")]
+    ContentDispositionMissing,
+
+    /// Content-Disposition name parameter was not found when parsing a "form-data" field.
+    ///
+    /// As per [RFC 7578 §4.2], a "multipart/form-data" field's Content-Disposition header must
+    /// always include a "name" parameter.
+    ///
+    /// [RFC 7578 §4.2]: https://datatracker.ietf.org/doc/html/rfc7578#section-4.2
+    #[display(fmt = "Content-Disposition header was not found when parsing a \"form-data\" field")]
+    ContentDispositionNameMissing,
 
-    /// Nested multipart is not supported
+    /// Nested multipart is not supported.
     #[display(fmt = "Nested multipart is not supported")]
     Nested,
 
-    /// Multipart stream is incomplete
+    /// Multipart stream is incomplete.
     #[display(fmt = "Multipart stream is incomplete")]
     Incomplete,
 
-    /// Error during field parsing
-    #[display(fmt = "{}", _0)]
+    /// Field parsing failed.
+    #[display(fmt = "Error during field parsing")]
     Parse(ParseError),
 
-    /// Payload error
-    #[display(fmt = "{}", _0)]
+    /// HTTP payload error.
+    #[display(fmt = "Payload error")]
     Payload(PayloadError),
 
-    /// Not consumed
-    #[display(fmt = "Multipart stream is not consumed")]
+    /// Stream is not consumed.
+    #[display(fmt = "Stream is not consumed")]
     NotConsumed,
 
-    /// An error from a field handler in a form
-    #[display(
-        fmt = "An error occurred processing field `{}`: {}",
-        field_name,
-        source
-    )]
+    /// Form field handler raised error.
+    #[display(fmt = "An error occurred processing field: {name}")]
     Field {
-        field_name: String,
+        name: String,
         source: actix_web::Error,
     },
 
-    /// Duplicate field
-    #[display(fmt = "Duplicate field found for: `{}`", _0)]
+    /// Duplicate field found (for structure that opted-in to denying duplicate fields).
+    #[display(fmt = "Duplicate field found: {_0}")]
     #[from(ignore)]
     DuplicateField(#[error(not(source))] String),
 
-    /// Missing field
-    #[display(fmt = "Field with name `{}` is required", _0)]
+    /// Required field is missing.
+    #[display(fmt = "Required field is missing: {_0}")]
     #[from(ignore)]
     MissingField(#[error(not(source))] String),
 
-    /// Unknown field
-    #[display(fmt = "Unsupported field `{}`", _0)]
+    /// Unknown field (for structure that opted-in to denying unknown fields).
+    #[display(fmt = "Unknown field: {_0}")]
     #[from(ignore)]
-    UnsupportedField(#[error(not(source))] String),
+    UnknownField(#[error(not(source))] String),
 }
 
-/// Return `BadRequest` for `MultipartError`
+/// Return `BadRequest` for `MultipartError`.
 impl ResponseError for MultipartError {
     fn status_code(&self) -> StatusCode {
         match &self {
             MultipartError::Field { source, .. } => source.as_response_error().status_code(),
+            MultipartError::ContentTypeIncompatible => StatusCode::UNSUPPORTED_MEDIA_TYPE,
             _ => StatusCode::BAD_REQUEST,
         }
     }
@@ -93,7 +111,7 @@ mod tests {
 
     #[test]
     fn test_multipart_error() {
-        let resp = MultipartError::Boundary.error_response();
+        let resp = MultipartError::BoundaryMissing.error_response();
         assert_eq!(resp.status(), StatusCode::BAD_REQUEST);
     }
 }

actix-multipart/src/extractor.rs

@@ -10,6 +10,7 @@ use crate::server::Multipart;
 /// Content-type: multipart/form-data;
 ///
 /// # Examples
+///
 /// ```
 /// use actix_web::{web, HttpResponse, Error};
 /// use actix_multipart::Multipart;
@@ -35,9 +36,6 @@ impl FromRequest for Multipart {
 
     #[inline]
     fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future {
-        ready(Ok(match Multipart::boundary(req.headers()) {
-            Ok(boundary) => Multipart::from_boundary(boundary, payload.take()),
-            Err(err) => Multipart::from_error(err),
-        }))
+        ready(Ok(Multipart::from_req(req, payload)))
     }
 }

actix-multipart/src/form/bytes.rs

@@ -41,8 +41,9 @@ impl<'t> FieldReader<'t> for Bytes {
                 content_type: field.content_type().map(ToOwned::to_owned),
                 file_name: field
                     .content_disposition()
+                    .expect("multipart form fields should have a content-disposition header")
                     .get_filename()
-                    .map(str::to_owned),
+                    .map(ToOwned::to_owned),
             })
         })
     }

actix-multipart/src/form/json.rs

@@ -32,7 +32,6 @@ where
     fn read_field(req: &'t HttpRequest, field: Field, limits: &'t mut Limits) -> Self::Future {
         Box::pin(async move {
             let config = JsonConfig::from_req(req);
-            let field_name = field.name().to_owned();
 
             if config.validate_content_type {
                 let valid = if let Some(mime) = field.content_type() {
@@ -43,17 +42,19 @@ where
 
                 if !valid {
                     return Err(MultipartError::Field {
-                        field_name,
+                        name: field.form_field_name,
                         source: config.map_error(req, JsonFieldError::ContentType),
                     });
                 }
             }
 
+            let form_field_name = field.form_field_name.clone();
+
             let bytes = Bytes::read_field(req, field, limits).await?;
 
             Ok(Json(serde_json::from_slice(bytes.data.as_ref()).map_err(
                 |err| MultipartError::Field {
-                    field_name,
+                    name: form_field_name,
                     source: config.map_error(req, JsonFieldError::Deserialize(err)),
                 },
             )?))