encoding/asn1: error instead of panic on invalid value to Unmarshal

Changes Unmarshal to return an error, instead of
panicking when its value is nil or not a pointer.

This change matches the behavior of other encoding
packages like json.

Fixes #41509.

Change-Id: I92c3af3a960144566e4c2b55d00c3a6fe477c8d5
GitHub-Last-Rev: c668b6e4ad826f84542c2675eb31ccfb010c45bb
GitHub-Pull-Request: golang/go#41485
Reviewed-on: https://go-review.googlesource.com/c/go/+/255881
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Trust: Emmanuel Odeke <emm.odeke@gmail.com>

diff --git a/src/encoding/asn1/asn1.go b/src/encoding/asn1/asn1.go
index d809dde2781cd807566093f96d3b0b6391d33fc5..fa3d4e327b83df4559a18c140e26133408c07d0f 100644 (file)
--- a/src/encoding/asn1/asn1.go
+++ b/src/encoding/asn1/asn1.go
@@ -1035,7 +1035,8 @@ func setDefaultValue(v reflect.Value, params fieldParameters) (ok bool) {
 // Unmarshal parses the DER-encoded ASN.1 data structure b
 // and uses the reflect package to fill in an arbitrary value pointed at by val.
 // Because Unmarshal uses the reflect package, the structs
-// being written to must use upper case field names.
+// being written to must use upper case field names. If val
+// is nil or not a pointer, Unmarshal returns an error.
 //
 // After parsing b, any bytes that were leftover and not used to fill
 // val will be returned in rest. When parsing a SEQUENCE into a struct,
@@ -1095,11 +1096,31 @@ func Unmarshal(b []byte, val interface{}) (rest []byte, err error) {
        return UnmarshalWithParams(b, val, "")
 }
 
+// An invalidUnmarshalError describes an invalid argument passed to Unmarshal.
+// (The argument to Unmarshal must be a non-nil pointer.)
+type invalidUnmarshalError struct {
+       Type reflect.Type
+}
+
+func (e *invalidUnmarshalError) Error() string {
+       if e.Type == nil {
+               return "asn1: Unmarshal recipient value is nil"
+       }
+
+       if e.Type.Kind() != reflect.Ptr {
+               return "asn1: Unmarshal recipient value is non-pointer " + e.Type.String()
+       }
+       return "asn1: Unmarshal recipient value is nil " + e.Type.String()
+}
+
 // UnmarshalWithParams allows field parameters to be specified for the
 // top-level element. The form of the params is the same as the field tags.
 func UnmarshalWithParams(b []byte, val interface{}, params string) (rest []byte, err error) {
-       v := reflect.ValueOf(val).Elem()
-       offset, err := parseField(v, b, 0, parseFieldParameters(params))
+       v := reflect.ValueOf(val)
+       if v.Kind() != reflect.Ptr || v.IsNil() {
+               return nil, &invalidUnmarshalError{reflect.TypeOf(val)}
+       }
+       offset, err := parseField(v.Elem(), b, 0, parseFieldParameters(params))
        if err != nil {
                return nil, err
        }

diff --git a/src/encoding/asn1/asn1_test.go b/src/encoding/asn1/asn1_test.go
index 8daae97faad4003990b899a7b205dcd46ab99c7e..8985538468e6b37ce6e9b094e163239dd42386b4 100644 (file)
--- a/src/encoding/asn1/asn1_test.go
+++ b/src/encoding/asn1/asn1_test.go
@@ -518,6 +518,29 @@ func TestUnmarshal(t *testing.T) {
        }
 }
 
+func TestUnmarshalWithNilOrNonPointer(t *testing.T) {
+       tests := []struct {
+               b    []byte
+               v    interface{}
+               want string
+       }{
+               {b: []byte{0x05, 0x00}, v: nil, want: "asn1: Unmarshal recipient value is nil"},
+               {b: []byte{0x05, 0x00}, v: RawValue{}, want: "asn1: Unmarshal recipient value is non-pointer asn1.RawValue"},
+               {b: []byte{0x05, 0x00}, v: (*RawValue)(nil), want: "asn1: Unmarshal recipient value is nil *asn1.RawValue"},
+       }
+
+       for _, test := range tests {
+               _, err := Unmarshal(test.b, test.v)
+               if err == nil {
+                       t.Errorf("Unmarshal expecting error, got nil")
+                       continue
+               }
+               if g, w := err.Error(), test.want; g != w {
+                       t.Errorf("InvalidUnmarshalError mismatch\nGot:  %q\nWant: %q", g, w)
+               }
+       }
+}
+
 type Certificate struct {
        TBSCertificate     TBSCertificate
        SignatureAlgorithm AlgorithmIdentifier