rwf2 / multer

An async parser for multipart/form-data content-type in Rust
MIT License
158 stars 37 forks source link

Update to http 1.0 #59

Closed davidpdrsn closed 10 months ago

davidpdrsn commented 10 months ago

No changes needed in the code but I updated the example to hyper 1.0 as well which required changes.

This is required for axum to support hyper 1.0.

SergioBenitez commented 10 months ago

Please see the build failures.

davidpdrsn commented 10 months ago

@SergioBenitez I believe the errors should be fixed now.

davidpdrsn commented 10 months ago

@SergioBenitez do you have time to take a look at this? Its the only thing blocking the next major release of axum.

SergioBenitez commented 10 months ago

Can you squash into a single commit?

davidpdrsn commented 10 months ago

Done!

SergioBenitez commented 10 months ago

Ah, I see now that we're exposing types from http. This is rather unfortunate as it means we need to do a major version bump on multer. Unfortunately this means I'll need to do a bit more thorough of a review before committing to a 3.0.

Is this completely blocking you? Are you interfacing through the HeaderMap, and if so, would it be acceptable to convert the old map to the new map in the interim?

davidpdrsn commented 10 months ago

Is this completely blocking you? Are you interfacing through the HeaderMap, and if so, would it be acceptable to convert the old map to the new map in the interim?

Not completely. The context is that we have a Field type which wraps multer::Field. Our Field has a headers method which returns &http::HeaderMap by simply delegating to the underlying multer::Field. So that would expose a HeaderMap from http 0.2. We can't convert the headers in our headers method because it must return a &HeaderMap that's borrowed from self.

I think our best option probably is to convert the headers eagerly when we construct our wrapper Field. That will hurt performance a bit but I'd be okay with that since it's just temporary. I wouldn't wanna remove axum's multipart handling (to then add it back in a point release) or publish a fork of multer.

SergioBenitez commented 10 months ago

Got it, okay, that's great. I've moved this up on my priority list. I should be able to get to it fairly soon. Will keep you posted.

davidpdrsn commented 10 months ago

Thanks!