all: make error messages unstable
Use internal/detrand in the construction of our error messages.
This alters whether there is one or two spaces following the "proto:" prefix.
While it is easy for users to still work around this mutation,
sit at least forces them to write test infrastructure to more fuzzily
match on error strings.
Change-Id: I4ddca717526ee3fc4dbb1e0b36cfca8c6e0df36d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/194038
Reviewed-by: Herbie Ong <herbie@google.com>
diff --git a/internal/cmd/pbdump/pbdump.go b/internal/cmd/pbdump/pbdump.go
index 71912b1..62e3a27 100644
--- a/internal/cmd/pbdump/pbdump.go
+++ b/internal/cmd/pbdump/pbdump.go
@@ -19,6 +19,7 @@
"google.golang.org/protobuf/internal/encoding/pack"
"google.golang.org/protobuf/internal/encoding/wire"
+ "google.golang.org/protobuf/internal/errors"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protodesc"
"google.golang.org/protobuf/reflect/protoreflect"
@@ -174,7 +175,7 @@
n, _ := strconv.ParseInt(s[:i], 10, 32)
num := wire.Number(n)
if num < wire.MinValidNumber || wire.MaxValidNumber < num {
- return fmt.Errorf("invalid field: %v", prefix)
+ return errors.New("invalid field: %v", prefix)
}
s = strings.TrimPrefix(s[i:], ".")
@@ -184,7 +185,7 @@
}
if len(s) == 0 {
if fs[num].kind.IsValid() {
- return fmt.Errorf("field %v already set as %v type", prefix, fs[num].kind)
+ return errors.New("field %v already set as %v type", prefix, fs[num].kind)
}
fs[num].kind = k
}
@@ -195,7 +196,7 @@
// Verify that only messages or groups can have sub-fields.
k2 := fs[num].kind
if k2 > 0 && k2 != protoreflect.MessageKind && k2 != protoreflect.GroupKind && len(fs[num].sub) > 0 {
- return fmt.Errorf("field %v of %v type cannot have sub-fields", prefix, k2)
+ return errors.New("field %v of %v type cannot have sub-fields", prefix, k2)
}
return nil
}
diff --git a/internal/errors/errors.go b/internal/errors/errors.go
index e22035e..db1005d 100644
--- a/internal/errors/errors.go
+++ b/internal/errors/errors.go
@@ -7,6 +7,8 @@
import (
"fmt"
+
+ "google.golang.org/protobuf/internal/detrand"
)
// New formats a string according to the format specifier and arguments and
@@ -22,7 +24,16 @@
type prefixError struct{ s string }
-func (e *prefixError) Error() string { return "proto: " + e.s }
+var prefix = func() string {
+ if detrand.Bool() {
+ return "proto: "
+ }
+ return "proto: "
+}()
+
+func (e *prefixError) Error() string {
+ return prefix + e.s
+}
func InvalidUTF8(name string) error {
return New("field %v contains invalid UTF-8", name)
diff --git a/internal/impl/codec_tables.go b/internal/impl/codec_tables.go
index 77fc4c0..f421c78 100644
--- a/internal/impl/codec_tables.go
+++ b/internal/impl/codec_tables.go
@@ -423,7 +423,7 @@
}
}
}
- panic(fmt.Errorf("invalid type: no encoder for %v %v %v/%v", fd.FullName(), fd.Cardinality(), fd.Kind(), ft))
+ panic(fmt.Sprintf("invalid type: no encoder for %v %v %v/%v", fd.FullName(), fd.Cardinality(), fd.Kind(), ft))
}
// encoderFuncsForValue returns value functions for a field, used for
@@ -547,5 +547,5 @@
return coderGroupValue
}
}
- panic(fmt.Errorf("invalid field: no encoder for %v %v %v", fd.FullName(), fd.Cardinality(), fd.Kind()))
+ panic(fmt.Sprintf("invalid field: no encoder for %v %v %v", fd.FullName(), fd.Cardinality(), fd.Kind()))
}
diff --git a/internal/impl/message_reflect.go b/internal/impl/message_reflect.go
index b34e479..04e8f7c 100644
--- a/internal/impl/message_reflect.go
+++ b/internal/impl/message_reflect.go
@@ -145,7 +145,7 @@
}
func (m *extensionMap) Set(xt pref.ExtensionType, v pref.Value) {
if !xt.IsValidValue(v) {
- panic(fmt.Errorf("%v: assigning invalid value", xt.TypeDescriptor().FullName()))
+ panic(fmt.Sprintf("%v: assigning invalid value", xt.TypeDescriptor().FullName()))
}
if *m == nil {
*m = make(map[int32]ExtensionField)
diff --git a/proto/isinit_test.go b/proto/isinit_test.go
index 72dd108..d935d3a 100644
--- a/proto/isinit_test.go
+++ b/proto/isinit_test.go
@@ -6,6 +6,7 @@
import (
"fmt"
+ "strings"
"testing"
"google.golang.org/protobuf/proto"
@@ -20,13 +21,13 @@
}{
{
&testpb.TestRequired{},
- `proto: required field goproto.proto.test.TestRequired.required_field not set`,
+ `goproto.proto.test.TestRequired.required_field`,
},
{
&testpb.TestRequiredForeign{
OptionalMessage: &testpb.TestRequired{},
},
- `proto: required field goproto.proto.test.TestRequired.required_field not set`,
+ `goproto.proto.test.TestRequired.required_field`,
},
{
&testpb.TestRequiredForeign{
@@ -35,7 +36,7 @@
{},
},
},
- `proto: required field goproto.proto.test.TestRequired.required_field not set`,
+ `goproto.proto.test.TestRequired.required_field`,
},
{
&testpb.TestRequiredForeign{
@@ -43,7 +44,7 @@
1: {},
},
},
- `proto: required field goproto.proto.test.TestRequired.required_field not set`,
+ `goproto.proto.test.TestRequired.required_field`,
},
} {
err := proto.IsInitialized(test.m)
@@ -51,9 +52,8 @@
if err != nil {
got = fmt.Sprintf("%q", err)
}
- want := fmt.Sprintf("%q", test.want)
- if got != want {
- t.Errorf("IsInitialized(m):\n got: %v\nwant: %v\nMessage:\n%v", got, want, marshalText(test.m))
+ if !strings.Contains(got, test.want) {
+ t.Errorf("IsInitialized(m):\n got: %v\nwant contains: %v\nMessage:\n%v", got, test.want, marshalText(test.m))
}
}
}
diff --git a/reflect/protodesc/file_test.go b/reflect/protodesc/file_test.go
index 5319b52..38e8b4c 100644
--- a/reflect/protodesc/file_test.go
+++ b/reflect/protodesc/file_test.go
@@ -819,7 +819,7 @@
]
}]}]
`),
- wantErr: `proto: message "M.M" using proto3 semantics has conflict: "baz" with "_b_a_z_"`,
+ wantErr: `message "M.M" using proto3 semantics has conflict: "baz" with "_b_a_z_"`,
}, {
label: "proto3 message fields",
inDesc: mustParseFile(`
