MultipartData is not composable due to lifetime restrictions about multipart HOT 16 CLOSED

I've published 0.10.0-alpha.2 which addresses the hang and I think the problem with save_to_limited() as well. If the latter is still an issue, can I see the part of the file or stream that's corrupted, and also logs for multipart=trace. Otherwise I'm just left guessing. I'm working out how to add file saving to the integration test in a way that's reproducible.

In the process of fixing the hang I also added the entry API to local_test.rs in combination with both the lazy and the regular client APIs.

from multipart.

That's right. That's the "abstraction" part.

from multipart.

Do you mind showing me what you're trying to do? The borrowing has generally not been an issue before, and I'm hesitant to make such a sweeping change for one framework. There is Multipart::save_all() which provides owned copies of all fields (saving streams to temporary files). However, I'm wondering if there's not otherwise a way to adapt what you're trying to do to the existing API.

from multipart.

I mentioned my use case in the original post:

call a function that creates a Multipart structure, reads a MultipartData entry from a stream, and returns a MultipartFile.

This isn't about changing your library to work with one framework, this is about ensuring your library can meet any applicable use case. At present, you're imposing a rather strong restriction; your library is literally impossible to use under certain scenarios, and the restriction is unnecessary. As far as I can tell, your library is the most complete one of its kind, and the only one that doesn't parse eagerly. It would be a shame if there needed to exist yet another library to meet a use case this one can easily meet.

from multipart.

Does the save_all() method work or do you find going to the filesystem undesirable? It doesn't use the filenames supplied in the request.

from multipart.

I don't want to save to the file system unnecessarily. I want the caller of the function to determine what to do with the file stream. For instance, it should be trivial to gzip the stream without copying the contents to the file system unnecessarily. I want a stream end-to-end.

from multipart.

And allowing the user to consume the Multipart directly isn't acceptable?

from multipart.

Following up on this. Have you given this any more thought, @abonander?

from multipart.

I have some ideas on how to make this work without breaking existing usage. I'll get back to you.

from multipart.

@SergioBenitez

• https://docs.rs/multipart/0.10.0-alpha.1/multipart/server/struct.Multipart.html#method.into_entry

• https://docs.rs/multipart/0.10.0-alpha.1/multipart/server/struct.MultipartField.html#method.next_entry

• https://docs.rs/multipart/0.10.0-alpha.1/multipart/server/struct.MultipartField.html#method.next_entry_inplace

from multipart.

I ended up making the multipart: M fields private because I also wanted to implement nested multipart fields (e.g. a file field with more than one file selected) and the way I wanted that to work necessitated more control over the inner Multipart. The changes are on 0.10 branch, I have a couple more changes to make before I publish another alpha.

from multipart.

Awesome! This looks like it'll do what I need.

Am completely inundated with work for the next week or so but will play with this afterwards.

from multipart.

Finally had a chance to give this a try. While the API seems like it will work (though I do find it a bit clumsy to work with, though maybe that's unavoidable), I've hit a bug. Here's what I'm doing, in pseudocode:

let multipart = Multipart::with_body(body, boundary);
let mut field = multipart.into_entry();
let outcome = if let Some(file) = field.data.as_file() {
    let mut temp_file = tempfile();
    file.save_to_limited(&mut temp_file, 1 << 20).expect("ok.");
    Success(temp_file)
} else {
    Failure
};

// There should only be one field.
match field.next_entry() {
    Ok(None) => { outcome }
    Ok(Some(_)) => { Failure }
    Err(e) => { Failure }
}

The issue is that the program hangs on the call to next_entry(). My guess is that there's something wrong with whatever is reading from the body. This happens every time, so this should be easy to add a test for (re: #59).

Another bug is that calling save_to_limited with a limit that is in-fact enforced (IE, the file is larger than the limit) results in parsing the next entry incorrectly. This is contrary to what the documentation says: "If the previously returned entry had contents of type MultipartField::File, calling this again will discard any unread contents of that entry."

from multipart.

@abonander Have you had a chance to mull over my last comment? This is a showstopper.

from multipart.

I'm working on it.

from multipart.

Awesome! Thanks. :)

from multipart.