rpc: fix null result unmarshalling in CallContext#20393
Merged
AskAlexSharov merged 5 commits intomainfrom Apr 10, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes JSON-RPC client unmarshalling when the server returns a JSON null result by removing the accidental double-indirection (&result) and adding a regression test to ensure null results are preserved correctly.
Changes:
- Update
Client.CallContextto unmarshal into the provided result pointer (not&result) and skip unmarshalling whenresultisnil. - Add
testService.ReturnNulland a client test that assertsnullresponses are returned as"null"(viajson.RawMessage). - Update server registration test expectations to account for the newly exposed callback.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rpc/client.go | Fixes result unmarshalling by passing the actual result pointer to json.Unmarshal (avoids double indirection). |
| rpc/client_test.go | Adds a regression test validating correct handling of JSON null results. |
| rpc/testservice_test.go | Adds a ReturnNull RPC method used by the new client regression test. |
| rpc/server_test.go | Adjusts callback count expectation due to the added ReturnNull method. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return testError{} | ||
| } | ||
|
|
||
| func (s *testService) ReturnNull() interface{} { |
There was a problem hiding this comment.
For consistency with the rest of this test service (which uses any elsewhere), consider changing the return type from interface{} to any here. This avoids mixing the two spellings in the same file and matches current Go style.
Suggested change
| func (s *testService) ReturnNull() interface{} { | |
| func (s *testService) ReturnNull() any { |
yperbasis
approved these changes
Apr 8, 2026
Using &result (double indirection on interface{} already holding a pointer)
prevented correct unmarshalling of JSON null responses. Fix mirrors
go-ethereum PR #26723.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ReturnNull adds one more registered callback to testService. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
lupin012
force-pushed
the
lupin012/fix-callcontext-null-unmarshal
branch
from
e936e44 to
ae839c1
Compare
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
AskAlexSharov
added a commit
that referenced
this pull request
Apr 13, 2026
Cherry-pick of #20393 from main.
AskAlexSharov
added a commit
that referenced
this pull request
Apr 14, 2026
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.
Using &result (double indirection on interface{} already holding a pointer) prevented correct unmarshalling of JSON null responses. Fix mirrors go-ethereum PR #26723.
For erigon fixes:
https://github.com/orgs/erigontech/projects/21?pane=issue&itemId=94164858&issue=erigontech%7Csecurity%7C7