Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

encoding/json: invert panic-recover capture type #23012

Closed
dsnet opened this issue Dec 6, 2017 · 1 comment

Comments

Projects
None yet
2 participants
@dsnet
Copy link
Member

commented Dec 6, 2017

The current logic in json/decode.go does the following:

if r := recover(); r != nil {
if _, ok := r.(runtime.Error); ok {
panic(r)
}
err = r.(error)
}

This captures any panics that aren't runtime.Error, which is arguably a wider range of panics than should be captured. For example, a panic in the reflect package gets masked away as an error.

The logic in decodeState.error should wrap the error before panicking to ensure the recover only captures errors originating from the json package:

// error aborts the decoding by panicking with err.
func (d *decodeState) error(err error) {
panic(d.addErrorContext(err))
}

@dsnet dsnet self-assigned this Dec 6, 2017

@dsnet dsnet added this to the Go1.11 milestone Dec 6, 2017

@gopherbot

This comment has been minimized.

Copy link

commented Feb 14, 2018

Change https://golang.org/cl/94019 mentions this issue: encoding/json: make error capture logic in recover more type safe

@gopherbot gopherbot closed this in 91a6a2a Feb 14, 2018

@golang golang locked and limited conversation to collaborators Feb 14, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.