Repository navigation
Conversation
HTTP/1 JSON-RPC collected the entire POST body before checking the 1 MiB limit and before authentication. Read through http_body_util::Limited so the request is refused with 413 as soon as the cap is crossed. The public-bind auth guard from the original branch (RPC and REST) is already on main via 8fcda11 and is not repeated here. Re-applied on main d9c9b37 from cursor/rpc-body-auth-guard-7567 (original branch left as-is). Co-authored-by: joshswift <joshswift@protonmail.com>
Keep main's streaming Limited body read (a694465) and change only its cap from the fixed DEFAULT_MAX_REQUEST_SIZE to the configured max_request_size. Add a test that a configured cap above 1 MiB is usable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What remains after main
Josh's a694465 ("cap RPC bodies…") already moved the HTTP/1 JSON-RPC read onto
http_body_util::Limited, so an unauthenticated client can no longer make the node buffer an unbounded body. This PR is now rebased onto that change with a merge commit, and only the remaining delta is left:DEFAULT_MAX_REQUEST_SIZE(1 MiB), so a configuredmax_request_sizeabove 1 MiB can never be used. Any larger body gets a 413 before the configured check runs.Limited::new(req.into_body(), server.max_request_size), so the server stops reading at the configured cap, whether that is smaller or larger than 1 MiB.Diff vs main:
src/rpc/server.rs, the one-line cap change plus two tests.Verification (Rust 1.88.0, branch head 8641580, includes node main 4f1be96)
http1_accepts_body_under_a_cap_above_the_default(2 MiB cap, ~1.5 MiB body)413 Payload Too Large200http1_rejects_oversized_body_before_the_remainder_is_sentElapsed(waits for the rest of the body)rpc::161,rpc_tests29,rpc_edge_cases9,rpc_advanced_scenarios9.cargo test --no-fail-fast: 3016 passed / 2 failed / 26 ignored (main 4f1be96: 3016 / 0 / 26). Both failures are in modules this PR doesn't touch (storage::ibd_engine::disk_index::…async_spill_serves_queries_via_pending_then_registers,node::parallel_ibd::chunk_assigner::…a6m_gd_slow_open_uncools_prior_hero_when_pin_empty). Rerun on this branch, they passed 5 out of 5 times, so they are timing-dependent under the full parallel run. The other three reconciled branches had 0 failures.cargo clippy --all-targets: the same 90 diagnostics as main, with 0 new. Main already fails lib-test clippy on twononminimal_boolerrors insrc/node/parallel_ibd/chunk_assigner_tests.rs(5044, 5105), which this PR doesn't touch.rustfmt --checkonsrc/rpc/server.rs: clean. Note that main itself isn'tcargo fmt --checkclean (tests/feature_flags_reflection_tests.rs:7, already there on 4f1be96).Review notes
mainat 4f1be96; no force-push.git merge-treeis clean against fix(node): anchor assume-valid per network with Core's hash and minimum work #31, fix(node): count witness bytes in the mempool cap at insert #34, fix(node): reject mempool txs over standard weight #35, fix(network): bound inbound frames and drop bad checksums #36, fix(network): ban public peers for oversized messages #37, fix(node): refuse a reorg over empty undo data and tell the operator at startup #38 and Optional: pass assume-valid ancestry to consensus (companion to blvm-consensus#24) #39).