-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support per-endpoint request size limits #380
Conversation
Generate changelog in
|
@@ -0,0 +1,50 @@ | |||
pub fn parse(s: &str) -> Result<usize, String> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this based on an equivalent java class where we are trying to maintain the same logic? Or is there some crate we could use that handles this so we're not worried about edge cases?
The logic here seems fine to be but I'm a bit paranoid that there's some edge case we've missed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the Java version is here: https://github.com/palantir/human-readable-types/blob/develop/human-readable-types/src/main/java/com/palantir/humanreadabletypes/HumanReadableByteCount.java. It should be the same except the Rust version doesn't currently handle byte
, mebibyte
, etc since I never see those used in practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't found an existing crate that handles this parsing unfortunately.
Before this PR
All endpoints with serializable request bodies limited them to 50MiB without the ability to configure that.
After this PR
==COMMIT_MSG==
Added support for per-endpoint request size limits via the
server-limit-request-size: N
tag.==COMMIT_MSG==
There's a bit of jank in the codegen since up until now it's been totally infallible. To work around this, we inject
compile_error!
macro calls when the configured limit fails to parse properly. In the Java implementation, this parsing is deferred to runtime.The limit is passed as a const generic parameter in the
StdRequestDeserializer
. I'm not sure if this is the "right" approach, but it lets us preserve back compat. Custom endpoints can set the parameter directly, which is workable but a bit weird. In the future it might make sense for the limit to be a more first-class concept in some way.Closes #374