Add layer to validate requests #289
Conversation
|
@davidpdrsn @Nehliin thanks for your comments! I've updated the PR |
|
CI sometimes fails on
|
tower-http/src/validate_request.rs
Outdated
|
|
||
| /// Type that performs validation of the Accept header. | ||
| pub struct AcceptHeader<ResBody> { | ||
| header_value: Mime, |
There was a problem hiding this comment.
I think we should use Arc<Mime> since Mime might contain allocated collections like String and Vec. Just as a small perf optimization.
| header_value: Mime, | |
| header_value: Arc<Mime>, |
There was a problem hiding this comment.
Ok, thanks. What kind of performance gains does this achieve?
There was a problem hiding this comment.
Services tend to be cloned a lot. So wrapping this in an Arc makes it cheaper to clone.
| .to_str() | ||
| .ok() | ||
| .into_iter() | ||
| .flat_map(|s| s.split(",").map(|typ| typ.trim())) |
There was a problem hiding this comment.
Worth noting that this is incorrect for the same reason as this issue http-rs/http-types#349
There was a problem hiding this comment.
Oh thanks for catching that! Would you mind opening an issue?
There was a problem hiding this comment.
I might not be the best at doing this — I don't really know Rust and just stumbled on this when giving a colleague advice on how to port something from a Node project to their Rust project. But I can if you would prefer an issue from somebody who doesn't actually directly use your project or know Rust :)
There was a problem hiding this comment.
Alright. I've opened #291. Thanks for reporting this.
This commit adds a new layer to customize validating requests.
Motivation
The new layer adds support for custom validation of requests.
In smithy-rs, we allow requests based on the
Acceptheader. Our use-case could be common to others using tower and more general than just validating a header, hence this layer to validate any request, with headers being a special case.Solution
The new layer takes from the
RequireAuthorizationlayer and generalizes it for any request. If this is accepted, we can have the authorization layer be like theAcceptHeaderlayer and use this general validator.Alternatively, we could have a layer to validate headers specifically or only for the Accept header.