From 6904889488cd1385a6973e8ebbd25168e4f50e56 Mon Sep 17 00:00:00 2001 From: Eshed Schacham Date: Thu, 14 Mar 2024 22:04:34 +0200 Subject: [PATCH] (wasm) fix: Form::text on wasm setting octet-stream mime type and file name. (#2174) Unfortunately JS's fetch API is somewhat limited and only supports two options for multipart form data parts: 1) String with no file name and no mime type (which should be interpreted as text/plain). 2) Blob with a file name ("blob" if not provided by the user) and a mime type if provided by the user. Until this commit, reqwest always used the latter option, so when a user tried to add a text part with no file name, it would be sent with a mime type of `application/octet-stream` and the filename "blob". While we can't make the behaviour identical to the native implementation, we can do a best-effort approach, using option (1) as long as the user hasn't set a file name or a non plain text mime type. --- src/wasm/body.rs | 101 ++++++++++++++++++++++++------------------ src/wasm/multipart.rs | 78 +++++++++++++++++++++++++------- 2 files changed, 121 insertions(+), 58 deletions(-) diff --git a/src/wasm/body.rs b/src/wasm/body.rs index dc38ed8ff..751cf586e 100644 --- a/src/wasm/body.rs +++ b/src/wasm/body.rs @@ -3,7 +3,7 @@ use super::multipart::Form; /// dox use bytes::Bytes; use js_sys::Uint8Array; -use std::fmt; +use std::{borrow::Cow, fmt}; use wasm_bindgen::JsValue; /// The body of a `Request`. @@ -18,13 +18,44 @@ pub struct Body { } enum Inner { - Bytes(Bytes), + Single(Single), /// MultipartForm holds a multipart/form-data body. #[cfg(feature = "multipart")] MultipartForm(Form), - /// MultipartPart holds the body of a multipart/form-data part. - #[cfg(feature = "multipart")] - MultipartPart(Bytes), +} + +#[derive(Clone)] +pub(crate) enum Single { + Bytes(Bytes), + Text(Cow<'static, str>), +} + +impl Single { + fn as_bytes(&self) -> &[u8] { + match self { + Single::Bytes(bytes) => bytes.as_ref(), + Single::Text(text) => text.as_bytes(), + } + } + + pub(crate) fn to_js_value(&self) -> JsValue { + match self { + Single::Bytes(bytes) => { + let body_bytes: &[u8] = bytes.as_ref(); + let body_uint8_array: Uint8Array = body_bytes.into(); + let js_value: &JsValue = body_uint8_array.as_ref(); + js_value.to_owned() + } + Single::Text(text) => JsValue::from_str(text), + } + } + + fn is_empty(&self) -> bool { + match self { + Single::Bytes(bytes) => bytes.is_empty(), + Single::Text(text) => text.is_empty(), + } + } } impl Body { @@ -34,36 +65,29 @@ impl Body { #[inline] pub fn as_bytes(&self) -> Option<&[u8]> { match &self.inner { - Inner::Bytes(bytes) => Some(bytes.as_ref()), + Inner::Single(single) => Some(single.as_bytes()), #[cfg(feature = "multipart")] Inner::MultipartForm(_) => None, - #[cfg(feature = "multipart")] - Inner::MultipartPart(bytes) => Some(bytes.as_ref()), } } + pub(crate) fn to_js_value(&self) -> crate::Result { match &self.inner { - Inner::Bytes(body_bytes) => { - let body_bytes: &[u8] = body_bytes.as_ref(); - let body_uint8_array: Uint8Array = body_bytes.into(); - let js_value: &JsValue = body_uint8_array.as_ref(); - Ok(js_value.to_owned()) - } + Inner::Single(single) => Ok(single.to_js_value()), #[cfg(feature = "multipart")] Inner::MultipartForm(form) => { let form_data = form.to_form_data()?; let js_value: &JsValue = form_data.as_ref(); Ok(js_value.to_owned()) } - #[cfg(feature = "multipart")] - Inner::MultipartPart(body_bytes) => { - let body_bytes: &[u8] = body_bytes.as_ref(); - let body_uint8_array: Uint8Array = body_bytes.into(); - let body_array = js_sys::Array::new(); - body_array.push(&body_uint8_array); - let js_value: &JsValue = body_array.as_ref(); - Ok(js_value.to_owned()) - } + } + } + + #[cfg(feature = "multipart")] + pub(crate) fn as_single(&self) -> Option<&Single> { + match &self.inner { + Inner::Single(single) => Some(single), + Inner::MultipartForm(_) => None, } } @@ -79,39 +103,30 @@ impl Body { #[cfg(feature = "multipart")] pub(crate) fn into_part(self) -> Body { match self.inner { - Inner::Bytes(bytes) => Self { - inner: Inner::MultipartPart(bytes), + Inner::Single(single) => Self { + inner: Inner::Single(single), }, Inner::MultipartForm(form) => Self { inner: Inner::MultipartForm(form), }, - Inner::MultipartPart(bytes) => Self { - inner: Inner::MultipartPart(bytes), - }, } } pub(crate) fn is_empty(&self) -> bool { match &self.inner { - Inner::Bytes(bytes) => bytes.is_empty(), + Inner::Single(single) => single.is_empty(), #[cfg(feature = "multipart")] Inner::MultipartForm(form) => form.is_empty(), - #[cfg(feature = "multipart")] - Inner::MultipartPart(bytes) => bytes.is_empty(), } } pub(crate) fn try_clone(&self) -> Option { match &self.inner { - Inner::Bytes(bytes) => Some(Self { - inner: Inner::Bytes(bytes.clone()), + Inner::Single(single) => Some(Self { + inner: Inner::Single(single.clone()), }), #[cfg(feature = "multipart")] Inner::MultipartForm(_) => None, - #[cfg(feature = "multipart")] - Inner::MultipartPart(bytes) => Some(Self { - inner: Inner::MultipartPart(bytes.clone()), - }), } } } @@ -120,7 +135,7 @@ impl From for Body { #[inline] fn from(bytes: Bytes) -> Body { Body { - inner: Inner::Bytes(bytes), + inner: Inner::Single(Single::Bytes(bytes)), } } } @@ -129,7 +144,7 @@ impl From> for Body { #[inline] fn from(vec: Vec) -> Body { Body { - inner: Inner::Bytes(vec.into()), + inner: Inner::Single(Single::Bytes(vec.into())), } } } @@ -138,7 +153,7 @@ impl From<&'static [u8]> for Body { #[inline] fn from(s: &'static [u8]) -> Body { Body { - inner: Inner::Bytes(Bytes::from_static(s)), + inner: Inner::Single(Single::Bytes(Bytes::from_static(s))), } } } @@ -147,7 +162,7 @@ impl From for Body { #[inline] fn from(s: String) -> Body { Body { - inner: Inner::Bytes(s.into()), + inner: Inner::Single(Single::Text(s.into())), } } } @@ -155,7 +170,9 @@ impl From for Body { impl From<&'static str> for Body { #[inline] fn from(s: &'static str) -> Body { - s.as_bytes().into() + Body { + inner: Inner::Single(Single::Text(s.into())), + } } } diff --git a/src/wasm/multipart.rs b/src/wasm/multipart.rs index f676784d2..9b5b4c951 100644 --- a/src/wasm/multipart.rs +++ b/src/wasm/multipart.rs @@ -95,15 +95,9 @@ impl Form { .map_err(crate::error::builder)?; for (name, part) in self.inner.fields.iter() { - let blob = part.blob()?; - - if let Some(file_name) = &part.metadata().file_name { - form.append_with_blob_and_filename(name, &blob, &file_name) - } else { - form.append_with_blob(name, &blob) - } - .map_err(crate::error::wasm) - .map_err(crate::error::builder)?; + part.append_to_form(name, &form) + .map_err(crate::error::wasm) + .map_err(crate::error::builder)?; } Ok(form) } @@ -187,18 +181,58 @@ impl Part { } } - fn blob(&self) -> crate::Result { + fn append_to_form( + &self, + name: &str, + form: &web_sys::FormData, + ) -> Result<(), wasm_bindgen::JsValue> { + let single = self + .value + .as_single() + .expect("A part's body can't be multipart itself"); + + let mut mime_type = self.metadata().mime.as_ref(); + + // The JS fetch API doesn't support file names and mime types for strings. So we do our best + // effort to use `append_with_str` and fallback to `append_with_blob_*` if that's not + // possible. + if let super::body::Single::Text(text) = single { + if mime_type.is_none() || mime_type == Some(&mime_guess::mime::TEXT_PLAIN) { + if self.metadata().file_name.is_none() { + return form.append_with_str(name, text); + } + } else { + mime_type = Some(&mime_guess::mime::TEXT_PLAIN); + } + } + + let blob = self.blob(mime_type)?; + + if let Some(file_name) = &self.metadata().file_name { + form.append_with_blob_and_filename(name, &blob, file_name) + } else { + form.append_with_blob(name, &blob) + } + } + + fn blob(&self, mime_type: Option<&Mime>) -> crate::Result { use web_sys::Blob; use web_sys::BlobPropertyBag; let mut properties = BlobPropertyBag::new(); - if let Some(mime) = &self.meta.mime { + if let Some(mime) = mime_type { properties.type_(mime.as_ref()); } - // BUG: the return value of to_js_value() is not valid if - // it is a MultipartForm variant. - let js_value = self.value.to_js_value()?; - Blob::new_with_u8_array_sequence_and_options(&js_value, &properties) + let js_value = self + .value + .as_single() + .expect("A part's body can't be set to a multipart body") + .to_js_value(); + + let body_array = js_sys::Array::new(); + body_array.push(&js_value); + + Blob::new_with_u8_array_sequence_and_options(body_array.as_ref(), &properties) .map_err(crate::error::wasm) .map_err(crate::error::builder) } @@ -319,11 +353,16 @@ mod tests { .mime_str(binary_file_type) .expect("invalid mime type"); + let string_name = "string"; + let string_content = "CONTENT"; + let string_part = Part::text(string_content); + let text_name = "text part"; let binary_name = "binary part"; let form = Form::new() .part(text_name, text_part) - .part(binary_name, binary_part); + .part(binary_name, binary_part) + .part(string_name, string_part); let mut init = web_sys::RequestInit::new(); init.method("POST"); @@ -361,6 +400,13 @@ mod tests { assert_eq!(binary_file.name(), binary_file_name); assert_eq!(binary_file.type_(), binary_file_type); + // check string part + let string = form_data + .get(string_name) + .as_string() + .expect("content is not a string"); + assert_eq!(string, string_content); + let binary_array_buffer_promise = binary_file.array_buffer(); let array_buffer = crate::wasm::promise::(binary_array_buffer_promise) .await