]> Sergey Matveev's repositories - btrtrc.git/commitdiff
Don't panic on int parse failures
authorMatt Joiner <anacrolix@gmail.com>
Thu, 12 Aug 2021 03:46:02 +0000 (13:46 +1000)
committerMatt Joiner <anacrolix@gmail.com>
Thu, 12 Aug 2021 03:46:02 +0000 (13:46 +1000)
This means for UnmarshalTypeErrors we now include context. There are still some other error types remaining that are thrown up via panic.

bencode/api.go
bencode/decode.go
bencode/decode_test.go

index 80d9951bc6061654c591cc94896f023a4565debb..3c379abbf837355da04bf1cd9cd7522f0b110619 100644 (file)
@@ -41,12 +41,18 @@ func (e *UnmarshalInvalidArgError) Error() string {
 
 // Unmarshaler spotted a value that was not appropriate for a given Go value.
 type UnmarshalTypeError struct {
-       Value string
-       Type  reflect.Type
+       BencodeTypeName     string
+       UnmarshalTargetType reflect.Type
 }
 
+// This could probably be a value type, but we may already have users assuming
+// that it's passed by pointer.
 func (e *UnmarshalTypeError) Error() string {
-       return fmt.Sprintf("cannot unmarshal a bencode %s into a %s", e.Value, e.Type)
+       return fmt.Sprintf(
+               "can't unmarshal a bencode %v into a %v",
+               e.BencodeTypeName,
+               e.UnmarshalTargetType,
+       )
 }
 
 // Unmarshaler tried to write to an unexported (therefore unwritable) field.
index f8ec82955a40ee4d1d9c0955fe19f45d1be2af25..5dbcbe5a831f253e6dd79e561487e1553672923d 100644 (file)
@@ -102,7 +102,7 @@ func (d *Decoder) throwSyntaxError(offset int64, err error) {
 }
 
 // called when 'i' was consumed
-func (d *Decoder) parseInt(v reflect.Value) {
+func (d *Decoder) parseInt(v reflect.Value) error {
        start := d.Offset - 1
        d.readUntil('e')
        if d.buf.Len() == 0 {
@@ -120,10 +120,10 @@ func (d *Decoder) parseInt(v reflect.Value) {
                checkForIntParseError(err, start)
 
                if v.OverflowInt(n) {
-                       panic(&UnmarshalTypeError{
-                               Value: "integer " + s,
-                               Type:  v.Type(),
-                       })
+                       return &UnmarshalTypeError{
+                               BencodeTypeName:     "int",
+                               UnmarshalTargetType: v.Type(),
+                       }
                }
                v.SetInt(n)
        case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
@@ -131,21 +131,22 @@ func (d *Decoder) parseInt(v reflect.Value) {
                checkForIntParseError(err, start)
 
                if v.OverflowUint(n) {
-                       panic(&UnmarshalTypeError{
-                               Value: "integer " + s,
-                               Type:  v.Type(),
-                       })
+                       return &UnmarshalTypeError{
+                               BencodeTypeName:     "int",
+                               UnmarshalTargetType: v.Type(),
+                       }
                }
                v.SetUint(n)
        case reflect.Bool:
                v.SetBool(s != "0")
        default:
-               panic(&UnmarshalTypeError{
-                       Value: "integer " + s,
-                       Type:  v.Type(),
-               })
+               return &UnmarshalTypeError{
+                       BencodeTypeName:     "int",
+                       UnmarshalTargetType: v.Type(),
+               }
        }
        d.buf.Reset()
+       return nil
 }
 
 func (d *Decoder) parseString(v reflect.Value) error {
@@ -198,8 +199,8 @@ func (d *Decoder) parseString(v reflect.Value) error {
        read(d.buf.Bytes()[:length])
        // I believe we return here to support "ignore_unmarshal_type_error".
        return &UnmarshalTypeError{
-               Value: "string",
-               Type:  v.Type(),
+               BencodeTypeName:     "string",
+               UnmarshalTargetType: v.Type(),
        }
 }
 
@@ -238,7 +239,7 @@ func getDictField(dict reflect.Type, key string) (_ dictField, err error) {
                //      })
                //}
        default:
-               err = fmt.Errorf("can't parse bencode dict items into %v", k)
+               err = fmt.Errorf("can't assign bencode dict items into a %v", k)
                return
        }
 }
@@ -347,8 +348,9 @@ func (d *Decoder) parseDict(v reflect.Value) error {
                //log.Printf("parsing into %v", setValue.Type())
                ok, err = d.parseValue(setValue)
                if err != nil {
-                       if _, ok := err.(*UnmarshalTypeError); !ok || !df.Tags.IgnoreUnmarshalTypeError() {
-                               return fmt.Errorf("parsing value for key %q: %s", keyStr, err)
+                       var target *UnmarshalTypeError
+                       if !(errors.As(err, &target) && df.Tags.IgnoreUnmarshalTypeError()) {
+                               return fmt.Errorf("parsing value for key %q: %w", keyStr, err)
                        }
                }
                if !ok {
@@ -369,8 +371,8 @@ func (d *Decoder) parseList(v reflect.Value) error {
                }
                if l.Elem().Len() != 1 {
                        return &UnmarshalTypeError{
-                               Value: "list",
-                               Type:  v.Type(),
+                               BencodeTypeName:     "list",
+                               UnmarshalTargetType: v.Type(),
                        }
                }
                v.Set(l.Elem().Index(0))
@@ -527,8 +529,7 @@ func (d *Decoder) parseValue(v reflect.Value) (bool, error) {
        case 'l':
                return true, d.parseList(v)
        case 'i':
-               d.parseInt(v)
-               return true, nil
+               return true, d.parseInt(v)
        default:
                if b >= '0' && b <= '9' {
                        // It's a string.
index dc610daf4f8d646dff04f1c32b6a5ef45571af63..e9de91bd6c4c09c06e44465bea9369fa66f931a4 100644 (file)
@@ -183,3 +183,12 @@ func TestDecodeDictIntoUnsupported(t *testing.T) {
        t.Log(err)
        c.Check(err, qt.Not(qt.IsNil))
 }
+
+func TestUnmarshalDictKeyNotString(t *testing.T) {
+       // Any type that a dict shouldn't be unmarshallable into.
+       var i int
+       c := qt.New(t)
+       err := Unmarshal([]byte("di42e3:yese"), &i)
+       t.Log(err)
+       c.Check(err, qt.Not(qt.IsNil))
+}