fix(tm2): rpc unhandled panic #711
Merged
Conversation
This file contains 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
When panic is called with a string argument, that triggers an other panic in the recover handler, because it tries to convert the argument to [an error type][1]. Fortunately this second panic is catched by the recover of the stdlib in [net/http/server.go][0], so the node doesn't halt. To trigger the problem, you can run the following command: ```sh $ gnokey query -data xxx vm/qrender Post "http://127.0.0.1:26657": EOF ``` Because the handler behind `vm/qrender` expects to have some data in a particular format and it's not, it [panics with a string][2]. [0]: https://cs.opensource.google/go/go/+/refs/tags/go1.20.3:src/net/http/server.go;l=1851 [1]: https://github.com/gnolang/gno/blob/0eb5ff762eeb7e6b98d1f8afdb1214566352cabf/tm2/pkg/bft/rpc/lib/server/http_server.go#L164 [2]: https://github.com/gnolang/gno/blob/0eb5ff762eeb7e6b98d1f8afdb1214566352cabf/tm2/pkg/sdk/vm/handler.go#L131-L134
moul
approved these changes
Apr 12, 2023
peter7891
pushed a commit
that referenced
this pull request
May 9, 2023
* fix(tm2): rpc unhandled panic When panic is called with a string argument, that triggers an other panic in the recover handler, because it tries to convert the argument to [an error type][1]. Fortunately this second panic is catched by the recover of the stdlib in [net/http/server.go][0], so the node doesn't halt. To trigger the problem, you can run the following command: ```sh $ gnokey query -data xxx vm/qrender Post "http://127.0.0.1:26657": EOF ``` Because the handler behind `vm/qrender` expects to have some data in a particular format and it's not, it [panics with a string][2]. [0]: https://cs.opensource.google/go/go/+/refs/tags/go1.20.3:src/net/http/server.go;l=1851 [1]: https://github.com/gnolang/gno/blob/0eb5ff762eeb7e6b98d1f8afdb1214566352cabf/tm2/pkg/bft/rpc/lib/server/http_server.go#L164 [2]: https://github.com/gnolang/gno/blob/0eb5ff762eeb7e6b98d1f8afdb1214566352cabf/tm2/pkg/sdk/vm/handler.go#L131-L134 * fix lint
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.
Description
When panic is called with a string argument, that triggers an other panic in the recover handler, because it tries to convert the argument to an error type. Fortunately this second panic is catched by the recover of the stdlib in net/http/server.go, so the node doesn't halt.
How has this been tested?
To trigger the problem, you can run the following command:
$ gnokey query -data xxx vm/qrender Post "http://127.0.0.1:26657": EOFBecause the handler behind
vm/qrenderexpects to have some data in a particular format and it's not, it panics with a string.