Repository: seanmonstar/warp (language: rust)

Issue:
Clarify API docs around multipart forms
**Is your feature request related to a problem? Please describe.**

I'm trying to get started using the `warp::multipart::FormData` API, however I have some confusion in lining up the documentation and the examples.

The [example code](https://github.com/seanmonstar/warp/blob/da47391afe7ede0d0efc671ca9e29160deec7917/examples/multipart.rs#L9) looks like this:

```
warp::multipart::form().and_then(|form: FormData| async move {
        let field_names: Vec<_> = form
            .and_then(|mut field| async move {
                let mut bytes: Vec<u8> = Vec::new();

                // field.data() only returns a piece of the content, you should call over it until it replies None
                while let Some(content) = field.data().await {
                    let content = content.unwrap();
                    let chunk: &[u8] = content.chunk();
                    bytes.extend_from_slice(chunk);
                }
                Ok((
                    field.name().to_string(),
                    field.filename().unwrap().to_string(),
                    String::from_utf8_lossy(&*bytes).to_string(),
                ))
            })
            .try_collect()
            .await
            .unwrap();
```

Here for each field [`warp::multipart::Part`](https://docs.rs/warp/latest/warp/filters/multipart/struct.Part.html), we call `field.data().await` in a loop, which has documented signature:

pub async fn [data](https://docs.rs/warp/latest/warp/filters/multipart/struct.Part.html#method.data)(&mut self) -> [Option](https://doc.rust-lang.org/nightly/core/option/enum.Option.html)<[Result](https://doc.rust-lang.org/nightly/core/result/enum.Result.html)<impl [Buf](https://docs.rs/bytes/1.1.0/x86_64-unknown-linux-gnu/bytes/buf/buf_impl/trait.Buf.html), [Error](https://docs.rs/warp/latest/warp/struct.Error.html)>>

This `impl Buf` object is called `content` in the example code, and we call `content.chunk()` and append that to our buffer to accumulate the data.

However, when I look at documentation for [`Buf::chunk`](https://docs.rs/bytes/1.1.0/bytes/buf/trait.Buf.html#tymethod.chunk), I get worried about this code.

> Returns a slice starting at the current position and of length between 0 and Buf::remaining(). Note that this can return shorter slice (this allows non-continuous internal representation).
>
> This is a lower level function. Most operations are done with other functions.

So, this worries me that by only calling `content.chunk()` once, and not having another loop inside there, I may not actually be storing all the content, and I may be dropping bytes and end up with a corrupted file upload. The documentation also suggests that this is a lower level function that I should probably not be calling.

If for example, I was reviewing a PR that added this functionality to my own codebase, I would say the example code is broken, because the warp library is documenting only that the return type is `impl Buf`, presumably so that they have the freedom to change to some more complicated discontiguous `impl Buf` object later, but our code would be broken when that happens since we only call chunk once.

While warp is documenting that they are returning `impl Buf` here, in reality the library is always returning `Bytes` (the tokio-rs Bytes type) at current revisions: https://github.com/seanmonstar/warp/blob/da47391afe7ede0d0efc671ca9e29160deec7917/src/filters/multipart.rs#L164

The tokio bytes type is not discontiguous, and so calling `.chunk()` will always work, and this example code will work at current revisions.

**Describe the solution you'd like**

The solution I'd like is, document that the return type of `Part::data()` is `Bytes` rather than `impl Buf`. Then the example code is correct, and none of your current users are broken or have to change anything, and no one has to do two rounds of looping for aggregating discontinuous bytes (once for calling `.data()`, and once for calling `.chunk()` repeatedly on that data).

**Describe alternatives you've considered**

The main drawback of this is that if you commit to `Bytes` now, you can't change to a discontiguous `impl Buf` later without a breaking change to the API.

However, since the example code basically assumed it was `Bytes` anyways, presumably in real life a lot of users just copied the example code and would be broken if you changed the implementation in this way. 

An alternative solution is, change the example code to use `content.reader().read_to_end()` or something like this, and maybe document in release notes that the previous example code is considered invalid.

**Additional context**

Right now, if you already have the implementation working with `Bytes`, even when the content may come to the webserver in several pieces, then `warp` is already doing the copies and the work to make the content continuous, so I don't see what's gained by backing out of that. But you may see it differently.


Discussion / hints:
Wow, thanks for the catch, good call! I'd say we could just `use bytes::BufMut`, and then could call `bytes.put(content)`.
I guess that's what I will do for now. I will leave it to you to decide whether to just update example code or change the API so that the example code that people probably copied is promised to be valid.

Thanks!

Fix patch:
diff --git a/examples/multipart.rs b/examples/multipart.rs
index 43215b165..e28dd01de 100644
--- a/examples/multipart.rs
+++ b/examples/multipart.rs
@@ -1,6 +1,6 @@
+use bytes::BufMut;
 use futures_util::TryStreamExt;
 use warp::multipart::FormData;
-use warp::Buf;
 use warp::Filter;
 
 #[tokio::main]
@@ -14,8 +14,7 @@ async fn main() {
                 // field.data() only returns a piece of the content, you should call over it until it replies None
                 while let Some(content) = field.data().await {
                     let content = content.unwrap();
-                    let chunk: &[u8] = content.chunk();
-                    bytes.extend_from_slice(chunk);
+                    bytes.put(content);
                 }
                 Ok((
                     field.name().to_string(),


Test patch: