fix(rpc): apply RST_STREAM frame limit by default - #8
Open
waynercheung wants to merge 1 commit into
Open
waynercheung wants to merge 1 commit into
waynercheung wants to merge 1 commit into
Conversation
The node.rpc.maxRstStream and secondsPerWindow options defaulted to 0, which left the HTTP/2 RST_STREAM frame limit unset, and the limit was configured only when both values were positive. Give them sensible defaults, validate them at load time, and apply the limit unconditionally so the configuration behaves consistently. - NodeConfig: default maxRstStream/secondsPerWindow to 1000/5. In postProcess, reject negative values for either option and reject Integer.MAX_VALUE specifically for maxRstStream, which grpc-java treats as disabling the limit. Independently replace each zero with its default and log a warning. - RpcService: read and validate the two values before allocating the executor, then always configure maxRstFramesPerWindow. - reference.conf, docs/configuration.md and config/README.md: document the new defaults and the 0 fallback for compatibility. - Tests: pin the Java and reference defaults, preserve positive overrides, and verify PING at the RST limit followed by GOAWAY after one additional reset using a prebuilt HTTP/2 frame burst. Note: an explicit 0 now falls back to the default instead of leaving the limit unset, and maxRstStream = 2147483647 is rejected at startup.
waynercheung
force-pushed
the
fix/rpc-rst-stream-limit
branch
from
September 21, 2026 10:22
bd99b08 to
cfdc57c
Compare
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 does this PR do?
Turns
node.rpc.maxRstStreamandnode.rpc.secondsPerWindowinto properly defaulted and validated options, and applies the gRPC HTTP/2 RST_STREAM frame limit unconditionally.common(NodeConfig): default both options to200 / 30, matching Netty's HTTP/2 server defaults. InpostProcess, reject negative values for either option and rejectInteger.MAX_VALUEspecifically formaxRstStream(which grpc-java treats as disabling the limit), and independently fall back each0to its default with a warning.framework(RpcService): read and validate the two values before allocating the RPC executor, then always callmaxRstFramesPerWindow(...).reference.conf,docs/configuration.md,config/README.md: document the new defaults and the0fallback kept for backward compatibility.Why are these changes required?
Both options previously defaulted to
0, andRpcServiceconfiguredmaxRstFramesPerWindowonly when both values were positive. As a result the RST_STREAM frame limit was left unset in a default deployment, and misconfigured values (negative for either option, ormaxRstStream == Integer.MAX_VALUE) were accepted silently. This brings the two options in line with how the neighboringmaxConcurrentCallsPerConnectionoption already behaves (a secure default plus validation), so the setting is consistent across the config, cannot be silently disabled, and fails fast on invalid input.This PR has been tested by:
NodeConfigTest: default resolves to200/30; each0independently falls back to its default; negative values andmaxRstStream == Integer.MAX_VALUEare rejected withPARAMETER_INIT; explicit positive values (includingInteger.MAX_VALUE - 1and anInteger.MAX_VALUEwindow) are preserved.RpcServiceHttp2SecurityTest: the limit is enforced at the connection layer under the default, under the legacy0/0, and under an explicit small limit; a connection at the limit still gets its PING acknowledged, and one more RST_STREAM past the limit is answered withGOAWAY(ENHANCE_YOUR_CALM); invalid runtime values are rejected before the RPC executor is allocated.reference.confvalidation.Follow up
None.
Extra details
0for either option now falls back to its default (with a startup warning) instead of leaving the limit unset;maxRstStream = 2147483647(Integer.MAX_VALUE) is rejected at startup. Upgrade notes are included indocs/configuration.mdandcommon/src/main/java/org/tron/core/config/README.md.