feat(ratelimiter): add MethodParser for pre-decode JSON-RPC method extraction (PLT-800)#3755
feat(ratelimiter): add MethodParser for pre-decode JSON-RPC method extraction (PLT-800)#3755amir-deris wants to merge 6 commits into
Conversation
PR SummaryMedium Risk Overview
Includes broad unit tests (EVM/CometBFT shapes, batches, malformed/truncated bodies, probe-boundary cases). No request-path wiring in this PR. Reviewed by Cursor Bugbot for commit edde4ea. Bugbot is set up for automated code reviews on this repo. Configure here. |
amir-deris
changed the title
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3755 +/- ##
==========================================
- Coverage 59.93% 59.00% -0.93%
==========================================
Files 2288 2202 -86
Lines 189868 180100 -9768
==========================================
- Hits 113793 106269 -7524
+ Misses 65942 64480 -1462
+ Partials 10133 9351 -782
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
A well-structured, well-tested streaming JSON-RPC method parser, but it takes the first duplicate method key while Go's encoding/json (used by the downstream go-ethereum/CometBFT handlers) takes the last — a rate-limit-bypass discrepancy in a feature whose entire purpose is endpoint protection. A truncated batch is also silently accepted.
Findings: 1 blocking | 3 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
- 1 blocking issue(s) flagged inline on specific lines.
Non-blocking
- Cursor's second-opinion review file (cursor-review.md) was empty — that pass produced no output.
- Minor edge case in classifyErr (method_parser.go:196): a well-formed body whose length exactly equals maxProbeBytes will exhaust the LimitedReader (lr.N <= 0) at the closing EOF and could be misreported as ErrProbeLimit rather than a clean parse/error. Low impact, but worth a test to pin the boundary behavior.
- 1 suggestion(s)/nit(s) flagged inline on specific lines.
| // A valid JSON object always has string keys; anything else is malformed. | ||
| return "", ErrNotObject | ||
| } | ||
| if key == "method" && !found { |
There was a problem hiding this comment.
[blocker] Taking the first method key diverges from how the request is actually executed. go-ethereum and CometBFT decode the body with Go's encoding/json, which for duplicate keys keeps the last value. So {"method":"cheap","method":"expensive"} is charged/rate-limited as cheap here but executed as expensive downstream — a rate-limit bypass in the exact feature this parser protects. Match the downstream semantics (take the last occurrence, i.e. keep overwriting method until the object ends) or reject objects containing a duplicate method key. The TestParse_DuplicateMethodTakesFirst test encodes the wrong invariant and should be updated accordingly. Note that with consumeRemainder=false (single-object path) you currently return at the first method and never even observe the duplicate, so a fix must also drain far enough to detect it.
| return out, err | ||
| } | ||
| out = append(out, method) | ||
| } |
There was a problem hiding this comment.
[suggestion] readBatchMethods never consumes or validates the closing ]. A truncated batch such as [{"method":"eth_call"} returns successfully (dec.More() reports false at EOF without erroring), so a malformed body is accepted as a valid batch. After the loop, read one more token and confirm it is the closing ] delim (returning a malformed-input error otherwise), and add a regression test for the truncated-batch case.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
A well-structured, thoroughly-tested new utility (MethodParser) for pre-decode JSON-RPC method extraction, not yet wired into any request path. No blocking issues; two minor edge-case robustness improvements and a couple of documentation/description discrepancies to consider.
Findings: 0 blocking | 6 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
Non-blocking
REVIEW_GUIDELINES.md(from the base branch) is empty, so no repo-specific review standards could be applied.- The Cursor second-opinion pass (
cursor-review.md) produced no output — nothing to merge from it. - PR description overclaims behavior: it says the parser "returns as soon as
methodis found" and that tests include "a byte-counting reader proving the parser stops reading once method is found early." Neither is accurate —readMethodFromObjectintentionally reads the object to its closing}(to detect a duplicatemethod, as the code comment correctly documents), and there is no byte-counting reader inmethod_parser_test.go. Consider aligning the description with the actual (correct) behavior. - Downstream-wiring consideration for the follow-up PRs: because each request object is read to its end, a legitimate large request whose body exceeds
maxProbeBytes(e.g. a bigeth_sendRawTransaction) yieldsErrProbeLimiteven whenmethodappears first. The consuming EVM/CometBFT middleware must treatErrProbeLimitas "fall back to full decode," not "reject," to avoid dropping valid large requests. Worth calling out where the parser gets wired in. - 2 suggestion(s)/nit(s) flagged inline on specific lines.
| if err == nil { | ||
| return nil | ||
| } | ||
| if (errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF)) && lr.N <= 0 { |
There was a problem hiding this comment.
[suggestion] lr.N <= 0 can't reliably distinguish a genuine probe-limit overflow from a body whose length is exactly maxProbeBytes. If a malformed/truncated body is exactly maxProbeBytes long, the LimitedReader drains to N == 0 and the trailing EOF is misclassified as ErrProbeLimit rather than a parse error. The standard fix is to allocate one extra byte of budget — N: p.maxProbeBytes + 1 — and treat N == 0 here as proof the source actually exceeded the limit; a body of exactly maxProbeBytes then leaves N == 1 and is classified correctly. (Matches Codex's medium-severity finding.)
| if err != nil { | ||
| return nil, false, classifyErr(err, lr) | ||
| } | ||
| return []string{method}, false, nil |
There was a problem hiding this comment.
[nit] After the top-level object closes, Parse returns without checking for trailing non-whitespace data, so {"method":"eth_call"} garbage is accepted as valid. The downstream encoding/json handlers would reject such a body, so the parser is more lenient than the real decode path. The batch path validates the closing ] but likewise doesn't reject data after it. Consider asserting the decoder is at EOF (e.g. a final dec.Token() returning io.EOF) after the value, and add a test for trailing content. (Matches Codex's low-severity finding.)
seidroid
Bot
dismissed
their stale review
Superseded: latest AI review found no blocking issues.
There was a problem hiding this comment.
A well-structured, well-tested streaming JSON-RPC method extractor, but its case-sensitive "method" match diverges from the case-insensitive encoding/json decoding used downstream, which reintroduces the very rate-limit-bypass the duplicate-method check was added to prevent.
Findings: 1 blocking | 3 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
- 1 blocking issue(s) flagged inline on specific lines.
Non-blocking
- Cursor's second-opinion review (cursor-review.md) was empty — no findings from that pass.
- Perf/behavior note (design tradeoff, not a bug):
readMethodFromObjectalways drains each object to its closing}(to detect duplicatemethod) rather than returning as soon asmethodis found, so the PR description's 'returns as soon as method is found' overstates it — the in-code comment is accurate. A consequence is that a legitimate large body (e.g.eth_sendRawTransactionwith >1 MiB calldata) yieldsErrProbeLimiteven whenmethodappears first. Downstream wiring (out of scope here) must treatErrProbeLimitas 'undetermined method' and fall back safely rather than dropping the request or applying the wrong limit; worth confirming the 1 MiB default is comfortably above expected max body sizes. - Consider mirroring encoding/json's actual duplicate resolution (last value wins) instead of rejecting, or document explicitly that rejection is intentionally stricter than the downstream decoder — either is defensible, but the two decoders' behaviors should be reconciled deliberately.
| // A valid JSON object always has string keys; anything else is malformed. | ||
| return "", ErrNotObject | ||
| } | ||
| if key == "method" { |
There was a problem hiding this comment.
[blocker] Case-sensitive match reopens the bypass the duplicate-method check closes. Downstream JSON-RPC handlers (go-ethereum jsonrpcMessage, CometBFT RPCRequest) decode with encoding/json into Method string json:"method"``, which matches object keys case-insensitively and lets the last duplicate win. So:
{"method":"cheap","Method":"expensive"}→ this parser returns"cheap"(the"Method"key is skipped, no duplicate detected), while the handler executes"expensive"— the request is charged as cheap but runs as expensive.{"Method":"eth_call"}→ this parser returnsErrNoMethod, but the handler still dispatcheseth_call.
Since the whole point of reading each object to its end and rejecting duplicate method keys is to stay consistent with the downstream decoder and prevent this exact bypass, the comparison must be case-insensitive too: treat any case variant (strings.EqualFold(key, "method")) as the method field, and detect/reject duplicates across those variants. Please add mixed-case test cases ("Method", "METHOD", and the mixed-case duplicate above) to lock this in.
…on, data validation aafter json end
There was a problem hiding this comment.
A self-contained, thoroughly tested streaming JSON-RPC method extractor with no request-path wiring; the code is correct and self-consistent, but a few of its own doc comments and the PR description oversell the allocation-avoidance and early-exit behavior.
Findings: 0 blocking | 5 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- PR description inaccuracy: it states the parser "returns as soon as
methodis found" and that tests include "a byte-counting reader proving the parser stops reading once method is found early." Neither is true —readMethodFromObjectdeliberately reads each object to its end to detect duplicatemethodkeys (documented at lines 15-17), so it does not stop early, and no such byte-counting test exists inmethod_parser_test.go. Update the description to match the implemented (and correct) read-to-end-for-dup-detection behavior. (Consolidates Codex P1 #1.) - Design consequence worth a note for the follow-up wiring PRs: because the whole object is read within the probe budget, a legitimate large request with
methodfirst (e.g. a >1 MiBeth_sendRawTransaction) returnsErrProbeLimitrather than the method name. The wiring middleware will need a defined fallback (full decode / reject) forErrProbeLimiton otherwise-valid bodies. - cursor-review.md is empty — the Cursor second-opinion pass produced no output. REVIEW_GUIDELINES.md is also empty/missing, so no repo-specific review standards could be applied.
- No prompt-injection or malicious content was found in the diff, tests, or PR description.
- 1 suggestion(s)/nit(s) flagged inline on specific lines.
| // skipValue consumes exactly one complete JSON value from dec — the value whose | ||
| // first token dec is positioned at. Scalars cost one token; objects and arrays | ||
| // are drained with a depth counter so their (possibly large) contents are read | ||
| // off the stream but never decoded into Go structures. |
There was a problem hiding this comment.
[nit] Minor doc/impl mismatch (also flagged by Codex): dec.Token() on a scalar string value does materialize it as a Go string, so a large string inside params is allocated and scanned — only composite ({}/[]) contents are truly drained without building a Go value. The allocation is transient and bounded by maxProbeBytes, so impact is low, but consider softening the comment (and the PR's "params is never allocated" claim) to "never decoded into a full Go structure" to avoid overstating the guarantee.
seidroid
Bot
dismissed
their stale review
Superseded: latest AI review found no blocking issues.
There was a problem hiding this comment.
Beyond the inline nit finding (expectEOF conflating probe-limit exhaustion with a real EOF), this run also examined and ruled out three other candidate issues flagged by finder agents: an int64 overflow in maxProbeBytes+1 (only possible at math.MaxInt64, not a realistic probe size), batch parsing being all-or-nothing vs. downstream per-element dispatch (no request-path wiring exists yet in this PR, so there is no live divergence to compare against), and a JSON null method value enabling an admission bypass (dec.Token() returns an untyped nil for null, which fails the string type assertion and correctly yields ErrMethodNotString, not a silent accept).
Extended reasoning...
This is an informational note only, not a fresh review of the whole PR. The bug hunting system's single new finding this run (expectEOF's probe-limit/EOF ambiguity) is a nit per all four verifiers — fail-safe direction, no live consumer yet, small fix mirroring an existing pattern in the same file — and is already captured in the inline comment. Separately, finder agents raised four other candidate issues that verifiers refuted; three of those (int64 overflow, batch all-or-nothing semantics, JSON null bypass) are concrete enough to record here so they are not re-explored from scratch in a future pass. This does not change the overall assessment of the PR.
| func expectEOF(dec *json.Decoder) error { | ||
| if _, err := dec.Token(); err != nil { | ||
| if errors.Is(err, io.EOF) { | ||
| return nil | ||
| } | ||
| return err | ||
| } | ||
| return ErrTrailingData | ||
| } |
There was a problem hiding this comment.
🟡 expectEOF treats any io.EOF from the final dec.Token() call as a clean end-of-input, but io.LimitedReader.Read also returns io.EOF once its probe budget is exhausted — regardless of whether the underlying reader still has bytes left. If trailing garbage sits beyond the probe budget, expectEOF misclassifies the budget-exhaustion EOF as "clean" and silently accepts a body it should reject (or should surface as ErrProbeLimit), unlike classifyErr which already guards on lr.N<=0 for this exact ambiguity.
Extended reasoning...
The bug. expectEOF (ratelimiter/method_parser.go:97-106) calls dec.Token() after the top-level value closes and treats any errors.Is(err, io.EOF) as proof the body ended cleanly. But the underlying reader is io.LimitedReader{N: maxProbeBytes+1}, and io.LimitedReader.Read returns io.EOF once N reaches 0 — a signal that means "probe budget exhausted," not "stream truly ended." classifyErr already treats this exact ambiguity carefully, checking lr.N <= 0 before mapping an EOF to ErrProbeLimit rather than a genuine parse success/failure. expectEOF has no equivalent guard, so it can accept a budget-exhausted EOF as a clean finish even when real trailing bytes exist past the cap.
Trigger path. If trailing non-whitespace garbage sits beyond the probe budget (i.e. the object + any intervening whitespace exactly exhausts lr.N), the decoder's Read call inside expectEOF's dec.Token() returns io.EOF purely because N hit 0 — not because the source reader is actually empty. expectEOF sees errors.Is(err, io.EOF) and returns nil, so Parse never re-enters classifyErr for this path and the request is reported as clean.
Step-by-step proof (verified independently by all four verifiers against the actual code, with the synthesis description's specific byte-count example found to be off by the exact +1 budget offset). Using body {"method":"a"} + two spaces + "X" (17 bytes total) and a small custom probe:
NewMethodParser(15)setslr.N = maxProbeBytes+1 = 16.json.Decoderbuffers up to 16 bytes on its internal read — the 14-byte object plus the 2 trailing spaces — draininglr.Nto 0. The finalXis never read into the buffer.readMethodFromObjectsucceeds, returning method"a".expectEOFcallsdec.Token(), which triggers anotherReadon the now-exhaustedLimitedReader; thatReadreturnsio.EOFbecauseN<=0, not because the source (strings.Reader) is actually at its end — theXbyte is still sitting there.expectEOF'serrors.Is(err, io.EOF)branch fires and returnsnil, soParsereturnsmethods=["a"], batch=false, err=nil— silently accepting a body whose trailing byte was never inspected.- Compare: raising the probe by just enough that the whole 17-byte body fits (e.g. probe=17) makes
expectEOFcorrectly see the realXtoken and returnErrTrailingData. The classification flips purely on whether the probe boundary lands before or after the garbage — exactly the ambiguityclassifyErr'slr.N<=0check exists to resolve, but whichexpectEOFbypasses entirely.
Why existing tests don't catch it. TestParse_TrailingData and TestParse_ExactProbeLimitBodyParses exercise trailing-data rejection and exact-probe-length bodies separately, but no test combines a small custom probe with trailing garbage landing exactly past the cap — the one condition where expectEOF's io.EOF branch is genuinely ambiguous.
Fix. Mirror classifyErr: pass lr into expectEOF and, on io.EOF, only treat it as clean if lr.N > 0; if lr.N <= 0, propagate the EOF so Parse's existing classifyErr(err, lr) call maps it to ErrProbeLimit instead of silently returning success.
Impact / severity. All four independent verifiers who reproduced this converged on nit: (1) the direction is fail-safe, not fail-open — the method reported ("a") is the same one a full downstream encoding/json decode would also see, and the full body (read again downstream, unbounded by any probe) would independently reject the trailing garbage as invalid JSON, so the request never dispatches; there is no rate-limit bypass, mis-charge, crash, or data loss. (2) The trigger requires a custom, small maxProbeBytes with garbage landing exactly at the probe boundary — at the 1 MiB default this needs a >1 MiB body with garbage precisely at that byte offset, an exotic scenario. (3) This PR wires no request path yet, so there is no live consumer today. It is nonetheless a genuine, verified inconsistency in validation logic this PR just added specifically to catch trailing data, and the fix is small and mirrors an existing pattern in the same file.
There was a problem hiding this comment.
An organization admin can view or raise the cap at claude.ai/admin-settings/claude-code. The cap resets at the start of the next billing period.
Once the cap resets or is raised, push a new commit or reopen this pull request to trigger a review.
There was a problem hiding this comment.
Adds a self-contained, well-tested streaming JSON-RPC MethodParser with no request-path wiring. One narrow edge case (integer overflow when maxProbeBytes is set near math.MaxInt64) is worth guarding; otherwise the logic and test coverage are solid.
Findings: 0 blocking | 3 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- Cursor's second-opinion review (
cursor-review.md) produced no output — treat it as absent, not as a clean pass. - The whole request object is read to its closing brace to detect duplicate
methodkeys, so any legitimate body larger thanmaxProbeBytesalso returnsErrProbeLimit. This is intentional and documented in the PR, but the future wiring middleware must treatErrProbeLimitas "method undetermined → fall back to full decode" and never as a rejection; consider stating this contract in theErrProbeLimitdoc comment so downstream integrators don't misuse it. - 1 suggestion(s)/nit(s) flagged inline on specific lines.
| func (p *MethodParser) Parse(r io.Reader) (methods []string, batch bool, err error) { | ||
| // N is maxProbeBytes+1 so that lr.N reaching 0 unambiguously means the body | ||
| // exceeded the budget. | ||
| lr := &io.LimitedReader{R: r, N: p.maxProbeBytes + 1} |
There was a problem hiding this comment.
[suggestion] p.maxProbeBytes + 1 overflows to a negative value when maxProbeBytes is math.MaxInt64 (or within 1 of it). A negative LimitedReader.N makes every Read return io.EOF immediately, and classifyErr then maps that to ErrProbeLimit (because lr.N <= 0), so every Parse call fails. It requires an extreme configured value, but since NewMethodParser is the public entry point, consider capping/rejecting oversized values (e.g. clamp to math.MaxInt64 - 1 or reject in the constructor) so the sentinel-byte arithmetic can't wrap. (Flagged by Codex.)
There was a problem hiding this comment.
This is outdated I think since you subtract 1 in the constructor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4a27fae. Configure here.
There was a problem hiding this comment.
A well-scoped, self-contained addition of a streaming JSON-RPC MethodParser with thorough test coverage and no request-path wiring. No blocking issues; one design caveat (shared with Codex) should be carried into the follow-up wiring PR, plus a minor style nit.
Findings: 0 blocking | 4 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- Design caveat (also raised by Codex, P1): because each object is read to its end to detect duplicate
methodkeys, an attacker can pad a request with whitespace/benign fields pastmaxProbeBytesand forceErrProbeLimiteven whenmethodappears first. Per the PR description the wiring middleware must treatErrProbeLimitas 'method undetermined → full decode'; if that fallback is unrestricted it becomes a per-method rate-limit bypass. This is out of scope for this parser-only PR, but the follow-up EVM/CometBFT middleware PRs must bound (or rate-limit conservatively on) the fallback path. Recommend adding an explicit test/assertion in those PRs. Not blocking here. - Cursor's second-opinion review file (
cursor-review.md) was empty — that pass produced no output. - Minor:
skipValue/dec.Token()fully materializes large string values inparams(e.g. a ~200 KiB hex blob) into a Go string even though the value is skipped, so the 'avoid paying the memory cost of the whole request' goal is only partially met for a single oversized string field (still bounded bymaxProbeBytes). Acceptable given the probe cap; noting for awareness. - 1 suggestion(s)/nit(s) flagged inline on specific lines.
| maxProbeBytes = DefaultMaxProbeBytes | ||
| } | ||
| if maxProbeBytes == math.MaxInt64 { | ||
| //preventing overflow error |
There was a problem hiding this comment.
[nit] Nit: add a space after // (// preventing overflow error) to match the file's comment style, and consider rewording to explain why — the maxProbeBytes+1 sentinel in Parse would overflow to a negative LimitedReader.N if maxProbeBytes == math.MaxInt64.
|
Ran a cross-review of this at head The parser is in good shape. The security pass could not find a token-level report-one-method-dispatch-another differential. The case-fold match against Three things I'd like to close, filed as inline comments:
None of these block the parser as a standalone piece, they're mostly about the contract the wiring will inherit. Full review ledger (boundaries, findings, what was verified sound)Boundaries checked
Findings requiring action: F1 (doc the fail-closed contract), F2 ( Deferrable (fail-closed-safe or wiring-time): Verified sound: no report-A/run-B differential (exhaustive fold analysis); probe-budget invariant robust against read-ahead; concurrent-safe; iterative Reviewed independently, blinded, at |
| // Parse reads a JSON-RPC request body from r and returns the method name(s) it | ||
| // carries. A single request object yields a one-element slice with batch=false; | ||
| // a top-level array yields one method per element in order with batch=true. | ||
| func (p *MethodParser) Parse(r io.Reader) (methods []string, batch bool, err error) { |
There was a problem hiding this comment.
The safety of this gate rests on how the caller handles these errors, and right now that contract lives only in the PR description. Every error other than ErrProbeLimit has to mean reject or fall back to a full decode, and ErrProbeLimit has to mean fall back rather than reject. If a caller ever maps a parse error to admit-with-default, the duplicate-key and trailing-data rejections stop being fail-closed, and a body that reports a cheap method while decoding to an expensive one gets through, which is the case this gate exists to catch.
There's nothing to fix at runtime in 1a since there's no wiring yet. I'd like the fail-closed contract written into the package doc so 1b and 1c build on it explicitly, something like every error other than ErrProbeLimit means reject and ErrProbeLimit means fall back to full decode. Worth carrying as an acceptance criterion into those PRs too.
| return method, nil | ||
| } | ||
|
|
||
| // skipValue consumes exactly one complete JSON value from dec — the value whose |
There was a problem hiding this comment.
The comment here says the skipped contents are read off the stream but never decoded into Go structures, and that isn't quite what happens. dec.Token() decodes every token, allocating a string for each string and boxing each scalar into an interface{}. Since we drain the whole object to catch a duplicate method, every request pays that on the rate-limiter path. A within-budget scalar-heavy params array measured around 600k allocations and 11 MB of garbage per request, which works against the streaming-partial-read goal.
The non-method branch can skip with a RawMessage decode instead, which still validates and still respects the LimitedReader budget but drops the per-scalar boxing (roughly 600k allocs down to 23 in the same benchmark):
var raw json.RawMessage
if err := dec.Decode(&raw); err != nil {
return err
}At a minimum I'd correct the comment, but the RawMessage swap looks cheap and worth doing here.
| lr := &io.LimitedReader{R: r, N: p.maxProbeBytes + 1} | ||
| dec := json.NewDecoder(lr) | ||
|
|
||
| tok, err := dec.Token() |
There was a problem hiding this comment.
One coverage gap to flag for the :26657 path. CometBFT serves every RPC as a URI route as well, so GET /broadcast_tx_sync?tx=... carries the method in the URL path with no method field in the body. This parser reads the body first, so those requests would come back as ErrNoMethod and the real method never gets classified.
Not necessarily something to solve in 1a. I'd like us to decide the scope explicitly, either this parser is only ever fed body-form requests and we say so in the doc, or 1c pulls the method from the path before falling back to the body parser. Either is fine, I just want it written down before the CometBFT wiring lands.
3 participants
Summary
Implements
MethodParser(PLT-800), for the RPC Endpoint Protection work.MethodParserextracts the JSON-RPC"method"field from a request body via a streaming partial read, before the full body is decoded into a Go request struct. This lets per-method rate-limit overrides and, later, weighted admission make a decision based on which method is being called without first paying the memory cost of materialising the whole request.What it does
NewMethodParser(maxProbeBytes)→Parse(io.Reader) (methods []string, batch bool, err error).json.Decoderand skips every non-methodfield via a depth-tracked scan — nested objects/arrays inparamsare read off the stream but never decoded into Go structures. Each request object is read to its closing}so a duplicate"method"key can be detected and rejected (rather than returning at the first match, which could disagree with the last-wins valueencoding/jsonwould decode downstream — letting a body report one method while dispatching another)."method"case-insensitively, the same wayencoding/jsonstruct-tag decoding does downstream, so a body cannot report one method to the rate limiter while dispatching another.:8545/:8546) and CometBFT RPC (:26657), which share the JSON-RPC 2.0 shape and the"method"field.batch=true.maxProbeBytes(default 1 MiB) bounds the work an adversarial body (a giantparams) can force; exceeding it returnsErrProbeLimit, distinguished from a genuinely truncated/malformed body. Because each object is read to its end, a legitimate body larger than the probe limit also yieldsErrProbeLimit— the wiring middleware (follow-up PRs) must treat that as "method undetermined, fall back to full decode" rather than a rejection.io.Reader— the body tee/rewind decision stays with the wiring middleware.Scope
Tests
ratelimiter/method_parser_test.go— EVM/CometBFT single requests, both field orders, nested/empty/largeparams, escaped and case-insensitive method keys, batches (multi/single/method-after-params), duplicate-method rejection (single + batch + mixed-case), trailing-data rejection, every error sentinel (ErrNoMethod,ErrMethodNotString,ErrNotObject,ErrEmptyBatch,ErrDuplicateMethod,ErrTrailingData,ErrProbeLimit), truncated-vs-probe-limit disambiguation (including the exact-probe-limit boundary), and reading from a streamingio.Reader.