Skip to content

Limit HTTP response size#3553

Merged
jvff merged 5 commits intolinera-io:mainfrom
jvff:limit-http-response-size
Mar 13, 2025
Merged

Limit HTTP response size#3553
jvff merged 5 commits intolinera-io:mainfrom
jvff:limit-http-response-size

Conversation

@jvff
Copy link
Copy Markdown
Contributor

@jvff jvff commented Mar 12, 2025

Motivation

Applications could cause excessive bandwidth costs to validators by performing HTTP requests to obtain large amounts of data as fast as possible.

Proposal

Ensure that HTTP responses are limited in size to a value agreed among the validators.

Test Plan

Wrote some unit tests in a separate branch, using the code from #3509:

https://github.com/jvff/linera-protocol-archive/blob/test-http-response-limits-p1/examples/how-to/perform-http-requests/tests/response_size_tests.rs

Release Plan

  • Backporting is not possible but we may want to deploy a new devnet and release a new
    SDK soon, because this contains changes to the consensus critical ResourceControlPolicy.

Links

@jvff jvff added the security label Mar 12, 2025
@jvff jvff added this to the Testnet #2 milestone Mar 12, 2025
@jvff jvff requested review from Twey, bart-linera, deuszx and ma2bd March 12, 2025 13:18
@jvff jvff self-assigned this Mar 12, 2025
@jvff jvff force-pushed the limit-http-response-size branch from cb2b864 to 22962d6 Compare March 12, 2025 13:22
@jvff jvff force-pushed the limit-http-response-size branch 2 times, most recently from 3b3e909 to 7ea2944 Compare March 12, 2025 21:35
@jvff jvff marked this pull request as ready for review March 12, 2025 21:43
Allow configuring a policy with a limit to HTTP response sizes.
@jvff jvff force-pushed the limit-http-response-size branch from 7ea2944 to 5bc3cbf Compare March 13, 2025 02:35
#[error("Serialized size of the executed block exceeds limit")]
ExecutedBlockTooLarge,
#[error("HTTP response exceeds the size limit of {limit} bytes, having at least {size} bytes")]
HttpResponseSizeLimitExceeded { limit: u64, size: u64 },
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make this a limit for all types of oracle responses?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, I'll open an issue to limit the oracle response size as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue #3563 created.

Copy link
Copy Markdown
Contributor

@deuszx deuszx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not panic if the header is misformatted.

jvff added 3 commits March 13, 2025 10:53
Prepare to enforce a size limit to the response.
It's no longer needed.
Prepare to abort early if too many bytes are received.
@jvff jvff force-pushed the limit-http-response-size branch from 5bc3cbf to 791e1be Compare March 13, 2025 10:54
Comment on lines +460 to +463
let total_header_size = headers
.iter()
.map(|header| (header.name.as_bytes().len() + header.value.len()) as u64)
.sum();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder - could we collect the sizes when we map response.headers() already? we have the data there (name, value, etc.).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but I think doing it separately is cleaner.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but not faster :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Premature optimization is the root of all evil ;)

@jvff
Copy link
Copy Markdown
Contributor Author

jvff commented Mar 13, 2025

Let's not panic if the header is misformatted.

I'll insist. It did not panic if the header is misformatted. It would have panicked if the http crate had a bug that would return None for the first header name. Here is the code:

https://docs.rs/http/0.2.12/src/http/header/map.rs.html#3039

Note that IntoIter.next starts as None: https://docs.rs/http/0.2.12/src/http/header/map.rs.html#1994

@jvff jvff force-pushed the limit-http-response-size branch from 791e1be to 7af077e Compare March 13, 2025 11:27
Return an error as soon as it is detected that the size will exceed the
limit.
@jvff jvff force-pushed the limit-http-response-size branch from 7af077e to 6f3ffe2 Compare March 13, 2025 11:28
@jvff jvff merged commit 97625ec into linera-io:main Mar 13, 2025
23 checks passed
@jvff jvff deleted the limit-http-response-size branch March 13, 2025 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Limit size of HTTP responses

3 participants