Skip to content

Commit

Permalink
all: return less-specific, but more informative wire unmarshal errors
Browse files Browse the repository at this point in the history 
When proto.Unmarshal fails, it is almost always due to being passed
input which is not a wire-format message. (A text-format message, a
message with framing left intact, and so forth.) An error like
"variable length integer overflow" can be confusing in this case, since
it implies the problem is the varint rather than the input being
entirely wrong.

Replace all Unmarshal parse errors with "cannot parse invalid
wire-format data".

Change-Id: Id97253bd39ac604e569df71778194f37b3c86c28
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/244297
Reviewed-by: Joe Tsai <joetsai@google.com>
  • Loading branch information
@neild
neild committed Jul 23, 2020
1 parent 28b807b commit b5523e3
Show file tree
Hide file tree
Showing 10 changed files with 260 additions and 257 deletions.
30 changes: 15 additions & 15 deletions internal/cmd/generate-types/impl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func consume{{.Name}}(b []byte, p pointer, wtyp protowire.Type, f *coderFieldInf
}
{{template "Consume" .}}
if n < 0 {
return out, protowire.ParseError(n)
return out, errDecode
}
*p.{{.GoType.PointerMethod}}() = {{.ToGoType}}
out.n = n
Expand Down Expand Up @@ -147,7 +147,7 @@ func consume{{.Name}}ValidateUTF8(b []byte, p pointer, wtyp protowire.Type, f *c
}
{{template "Consume" .}}
if n < 0 {
return out, protowire.ParseError(n)
return out, errDecode
}
if !utf8.Valid{{if eq .Name "String"}}String{{end}}(v) {
return out, errInvalidUTF8{}
Expand Down Expand Up @@ -196,7 +196,7 @@ func consume{{.Name}}NoZero(b []byte, p pointer, wtyp protowire.Type, f *coderFi
}
{{template "Consume" .}}
if n < 0 {
return out, protowire.ParseError(n)
return out, errDecode
}
*p.{{.GoType.PointerMethod}}() = {{.ToGoTypeNoZero}}
out.n = n
Expand Down Expand Up @@ -235,7 +235,7 @@ func consume{{.Name}}NoZeroValidateUTF8(b []byte, p pointer, wtyp protowire.Type
}
{{template "Consume" .}}
if n < 0 {
return out, protowire.ParseError(n)
return out, errDecode
}
if !utf8.Valid{{if eq .Name "String"}}String{{end}}(v) {
return out, errInvalidUTF8{}
Expand Down Expand Up @@ -280,7 +280,7 @@ func consume{{.Name}}Ptr(b []byte, p pointer, wtyp protowire.Type, f *coderField
}
{{template "Consume" .}}
if n < 0 {
return out, protowire.ParseError(n)
return out, errDecode
}
vp := p.{{.GoType.PointerMethod}}Ptr()
if *vp == nil {
Expand Down Expand Up @@ -319,7 +319,7 @@ func consume{{.Name}}PtrValidateUTF8(b []byte, p pointer, wtyp protowire.Type, f
}
{{template "Consume" .}}
if n < 0 {
return out, protowire.ParseError(n)
return out, errDecode
}
if !utf8.Valid{{if eq .Name "String"}}String{{end}}(v) {
return out, errInvalidUTF8{}
Expand Down Expand Up @@ -372,12 +372,12 @@ func consume{{.Name}}Slice(b []byte, p pointer, wtyp protowire.Type, f *coderFie
s := *sp
b, n := protowire.ConsumeBytes(b)
if n < 0 {
return out, protowire.ParseError(n)
return out, errDecode
}
for len(b) > 0 {
{{template "Consume" .}}
if n < 0 {
return out, protowire.ParseError(n)
return out, errDecode
}
s = append(s, {{.ToGoType}})
b = b[n:]
Expand All @@ -392,7 +392,7 @@ func consume{{.Name}}Slice(b []byte, p pointer, wtyp protowire.Type, f *coderFie
}
{{template "Consume" .}}
if n < 0 {
return out, protowire.ParseError(n)
return out, errDecode
}
*sp = append(*sp, {{.ToGoType}})
out.n = n
Expand Down Expand Up @@ -428,7 +428,7 @@ func consume{{.Name}}SliceValidateUTF8(b []byte, p pointer, wtyp protowire.Type,
}
{{template "Consume" .}}
if n < 0 {
return out, protowire.ParseError(n)
return out, errDecode
}
if !utf8.Valid{{if eq .Name "String"}}String{{end}}(v) {
return out, errInvalidUTF8{}
Expand Down Expand Up @@ -516,7 +516,7 @@ func consume{{.Name}}Value(b []byte, _ protoreflect.Value, _ protowire.Number, w
}
{{template "Consume" .}}
if n < 0 {
return protoreflect.Value{}, out, protowire.ParseError(n)
return protoreflect.Value{}, out, errDecode
}
out.n = n
return {{.ToValue}}, out, nil
Expand Down Expand Up @@ -551,7 +551,7 @@ func consume{{.Name}}ValueValidateUTF8(b []byte, _ protoreflect.Value, _ protowi
}
{{template "Consume" .}}
if n < 0 {
return protoreflect.Value{}, out, protowire.ParseError(n)
return protoreflect.Value{}, out, errDecode
}
if !utf8.ValidString(v) {
return protoreflect.Value{}, out, errInvalidUTF8{}
Expand Down Expand Up @@ -600,12 +600,12 @@ func consume{{.Name}}SliceValue(b []byte, listv protoreflect.Value, _ protowire.
if wtyp == protowire.BytesType {
b, n := protowire.ConsumeBytes(b)
if n < 0 {
return protoreflect.Value{}, out, protowire.ParseError(n)
return protoreflect.Value{}, out, errDecode
}
for len(b) > 0 {
{{template "Consume" .}}
if n < 0 {
return protoreflect.Value{}, out, protowire.ParseError(n)
return protoreflect.Value{}, out, errDecode
}
list.Append({{.ToValue}})
b = b[n:]
Expand All @@ -619,7 +619,7 @@ func consume{{.Name}}SliceValue(b []byte, listv protoreflect.Value, _ protowire.
}
{{template "Consume" .}}
if n < 0 {
return protoreflect.Value{}, out, protowire.ParseError(n)
return protoreflect.Value{}, out, errDecode
}
list.Append({{.ToValue}})
out.n = n
Expand Down
8 changes: 4 additions & 4 deletions internal/cmd/generate-types/proto.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ func (o UnmarshalOptions) unmarshalScalar(b []byte, wtyp protowire.Type, fd prot
v, n := protowire.Consume{{.WireType}}(b)
{{- end}}
if n < 0 {
return val, 0, protowire.ParseError(n)
return val, 0, errDecode
}
{{if (eq .Name "String") -}}
if strs.EnforceUTF8(fd) && !utf8.Valid(v) {
Expand All @@ -312,12 +312,12 @@ func (o UnmarshalOptions) unmarshalList(b []byte, wtyp protowire.Type, list prot
if wtyp == protowire.BytesType {
buf, n := protowire.ConsumeBytes(b)
if n < 0 {
return 0, protowire.ParseError(n)
return 0, errDecode
}
for len(buf) > 0 {
v, n := protowire.Consume{{.WireType}}(buf)
if n < 0 {
return 0, protowire.ParseError(n)
return 0, errDecode
}
buf = buf[n:]
list.Append({{.ToValue}})
Expand All @@ -334,7 +334,7 @@ func (o UnmarshalOptions) unmarshalList(b []byte, wtyp protowire.Type, list prot
v, n := protowire.Consume{{.WireType}}(b)
{{- end}}
if n < 0 {
return 0, protowire.ParseError(n)
return 0, errDecode
}
{{if (eq .Name "String") -}}
if strs.EnforceUTF8(fd) && !utf8.Valid(v) {
Expand Down
16 changes: 8 additions & 8 deletions internal/impl/codec_field.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ func consumeMessageInfo(b []byte, p pointer, wtyp protowire.Type, f *coderFieldI
}
v, n := protowire.ConsumeBytes(b)
if n < 0 {
return out, protowire.ParseError(n)
return out, errDecode
}
if p.Elem().IsNil() {
p.SetPointer(pointerOfValue(reflect.New(f.mi.GoReflectType.Elem())))
Expand Down Expand Up @@ -276,7 +276,7 @@ func consumeMessage(b []byte, m proto.Message, wtyp protowire.Type, opts unmarsh
}
v, n := protowire.ConsumeBytes(b)
if n < 0 {
return out, protowire.ParseError(n)
return out, errDecode
}
o, err := opts.Options().UnmarshalState(piface.UnmarshalInput{
Buf: v,
Expand Down Expand Up @@ -420,7 +420,7 @@ func consumeGroup(b []byte, m proto.Message, num protowire.Number, wtyp protowir
}
b, n := protowire.ConsumeGroup(num, b)
if n < 0 {
return out, protowire.ParseError(n)
return out, errDecode
}
o, err := opts.Options().UnmarshalState(piface.UnmarshalInput{
Buf: b,
Expand Down Expand Up @@ -494,7 +494,7 @@ func consumeMessageSliceInfo(b []byte, p pointer, wtyp protowire.Type, f *coderF
}
v, n := protowire.ConsumeBytes(b)
if n < 0 {
return out, protowire.ParseError(n)
return out, errDecode
}
m := reflect.New(f.mi.GoReflectType.Elem()).Interface()
mp := pointerOfIface(m)
Expand Down Expand Up @@ -550,7 +550,7 @@ func consumeMessageSlice(b []byte, p pointer, goType reflect.Type, wtyp protowir
}
v, n := protowire.ConsumeBytes(b)
if n < 0 {
return out, protowire.ParseError(n)
return out, errDecode
}
mp := reflect.New(goType.Elem())
o, err := opts.Options().UnmarshalState(piface.UnmarshalInput{
Expand Down Expand Up @@ -613,7 +613,7 @@ func consumeMessageSliceValue(b []byte, listv pref.Value, _ protowire.Number, wt
}
v, n := protowire.ConsumeBytes(b)
if n < 0 {
return pref.Value{}, out, protowire.ParseError(n)
return pref.Value{}, out, errDecode
}
m := list.NewElement()
o, err := opts.Options().UnmarshalState(piface.UnmarshalInput{
Expand Down Expand Up @@ -681,7 +681,7 @@ func consumeGroupSliceValue(b []byte, listv pref.Value, num protowire.Number, wt
}
b, n := protowire.ConsumeGroup(num, b)
if n < 0 {
return pref.Value{}, out, protowire.ParseError(n)
return pref.Value{}, out, errDecode
}
m := list.NewElement()
o, err := opts.Options().UnmarshalState(piface.UnmarshalInput{
Expand Down Expand Up @@ -767,7 +767,7 @@ func consumeGroupSlice(b []byte, p pointer, num protowire.Number, wtyp protowire
}
b, n := protowire.ConsumeGroup(num, b)
if n < 0 {
return out, protowire.ParseError(n)
return out, errDecode
}
mp := reflect.New(goType.Elem())
o, err := opts.Options().UnmarshalState(piface.UnmarshalInput{
Expand Down
Loading

0 comments on commit b5523e3

Please sign in to comment.