Skip to content
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

api: Limit request body size to 10MB #5246

Merged
merged 5 commits into from Apr 3, 2023
Merged

Conversation

algochoi
Copy link
Contributor

@algochoi algochoi commented Mar 31, 2023

Summary

This PR adds a limit to the request body size so clients cannot send algod requests larger than 10MB.

Closes #5219

Test Plan

Adds tests in the handler to check source/dryrun request limits and adds an e2e test to check that a simulate request body >10MB fails with a 413 error.

@codecov
Copy link

codecov bot commented Mar 31, 2023

Codecov Report

Merging #5246 (0da0a6c) into master (f8a130e) will decrease coverage by 2.41%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #5246      +/-   ##
==========================================
- Coverage   53.75%   51.35%   -2.41%     
==========================================
  Files         450      450              
  Lines       56191    56193       +2     
==========================================
- Hits        30207    28859    -1348     
- Misses      23632    24959    +1327     
- Partials     2352     2375      +23     
Impacted Files Coverage Δ
daemon/algod/api/server/router.go 11.62% <0.00%> (-0.57%) ⬇️
daemon/algod/api/server/v2/handlers.go 0.83% <0.00%> (ø)

... and 103 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@algochoi algochoi marked this pull request as ready for review March 31, 2023 17:24
// Create a program with MaxTealSourceBytes+1 bytes
// This should fail inside the handler when reading the bytes from the request body.
largeProgram := []byte(strings.Repeat("a", v2.MaxTealSourceBytes+1))
tealDisassembleTest(t, largeProgram, 400, "http: request body too large", true)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These tests are affected by a smaller size limit (200KB instead of 10MB) defined by MaxTealSourceBytes and will trigger the error here:

return badRequest(ctx, err, err.Error(), v2.Log)

winder
winder previously approved these changes Mar 31, 2023
Eric-Warehime
Eric-Warehime previously approved these changes Apr 3, 2023

// Create a program with MaxTealSourceBytes+1 bytes
// This should fail inside the handler when reading the bytes from the request body.
largeProgram := []byte(strings.Repeat("a", v2.MaxTealSourceBytes+1))
Copy link
Contributor

Choose a reason for hiding this comment

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

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

Co-authored-by: Bob Broderick <118225939+bbroder-algo@users.noreply.github.com>
@winder winder merged commit c27422c into algorand:master Apr 3, 2023
9 checks passed
@winder winder changed the title algod: Limit request body size to 10MB api: Limit request body size to 10MB Apr 3, 2023
@algochoi algochoi deleted the limit-api-body branch April 3, 2023 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add POST body size limiting to API server
4 participants