Skip to content

Commit

Permalink
proto: mention field name in error message (#616)
Browse files Browse the repository at this point in the history
Instead of simply saying there was a UTF-8 validation error, specify
which field in which message had an issue.
  • Loading branch information
dsnet authored May 22, 2018
1 parent 70c277a commit 3a3da3a
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 8 deletions.
4 changes: 4 additions & 0 deletions proto/table_marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,10 @@ func (u *marshalInfo) marshal(b []byte, ptr pointer, deterministic bool) ([]byte
if err == errRepeatedHasNil {
err = errors.New("proto: repeated field " + f.name + " has nil element")
}
if err == errInvalidUTF8 {
fullName := revProtoTypes[reflect.PtrTo(u.typ)] + "." + f.name
err = fmt.Errorf("proto: string field %q contains invalid UTF-8", fullName)
}
return b, err
}
}
Expand Down
30 changes: 22 additions & 8 deletions proto/table_unmarshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ type unmarshalFieldInfo struct {

// if a required field, contains a single set bit at this field's index in the required field list.
reqMask uint64

name string // name of the field, for error reporting
}

var (
Expand Down Expand Up @@ -181,6 +183,10 @@ func (u *unmarshalInfo) unmarshal(m pointer, b []byte) error {
continue
}
if err != errInternalBadWireType {
if err == errInvalidUTF8 {
fullName := revProtoTypes[reflect.PtrTo(u.typ)] + "." + f.name
err = fmt.Errorf("proto: string field %q contains invalid UTF-8", fullName)
}
return err
}
// Fragments with bad wire type are treated as unknown fields.
Expand Down Expand Up @@ -351,7 +357,7 @@ func (u *unmarshalInfo) computeUnmarshalInfo() {
}

// Store the info in the correct slot in the message.
u.setTag(tag, toField(&f), unmarshal, reqMask)
u.setTag(tag, toField(&f), unmarshal, reqMask, name)
}

// Find any types associated with oneof fields.
Expand All @@ -366,10 +372,17 @@ func (u *unmarshalInfo) computeUnmarshalInfo() {

f := typ.Field(0) // oneof implementers have one field
baseUnmarshal := fieldUnmarshaler(&f)
tagstr := strings.Split(f.Tag.Get("protobuf"), ",")[1]
tag, err := strconv.Atoi(tagstr)
tags := strings.Split(f.Tag.Get("protobuf"), ",")
fieldNum, err := strconv.Atoi(tags[1])
if err != nil {
panic("protobuf tag field not an integer: " + tagstr)
panic("protobuf tag field not an integer: " + tags[1])
}
var name string
for _, tag := range tags {
if strings.HasPrefix(tag, "name=") {
name = strings.TrimPrefix(tag, "name=")
break
}
}

// Find the oneof field that this struct implements.
Expand All @@ -380,7 +393,7 @@ func (u *unmarshalInfo) computeUnmarshalInfo() {
// That lets us know where this struct should be stored
// when we encounter it during unmarshaling.
unmarshal := makeUnmarshalOneof(typ, of.ityp, baseUnmarshal)
u.setTag(tag, of.field, unmarshal, 0)
u.setTag(fieldNum, of.field, unmarshal, 0, name)
}
}
}
Expand All @@ -401,7 +414,7 @@ func (u *unmarshalInfo) computeUnmarshalInfo() {
// [0 0] is [tag=0/wiretype=varint varint-encoded-0].
u.setTag(0, zeroField, func(b []byte, f pointer, w int) ([]byte, error) {
return nil, fmt.Errorf("proto: %s: illegal tag 0 (wire type %d)", t, w)
}, 0)
}, 0, "")

// Set mask for required field check.
u.reqMask = uint64(1)<<uint(len(u.reqFields)) - 1
Expand All @@ -413,8 +426,9 @@ func (u *unmarshalInfo) computeUnmarshalInfo() {
// tag = tag # for field
// field/unmarshal = unmarshal info for that field.
// reqMask = if required, bitmask for field position in required field list. 0 otherwise.
func (u *unmarshalInfo) setTag(tag int, field field, unmarshal unmarshaler, reqMask uint64) {
i := unmarshalFieldInfo{field: field, unmarshal: unmarshal, reqMask: reqMask}
// name = short name of the field.
func (u *unmarshalInfo) setTag(tag int, field field, unmarshal unmarshaler, reqMask uint64, name string) {
i := unmarshalFieldInfo{field: field, unmarshal: unmarshal, reqMask: reqMask, name: name}
n := u.typ.NumField()
if tag >= 0 && (tag < 16 || tag < 2*n) { // TODO: what are the right numbers here?
for len(u.dense) <= tag {
Expand Down

0 comments on commit 3a3da3a

Please sign in to comment.