Add Bytes DataGuard #2362
Add Bytes DataGuard #2362
Conversation
| content_type: Option<ContentType>, | ||
| ) -> io::Result<Capped<Bytes<'a>>> { | ||
| let limit = { content_type.as_ref() } | ||
| .and_then(|ct| ct.extension()?.limits().find(&["file", ext.as_str()])) |
There was a problem hiding this comment.
I think there's a mistake here, shouldn't be
.and_then(|ct| ct.extension())
.and_then(|ext| req.limits().find(&["file", ext.as_str()]))?
There was a problem hiding this comment.
Sorry for the delay, been busy working.
There is no need for a second .and_then(...) because the ? after .extension() extracts the value from Option::Some(T) or returns Option::None depending on which variant it happened to be, allowing for more concise code.
Apparently it was stabilized back in 2016, feels newer. There's a section in the book about it.
| pub struct Bytes<'v> { | ||
| pub file_name: Option<&'v FileName>, | ||
| pub content_type: Option<ContentType>, | ||
| pub content: Cow<'v, [u8]>, |
There was a problem hiding this comment.
Just wondering, is there a reason for using Cow<[u8]> here instead of Vec<u8>? As far as I can tell converting a Cow<[T]> back into a Vec<T> would cause a reallocation of the entire file even if it's owned and I would rather have the Vec for my use-case.
There was a problem hiding this comment.
I'm finally done with having to work so much, so hopefully I'll be more responsive now.
On line 52, I use it for the borrowed variant, so there is a purpose in using it and there should not be an unnecessary (re)allocation because:
For extra clarification, the type of content doesn't mention Vec because it is inferred from the ToOwned implementation for slices that specifies its Owned variant is Vec.
There was a problem hiding this comment.
This implementation should just be delegating to the existing &[u8] and Vec<u8> FromData implementations. See 569bc09 for how this was done for the &[u8] FromForm implementation.
|
Closing due to inactivity (apologies as I know it's largely on me), though I'm entirely open to this PR with the requested changed above. |
This will hopefully be sufficient for most of those concerned with issue #2148.
I might have gone the route of #2361 (implementing for
&[u8]), but some people may want to get the filename and content-type as well, though I think that PR is still nice to have if all you need is the data.