Skip to content

Commit

Permalink
[5/n] [dropshot] add support for per-endpoint size limits (#1171)
Browse files Browse the repository at this point in the history
For some endpoints like uploading update and support bundles, we would like to
have a larger request limit. Add support for that.

This is the other half of RFD 353, after #617. (A previous attempt was #618,
which I've abandoned.)

The PR consists of:

* The implementation.
* A small tweak to the API generation code for better span attribution.
* Tests.
  • Loading branch information
sunshowers authored Nov 21, 2024
1 parent 05af62e commit d5874eb
Show file tree
Hide file tree
Showing 19 changed files with 559 additions and 38 deletions.
20 changes: 20 additions & 0 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,26 @@ Defining the old config option will produce an error, guiding you to perform the
** `rqctx.path_variables` is now `rqctx.endpoint.variables`.
** `rqctx.body_content_type` is now `rqctx.endpoint.body_content_type`.

=== Other notable changes

* Dropshot now supports per-endpoint size limits, via the `request_body_max_bytes` parameter to `#[endpoint]`. For example, to set a limit of 1 MiB on an endpoint:
+
```rust
#[endpoint {
method = POST,
path = "/upload-bundle",
request_body_max_bytes = 1 * 1024 * 1024,
}]
async fn upload_bundle(
rqctx: RequestContext<MyContext>, // or RequestContext<Self::Context> with API traits
body: UntypedBody,
) -> /* ... */ {
// ...
}
```
+
If not specified, the limit defaults to the server configuration's `default_request_body_max_bytes`.

== 0.13.0 (released 2024-11-13)

https://github.com/oxidecomputer/dropshot/compare/v0.12.0\...v0.13.0[Full list of commits]
Expand Down
2 changes: 2 additions & 0 deletions README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ include:
|No
|Specifies the maximum number of bytes allowed in a request body. Larger requests will receive a 400 error. Defaults to 1024.

Can be overridden per-endpoint via the `request_body_max_bytes` parameter to `#[endpoint { ... }]`.

|`tls.type`
|`"AsFile"`
|No
Expand Down
11 changes: 11 additions & 0 deletions dropshot/src/api_description.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ pub struct ApiEndpoint<Context: ServerContext> {
pub path: String,
pub parameters: Vec<ApiEndpointParameter>,
pub body_content_type: ApiEndpointBodyContentType,
/// An override for the maximum allowed size of the request body.
///
/// `None` means that the server default is used.
pub request_body_max_bytes: Option<usize>,
pub response: ApiEndpointResponse,
pub summary: Option<String>,
pub description: Option<String>,
Expand Down Expand Up @@ -88,6 +92,7 @@ impl<'a, Context: ServerContext> ApiEndpoint<Context> {
path: path.to_string(),
parameters: func_parameters.parameters,
body_content_type,
request_body_max_bytes: None,
response,
summary: None,
description: None,
Expand All @@ -109,6 +114,11 @@ impl<'a, Context: ServerContext> ApiEndpoint<Context> {
self
}

pub fn request_body_max_bytes(mut self, max_bytes: usize) -> Self {
self.request_body_max_bytes = Some(max_bytes);
self
}

pub fn tag<T: ToString>(mut self, tag: T) -> Self {
self.tags.push(tag.to_string());
self
Expand Down Expand Up @@ -188,6 +198,7 @@ impl<'a> ApiEndpoint<StubContext> {
path: path.to_string(),
parameters: func_parameters.parameters,
body_content_type,
request_body_max_bytes: None,
response,
summary: None,
description: None,
Expand Down
11 changes: 10 additions & 1 deletion dropshot/src/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,14 @@ impl<Context: ServerContext> RequestContext<Context> {
}

/// Returns the maximum request body size.
///
/// This is typically the same as
/// `self.server.config.request_body_max_bytes`, but can be overridden on a
/// per-endpoint basis.
pub fn request_body_max_bytes(&self) -> usize {
self.server.config.default_request_body_max_bytes
self.endpoint
.request_body_max_bytes
.unwrap_or(self.server.config.default_request_body_max_bytes)
}

/// Returns the appropriate count of items to return for a paginated request
Expand Down Expand Up @@ -218,6 +224,9 @@ pub struct RequestEndpointMetadata {

/// The expected request body MIME type.
pub body_content_type: ApiEndpointBodyContentType,

/// The maximum number of bytes allowed in the request body, if overridden.
pub request_body_max_bytes: Option<usize>,
}

/// Helper trait for extracting the underlying Context type from the
Expand Down
2 changes: 1 addition & 1 deletion dropshot/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@
//!
//! ```text
//! // introduced in 1.0.0, present in all subsequent versions
//! versions = "1.0.0"..
//! versions = "1.0.0"..
//!
//! // removed in 2.0.0, present in all previous versions
//! // (not present in 2.0.0 itself)
Expand Down
2 changes: 2 additions & 0 deletions dropshot/src/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,7 @@ impl<Context: ServerContext> HttpRouter<Context> {
operation_id: handler.operation_id.clone(),
variables,
body_content_type: handler.body_content_type.clone(),
request_body_max_bytes: handler.request_body_max_bytes,
},
});
}
Expand Down Expand Up @@ -874,6 +875,7 @@ mod test {
path: path.to_string(),
parameters: vec![],
body_content_type: ApiEndpointBodyContentType::default(),
request_body_max_bytes: None,
response: ApiEndpointResponse::default(),
summary: None,
description: None,
Expand Down
1 change: 1 addition & 0 deletions dropshot/src/websocket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ mod tests {
operation_id: "".to_string(),
variables: Default::default(),
body_content_type: Default::default(),
request_body_max_bytes: None,
},
request_id: "".to_string(),
log: log.clone(),
Expand Down
24 changes: 24 additions & 0 deletions dropshot/tests/fail/bad_channel28.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2024 Oxide Computer Company

#![allow(unused_imports)]

use dropshot::channel;
use dropshot::RequestContext;
use dropshot::WebsocketUpgrade;

// Test: request_body_max_bytes specified for channel (this parameter is only
// accepted for endpoints, not channels)

#[channel {
protocol = WEBSOCKETS,
path = "/test",
request_body_max_bytes = 1024,
}]
async fn bad_channel(
_rqctx: RequestContext<()>,
_upgraded: WebsocketUpgrade,
) -> dropshot::WebsocketChannelResult {
Ok(())
}

fn main() {}
5 changes: 5 additions & 0 deletions dropshot/tests/fail/bad_channel28.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: extraneous member `request_body_max_bytes`
--> tests/fail/bad_channel28.rs:15:5
|
15 | request_body_max_bytes = 1024,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
23 changes: 23 additions & 0 deletions dropshot/tests/fail/bad_endpoint28.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2024 Oxide Computer Company

#![allow(unused_imports)]

use dropshot::endpoint;
use dropshot::HttpError;
use dropshot::HttpResponseUpdatedNoContent;
use dropshot::RequestContext;

// Test: incorrect type for request_body_max_bytes.

#[endpoint {
method = GET,
path = "/test",
request_body_max_bytes = "not_a_number"
}]
async fn bad_endpoint(
_rqctx: RequestContext<()>,
) -> Result<HttpResponseUpdatedNoContent, HttpError> {
Ok(HttpResponseUpdatedNoContent())
}

fn main() {}
14 changes: 14 additions & 0 deletions dropshot/tests/fail/bad_endpoint28.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0308]: mismatched types
--> tests/fail/bad_endpoint28.rs:15:30
|
15 | request_body_max_bytes = "not_a_number"
| ^^^^^^^^^^^^^^ expected `usize`, found `&str`
16 | }]
17 | async fn bad_endpoint(
| ------------ arguments to this method are incorrect
|
note: method defined here
--> src/api_description.rs
|
| pub fn request_body_max_bytes(mut self, max_bytes: usize) -> Self {
| ^^^^^^^^^^^^^^^^^^^^^^
44 changes: 44 additions & 0 deletions dropshot/tests/fail/bad_trait_channel28.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright 2024 Oxide Computer Company

#![allow(unused_imports)]

use dropshot::channel;
use dropshot::RequestContext;
use dropshot::WebsocketUpgrade;

#[dropshot::api_description]
trait MyServer {
type Context;

// Test: request_body_max_bytes specified for channel (this parameter is only
// accepted for endpoints, not channels)
#[channel {
protocol = WEBSOCKETS,
path = "/test",
request_body_max_bytes = 1024,
}]
async fn bad_channel(
_rqctx: RequestContext<Self::Context>,
_upgraded: WebsocketUpgrade,
) -> dropshot::WebsocketChannelResult;
}

enum MyImpl {}

// This should not produce errors about items being missing.
impl MyServer for MyImpl {
type Context = ();

async fn bad_channel(
_rqctx: RequestContext<Self::Context>,
_upgraded: WebsocketUpgrade,
) -> dropshot::WebsocketChannelResult {
Ok(())
}
}

fn main() {
// These items should be generated and accessible.
my_server_mod::api_description::<MyImpl>();
my_server_mod::stub_api_description();
}
5 changes: 5 additions & 0 deletions dropshot/tests/fail/bad_trait_channel28.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: endpoint `bad_channel` has invalid attributes: extraneous member `request_body_max_bytes`
--> tests/fail/bad_trait_channel28.rs:18:9
|
18 | request_body_max_bytes = 1024,
| ^^^^^^^^^^^^^^^^^^^^^^
43 changes: 43 additions & 0 deletions dropshot/tests/fail/bad_trait_endpoint28.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright 2024 Oxide Computer Company

#![allow(unused_imports)]

use dropshot::endpoint;
use dropshot::HttpError;
use dropshot::HttpResponseUpdatedNoContent;
use dropshot::RequestContext;

// Test: incorrect type for request_body_max_bytes.

#[dropshot::api_description]
trait MyApi {
type Context;

#[endpoint {
method = GET,
path = "/test",
request_body_max_bytes = "not_a_number",
}]
async fn bad_endpoint(
_rqctx: RequestContext<Self::Context>,
) -> Result<HttpResponseUpdatedNoContent, HttpError>;
}

enum MyImpl {}

// This should not produce errors about items being missing.

impl MyApi for MyImpl {
type Context = ();
async fn bad_endpoint(
_rqctx: RequestContext<Self::Context>,
) -> Result<HttpResponseUpdatedNoContent, HttpError> {
Ok(HttpResponseUpdatedNoContent())
}
}

fn main() {
// These items should be generated and accessible.
my_api_mod::api_description::<MyImpl>();
my_api_mod::stub_api_description();
}
14 changes: 14 additions & 0 deletions dropshot/tests/fail/bad_trait_endpoint28.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0308]: mismatched types
--> tests/fail/bad_trait_endpoint28.rs:19:34
|
16 | #[endpoint {
| - arguments to this method are incorrect
...
19 | request_body_max_bytes = "not_a_number",
| ^^^^^^^^^^^^^^ expected `usize`, found `&str`
|
note: method defined here
--> src/api_description.rs
|
| pub fn request_body_max_bytes(mut self, max_bytes: usize) -> Self {
| ^^^^^^^^^^^^^^^^^^^^^^
Loading

0 comments on commit d5874eb

Please sign in to comment.