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 <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/encoding/json/decode.go b/src/encoding/json/decode.go
index 536f25dc7c5eee9f5c995ef6c8437ea9c07a7370..f08b0a1c5890694132fdc8ea3017e30192fd857d 100644 (file)
--- a/src/encoding/json/decode.go
+++ b/src/encoding/json/decode.go
@@ -14,7 +14,6 @@ import (
        "errors"
        "fmt"
        "reflect"
-       "runtime"
        "strconv"
        "unicode"
        "unicode/utf16"
@@ -168,13 +167,19 @@ func (e *InvalidUnmarshalError) Error() string {
        return "json: Unmarshal(nil " + e.Type.String() + ")"
 }
 
+// jsonError is an error wrapper type for internal use only.
+// Panics with errors are wrapped in jsonError so that the top-level recover
+// can distinguish intentional panics from this package.
+type jsonError struct{ error }
+
 func (d *decodeState) unmarshal(v interface{}) (err error) {
        defer func() {
                if r := recover(); r != nil {
-                       if _, ok := r.(runtime.Error); ok {
+                       if je, ok := r.(jsonError); ok {
+                               err = je.error
+                       } else {
                                panic(r)
                        }
-                       err = r.(error)
                }
        }()
 
@@ -295,9 +300,9 @@ func (d *decodeState) init(data []byte) *decodeState {
        return d
 }
 
-// error aborts the decoding by panicking with err.
+// error aborts the decoding by panicking with err wrapped in jsonError.
 func (d *decodeState) error(err error) {
-       panic(d.addErrorContext(err))
+       panic(jsonError{d.addErrorContext(err)})
 }
 
 // saveError saves the first err it is called with,

diff --git a/src/encoding/json/decode_test.go b/src/encoding/json/decode_test.go
index 34b7ec6d97abc8fe50a302df253cca7371b586ae..90fdf93dbdf6bfcffecbf7c40e6242314f1c551c 100644 (file)
--- a/src/encoding/json/decode_test.go
+++ b/src/encoding/json/decode_test.go
@@ -2166,3 +2166,17 @@ func TestUnmarshalEmbeddedPointerUnexported(t *testing.T) {
                }
        }
 }
+
+type unmarshalPanic struct{}
+
+func (unmarshalPanic) UnmarshalJSON([]byte) error { panic(0xdead) }
+
+func TestUnmarshalPanic(t *testing.T) {
+       defer func() {
+               if got := recover(); !reflect.DeepEqual(got, 0xdead) {
+                       t.Errorf("panic() = (%T)(%v), want 0xdead", got, got)
+               }
+       }()
+       Unmarshal([]byte("{}"), &unmarshalPanic{})
+       t.Fatalf("Unmarshal should have panicked")
+}

diff --git a/src/encoding/json/encode.go b/src/encoding/json/encode.go
index 1e45e445d92163f9d756c7ddee66a16e816c483f..68512d0225a8684c7a4bb8a4aef37a52b5a1b6b7 100644 (file)
--- a/src/encoding/json/encode.go
+++ b/src/encoding/json/encode.go
@@ -17,7 +17,6 @@ import (
        "fmt"
        "math"
        "reflect"
-       "runtime"
        "sort"
        "strconv"
        "strings"
@@ -286,21 +285,20 @@ func newEncodeState() *encodeState {
 func (e *encodeState) marshal(v interface{}, opts encOpts) (err error) {
        defer func() {
                if r := recover(); r != nil {
-                       if _, ok := r.(runtime.Error); ok {
+                       if je, ok := r.(jsonError); ok {
+                               err = je.error
+                       } else {
                                panic(r)
                        }
-                       if s, ok := r.(string); ok {
-                               panic(s)
-                       }
-                       err = r.(error)
                }
        }()
        e.reflectValue(reflect.ValueOf(v), opts)
        return nil
 }
 
+// error aborts the encoding by panicking with err wrapped in jsonError.
 func (e *encodeState) error(err error) {
-       panic(err)
+       panic(jsonError{err})
 }
 
 func isEmptyValue(v reflect.Value) bool {

diff --git a/src/encoding/json/encode_test.go b/src/encoding/json/encode_test.go
index 0f194e13d2681f33e8b3752ec276fd851640ceca..b90483cf35f91d81d767aaee78e666e9804e0d7c 100644 (file)
--- a/src/encoding/json/encode_test.go
+++ b/src/encoding/json/encode_test.go
@@ -981,3 +981,17 @@ func TestMarshalRawMessageValue(t *testing.T) {
                }
        }
 }
+
+type marshalPanic struct{}
+
+func (marshalPanic) MarshalJSON() ([]byte, error) { panic(0xdead) }
+
+func TestMarshalPanic(t *testing.T) {
+       defer func() {
+               if got := recover(); !reflect.DeepEqual(got, 0xdead) {
+                       t.Errorf("panic() = (%T)(%v), want 0xdead", got, got)
+               }
+       }()
+       Marshal(&marshalPanic{})
+       t.Error("Marshal should have panicked")
+}