Skip to content

Commit 91a6a2a

Browse files
committed
encoding/json: make error capture logic in recover more type safe
Rather than only ignoring runtime.Error panics, which are a very narrow set of possible panic values, switch it such that the json package only captures panic values that have been properly wrapped in a jsonError struct. This ensures that only intentional panics originating from the json package are captured. Fixes #23012 Change-Id: I5e85200259edd2abb1b0512ce6cc288849151a6d Reviewed-on: https://go-review.googlesource.com/94019 Run-TryBot: Joe Tsai <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]>
1 parent 70a04f6 commit 91a6a2a

File tree

4 files changed

+43
-12
lines changed

4 files changed

+43
-12
lines changed

src/encoding/json/decode.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"errors"
1515
"fmt"
1616
"reflect"
17-
"runtime"
1817
"strconv"
1918
"unicode"
2019
"unicode/utf16"
@@ -168,13 +167,19 @@ func (e *InvalidUnmarshalError) Error() string {
168167
return "json: Unmarshal(nil " + e.Type.String() + ")"
169168
}
170169

170+
// jsonError is an error wrapper type for internal use only.
171+
// Panics with errors are wrapped in jsonError so that the top-level recover
172+
// can distinguish intentional panics from this package.
173+
type jsonError struct{ error }
174+
171175
func (d *decodeState) unmarshal(v interface{}) (err error) {
172176
defer func() {
173177
if r := recover(); r != nil {
174-
if _, ok := r.(runtime.Error); ok {
178+
if je, ok := r.(jsonError); ok {
179+
err = je.error
180+
} else {
175181
panic(r)
176182
}
177-
err = r.(error)
178183
}
179184
}()
180185

@@ -295,9 +300,9 @@ func (d *decodeState) init(data []byte) *decodeState {
295300
return d
296301
}
297302

298-
// error aborts the decoding by panicking with err.
303+
// error aborts the decoding by panicking with err wrapped in jsonError.
299304
func (d *decodeState) error(err error) {
300-
panic(d.addErrorContext(err))
305+
panic(jsonError{d.addErrorContext(err)})
301306
}
302307

303308
// saveError saves the first err it is called with,

src/encoding/json/decode_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2166,3 +2166,17 @@ func TestUnmarshalEmbeddedPointerUnexported(t *testing.T) {
21662166
}
21672167
}
21682168
}
2169+
2170+
type unmarshalPanic struct{}
2171+
2172+
func (unmarshalPanic) UnmarshalJSON([]byte) error { panic(0xdead) }
2173+
2174+
func TestUnmarshalPanic(t *testing.T) {
2175+
defer func() {
2176+
if got := recover(); !reflect.DeepEqual(got, 0xdead) {
2177+
t.Errorf("panic() = (%T)(%v), want 0xdead", got, got)
2178+
}
2179+
}()
2180+
Unmarshal([]byte("{}"), &unmarshalPanic{})
2181+
t.Fatalf("Unmarshal should have panicked")
2182+
}

src/encoding/json/encode.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"fmt"
1818
"math"
1919
"reflect"
20-
"runtime"
2120
"sort"
2221
"strconv"
2322
"strings"
@@ -286,21 +285,20 @@ func newEncodeState() *encodeState {
286285
func (e *encodeState) marshal(v interface{}, opts encOpts) (err error) {
287286
defer func() {
288287
if r := recover(); r != nil {
289-
if _, ok := r.(runtime.Error); ok {
288+
if je, ok := r.(jsonError); ok {
289+
err = je.error
290+
} else {
290291
panic(r)
291292
}
292-
if s, ok := r.(string); ok {
293-
panic(s)
294-
}
295-
err = r.(error)
296293
}
297294
}()
298295
e.reflectValue(reflect.ValueOf(v), opts)
299296
return nil
300297
}
301298

299+
// error aborts the encoding by panicking with err wrapped in jsonError.
302300
func (e *encodeState) error(err error) {
303-
panic(err)
301+
panic(jsonError{err})
304302
}
305303

306304
func isEmptyValue(v reflect.Value) bool {

src/encoding/json/encode_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -981,3 +981,17 @@ func TestMarshalRawMessageValue(t *testing.T) {
981981
}
982982
}
983983
}
984+
985+
type marshalPanic struct{}
986+
987+
func (marshalPanic) MarshalJSON() ([]byte, error) { panic(0xdead) }
988+
989+
func TestMarshalPanic(t *testing.T) {
990+
defer func() {
991+
if got := recover(); !reflect.DeepEqual(got, 0xdead) {
992+
t.Errorf("panic() = (%T)(%v), want 0xdead", got, got)
993+
}
994+
}()
995+
Marshal(&marshalPanic{})
996+
t.Error("Marshal should have panicked")
997+
}

0 commit comments

Comments
 (0)