Commit Graph

654 Commits

Author SHA1 Message Date
Joe Tsai
55f18259ef internal/testprotos/legacy: rename and regenerate
Avoid dots and dashes in the directory to avoid issues on
build systems that cannot support them well.

Change-Id: I7ea5e6ce0b16c7158c7e53bcf5c3c1a334fe4718
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/214342
Reviewed-by: Damien Neil <dneil@google.com>
2020-01-12 08:13:18 +00:00
Joe Tsai
4a7fc8200d cmd/protoc-gen-go: refactor package
Refactor the internal logic of protoc-gen-go to better plumb local
settings and parameters down the call tree.

Change-Id: I09fec188d7359f2b66be584aa8f10e682a7b6796
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/214357
Reviewed-by: Patrik Nyblom <pnyb@google.com>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-01-11 03:09:40 +00:00
Damien Neil
7abc2def69 internal/impl: omit isInit func when not needed
When generating the fast-path functions for a message, group, repeated
message, or repeated group field, check to see if the field message type
requires initialization checks. If not, leave the isInit func unset.

This permits the fast-path isInitialized to skip over these fields
entirely.

Change-Id: Icb5c380077d2216c4215bb0ebc16408e905aaece
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/214179
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-09 23:00:50 +00:00
Damien Neil
025a0d2da1 benchmarks: rename to internal/benchmarks
I've come to agree with Joe about the proper location for this.

Change-Id: Ia5adbd1cd18f8cf40f7c3cc6bf8c7833dac37f20
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/214041
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-09 18:21:53 +00:00
Damien Neil
ec00e32a8d all: remove APIv1 dependency
Remove support for running benchmarks with APIv1.

The comparisons have served their purpose, and this removes the last
dependency on the github.com/golang/protobuf module.

Fixes golang/protobuf#962.

Change-Id: I55758e19451fcd16ab1a5d66244eb8214ceb9fa7
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/214040
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-09 18:06:30 +00:00
Damien Neil
54a0a0476a internal/impl: check for required fields in missing map value
If a map value is a message with required fields, the validator should
note that it is uninitialized if a map item contains no value. In this
case, the value is an empty message which obviously does not have the
required field set.

Change-Id: I7698e60765e3c95478f293e121bba3ad7fc88e27
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213900
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-09 05:38:08 +00:00
Herbie Ong
b02b6d1da5 internal/encoding/json: fix performance cliff when decoding large integers that will go out of range.
For large positive integers, add check for number of decimal digits
before converting number to plain integer w/o exponent.

If exponent value is large, previous implementation may end up
constructing a large string with lots of zeroes that is not useful as it
will fail later on when called with strconv.Parse{Uint,Int} anyways.

Fixes golang/protobuf#1002.

Change-Id: I65bfad304401e076743853d7501786b7231b083b
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213717
Reviewed-by: Damien Neil <dneil@google.com>
2020-01-08 18:44:43 +00:00
Damien Neil
b0c26f1868 internal/impl: add message validator
This adds a experimental function to the internal/impl package which
validates a wire-format message against a message type. The validator
reports whether the message can be successfully unmarshaled, and whether
the result is initialized (all required fields are set). In some cases,
the validator returns ambiguous results when full validation would be
expensive.

The validator is unused outside of tests. In the future, it may be used
to permit lazy unmarshaling of some data. It is being added now for
testing; in particular, the wire fuzzer now checks the validator output
for consistency with the unmarshaler.

The validator adds a small amount of unused per-MessageType state. If
this becomes a concern, we could conditionalize it with a build tag.

Change-Id: I4216ef81d6a9ed975302eed189b02d08608858b4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/212302
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-01-07 21:36:47 +00:00
Damien Neil
2ad3f248e2 proto: fix equality on nil values of different types
Equal((*M1)(nil), (*M2)(nil)) should be false.

Change-Id: I7def8016fcf1e78d9e69f79c23ab44fc3d211bb0
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213479
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-06 23:36:47 +00:00
Damien Neil
2244bc5833 internal/testprotos/fuzz: add go_package option
Change-Id: Ic9ad6a5a96a3178cc6074e7e26b5ac864552d05f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/212863
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-06 22:47:51 +00:00
Damien Neil
b0d217f664 proto, internal/impl: don't create fast path Size for legacy Marshalers
Implementations of the legacy Marshaler type have no way to efficiently
compute the size of the message. Rather than generating an inefficient
fast-path Size method which marshals the message and examines the
length of the result, don't generate a fast-path at all.

Drop the requirement that a fast-path MarshalAppend requires a
corresponding Size.

Avoids O(N^2) behavior when marshaling a legacy Marshaler that
recursively calls proto.Marshal.

Change-Id: I4793cf32275d08f29c8e1a1a44a193d9a5724058
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213443
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-06 22:47:37 +00:00
Joe Tsai
b7695fab0d proto: add Clone function and MergeOptions.Clone method
We resisted adding Clone for a while since:
* It is a function that is perfectly suited for generics.
However, generics probably still won't be available in Go for some time
and it is impractical to block addition of this function when it is very
widely used and will be necessary for the v1 to v2 migration.
* In the past, there was no protoreflect.Message.IsValid, so there was
no proper API to detect invalid top-level messages and return them as such.

Since Clone relies on certain properties about proper round-tripping
of ProtoMessage.ProtoReflect <-> Message.Interface, we add a test
in testing/prototest to check for this.

Change-Id: Ic492b68f27b8b88322a6a3fa3a5e492228db79d9
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213297
Reviewed-by: Damien Neil <dneil@google.com>
2020-01-06 21:07:28 +00:00
Joe Tsai
ce496b5d4d proto: add MergeOptions.Shallow option
A shallow copy of a message is a common operation with over 10k
usages inside Google. However, the semantics of a shallow copy
on the struct is ill-defined and not officially supported by
the generated protobuf API.

To reduce improper usages, add an official implementation of
shallow merging that does something similar where messages, lists,
and maps are shallow copied into the destination if it does not
already have one populated.

In the common case where the destination is empty, this equivalent to:
	src.Range(func(fd protoreflect.FieldDescriptor, v protoreflect.Value) bool {
		dst.Set(fd, v)
	})
	if len(src.GetUnknown()) > 0 {
		dst.SetUnknown(src.GetUnknown())
	}
which is as simple of a shallow copy definition as you can get.

A future CL will add a fast-path implementation of both
deep and shallow merges.

Change-Id: Ic4a5503dd1b11b505738f5e503f97d55997e9418
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213131
Reviewed-by: Damien Neil <dneil@google.com>
2020-01-06 20:26:20 +00:00
Joe Tsai
96a44732e0 proto: distinguish between invalid and empty messages in Equal
The v1 proto.Equal function treats (*Message)(nil) and new(Message)
as being different, while v2 proto.Equal treated them as equal since
a typed nil pointer is functionally an empty message since the
protobuf data model has no concept of presence as a first-class
property of messages.

Unfortunately, a significant amount of code depends on this distinction
that it would be difficult to migrate users from v1 to v2 unless we
preserved similar semantics in the v2 proto.Equal.

Also, double down on these semantics for protocmp.Transform.

Fixes #965

Change-Id: I21e78ba6251401a0ac0ccf495188093973cd7f3f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213238
Reviewed-by: Damien Neil <dneil@google.com>
2020-01-06 19:37:38 +00:00
Joe Tsai
14ac241a96 internal/impl: return nil for nil enum or message
Ensure that EnumOf, EnumDescriptorOf, EnumTypeOf, ProtoMessageV1Of,
ProtoMessageV2Of, MessageOf, MessageDescriptorOf, and MessageTypeOf
all return nil if passed a nil interface.

This parallels the behavior of reflect.TypeOf or reflect.ValueOf,
which return nil or an invalid value rather than panicking.

Change-Id: I461f15542f16cb0922d627bca6fcad5fc27d87e2
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213239
Reviewed-by: Damien Neil <dneil@google.com>
2020-01-06 19:10:59 +00:00
Joe Tsai
6c26a04a51 internal/filedesc: use jsonName.Init method over JSONName constructor
The JSONName constructor returns a struct value which shallow copies
a sync.Once within it; this is a dubious pattern.
Instead, add a jsonName.Init method to initialize the value.

Change-Id: I190a7239b1b62a8041ee7e4e09c0fe37b64ff623
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213237
Reviewed-by: Damien Neil <dneil@google.com>
2020-01-06 18:42:14 +00:00
Damien Neil
1a08d54978 encoding/prototext: fix crash in map parsing
Fuzzer-detected crash when parsing map values which should be messages,
but are not.

Fixes golang/protobuf#1003

Change-Id: Ib34b13d1a6fef7209e7c17dc5d7f4bd8a1ebac87
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/212397
Reviewed-by: Herbie Ong <herbie@google.com>
2019-12-25 06:02:31 +00:00
Damien Neil
5ba0c29655 internal/encoding/json: fix crash in parsing
Fuzzer-detected crash when parsing: {""

Change-Id: I019c667f48e6a1237858b5abf7d34f43593fb3b6
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/212357
Reviewed-by: Herbie Ong <herbie@google.com>
2019-12-22 04:14:01 +00:00
Damien Neil
f2427c09d6 proto, internal/impl: reject invalid field numbers in map items
Change-Id: I44a44a36538f6f8b94078b43711d865edb6244f5
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/212257
Reviewed-by: Herbie Ong <herbie@google.com>
2019-12-21 00:16:12 +00:00
Damien Neil
2c0824b512 internal/impl: fix size for zero-length packed extensions
The size calculation for packed repeated extension fields was
considering a zero-length list as encoding to a zero-length
wire.BytesType field, rather than being omitted entirely.

Change-Id: I7d4424a21ca8afd4fa81391caede49cadb4e2505
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/212297
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-12-20 22:08:18 +00:00
Damien Neil
75f53c59e1 internal/fuzztest: factor out common fuzzer tests
All the fuzzers have the same test, which runs the fuzzer against every
entry in the corpus. Move the test logic into a separate package.

Change-Id: I3a7e2ca75d20a5ff6d51ed9e6151629e6667684b
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/212258
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-12-20 22:08:10 +00:00
Damien Neil
26451c0385 internal/fuzz: add fuzzers for prototext and protojson packages
Change-Id: Iee065070e6a983c303a3551a67fc32f0e94b649e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/212219
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-12-20 09:02:05 +00:00
Damien Neil
7f9c7d9fe4 internal/fuzz: refactor fuzzer
Add a new Fuzz message containing all the message types we want to make
available to fuzzers. Previously, testing (for example) required fields
would require modifying the fuzzer; now, it's just a matter of adding a
message with required fields as a field of the top-level Fuzz message.

Add internal/cmd/generate-corpus to codify where the fuzz seed corpus
comes from. This will simplify adding text and json fuzzers.

Rename internal/fuzz/wire to internal/fuzz/wirefuzz to minimize package
name ambiguity. Also, the addition of the Fuzz container message
invalidates the existing corpus, so using a new name seems like a good
idea.

Change-Id: I94f8f64ba93596c8e8cecb4d42bcc5b98c17d838
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/212218
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-12-20 09:01:58 +00:00
Damien Neil
7e690b5b4c internal/impl: fix map decode when value is before key
Fix a bug in handling the case where the encoding for a map item places
the value field (2) before the key field (1).

Change-Id: I2e6ad9af729a199e960e566ed7ef96bba3726990
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/211804
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-12-18 17:42:10 +00:00
Damien Neil
3e42b667d2 internal/impl: faster map fast path
Avoid using protobuf reflection on map values in the fast path. Range
operations in particular are expensive in protoreflect, because the
closure passed to Map.Range escapes.

Iterate maps using a reflect.MapIter when available.

When operating on maps of messages where we have a *MessageInfo for the
message type, directly jump to the fast-path *MessageInfo methods rather
than passing through the proto package.

Benchmarks deltas for a google.protobuf.Struct with JSON represention:
  {"parameters":{"a":{"b":{"c":{"d":{"e":{"f":{"g":{}}}}}}}}}

Compared to previous revision:

  name                      old time/op  new time/op  delta
  NestedStruct/Size         7.22µs ± 2%  4.84µs ± 2%  -32.96%  (p=0.000 n=8+8)
  NestedStruct/Size-8       9.30µs ± 2%  5.89µs ± 2%  -36.60%  (p=0.000 n=8+8)
  NestedStruct/Marshal      77.6µs ±12%   9.8µs ± 4%  -87.33%  (p=0.000 n=8+8)
  NestedStruct/Marshal-8    91.6µs ± 2%  11.9µs ± 2%  -86.99%  (p=0.000 n=8+8)
  NestedStruct/Unmarshal    11.5µs ± 4%   8.7µs ± 2%  -24.76%  (p=0.000 n=8+8)
  NestedStruct/Unmarshal-8  15.4µs ± 4%  11.9µs ± 2%  -22.41%  (p=0.000 n=8+8)

Compared to github.com/golang/protobuf:

  name                      old time/op  new time/op  delta
  NestedStruct/Size         5.42µs ± 1%  4.84µs ± 2%  -10.61%  (p=0.000 n=8+8)
  NestedStruct/Size-8       6.34µs ± 2%  5.89µs ± 2%   -7.10%  (p=0.000 n=8+8)
  NestedStruct/Marshal      12.5µs ± 2%   9.8µs ± 4%  -21.41%  (p=0.000 n=7+8)
  NestedStruct/Marshal-8    14.1µs ± 3%  11.9µs ± 2%  -15.52%  (p=0.000 n=8+8)
  NestedStruct/Unmarshal    9.66µs ± 1%  8.65µs ± 2%  -10.40%  (p=0.000 n=7+8)
  NestedStruct/Unmarshal-8  11.7µs ± 3%  11.9µs ± 2%   +1.95%  (p=0.038 n=8+8)

Change-Id: I0effe6491f30d41f31904777f74eca3ac3694db3
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/211737
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-12-17 22:14:17 +00:00
Damien Neil
d0b074956d proto: rearrange test messages
Move the test inputs for the wire marshaler and unmarshaler out of
decode_test.go and into a new file. Consolidate some tests for invalid
messages (UTF-8 validation failures, field numbers out of range) into
a single list of invalid messages. Break out the no-enforce-utf8 test
into a separate file, since it is both complicated and conditional on
legacy support.

Change-Id: Ide80fa9d3aec2b6d42a57e6f9265358aa5e661a7
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/211557
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-12-16 21:49:56 +00:00
Damien Neil
b120a23bc8 internal/impl: fix reversed IsValid test
Change-Id: Iaf5291a6bf31ad3dd130fca06840cec66b896f59
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/211558
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-12-16 21:49:33 +00:00
Joe Tsai
d65001875f internal/impl: fix aberrant message detection logic
In very old messages predating the existence of the size cache or the
proto3 unknown fields, it is possible that the generated struct lacks both
XXX_ fields and ones tagged with "protobuf". This can happen with a message
that only contains oneofs. As such, check for the "protobuf_oneof" tag as well.

Change-Id: I1981cd7dde68aece1a013356b6bc91cc5529f951
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210747
Reviewed-by: Damien Neil <dneil@google.com>
2019-12-11 04:25:07 +00:00
Joe Tsai
8517608739 cmd/protoc-gen-go: remove json ignore tags
These are not necessary now that weak fields are unexported.

Change-Id: Ida18b984abedfdf52fd3d5f3cb2f4ca580659a5c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210745
Reviewed-by: Damien Neil <dneil@google.com>
2019-12-10 23:22:16 +00:00
Damien Neil
4151cae27a internal/impl: more checks for aberrant messages
When loading a *MessageInfo for a legacy message type, check to see if
the Go type contains at least one field which looks like a message
field. Specifically, look for at least one field with a `protobuf:` tag,
or an XXX_unrecognized field.

If a message has no recognizable fields, assume that it's something we
don't know how to interpret and treat it as an aberrant message.

Change-Id: If5c09087f1a0187271c98539d761395a2ee70a9e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210617
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-12-10 23:02:58 +00:00
Joe Tsai
62d204ccc3 internal/impl: fix legacy UnmarshalJSONEnum
The conditional was accidentally inverted.
This function provides dubious support for encoding/json.

Change-Id: Ib4131a229afa14d9aef1ad31fec51f4dac417a3b
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210638
Reviewed-by: Damien Neil <dneil@google.com>
2019-12-10 16:55:03 +00:00
Joe Tsai
4663ebc852 internal/genname: centralize the definitions for generated names
Both the generator and the runtime need to agree upon the names of
specialized Go struct fields. Centralize that information in an
internal genname package.

In the mean time, also change the XXX_weak field name to match
the name used internally at Google.

Change-Id: I026bf354418c363482e5902f21aa5e0cacae24b0
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/207080
Reviewed-by: Damien Neil <dneil@google.com>
2019-12-09 22:57:38 +00:00
Damien Neil
ce413af0b3 internal/impl: faster oneof marshaling
Change size, marshal, and isinit operations on oneofs to look up the
currently-set oneof type in a map rather than testing for each possible
oneof field in turn.

Significantly improves oneof encoding speed for oneofs with a
substantial number of fields:

  go test ./proto -bench=./oneof.*string.*test.TestAll -benchmem -count=8 -cpu=1

  name                                        old time/op    new time/op    delta
  Encode/oneof_(string)_(*test.TestAllTypes)     911ns ± 1%     397ns ± 3%  -56.45%  (p=0.000 n=8+7)
  Decode/oneof_(string)_(*test.TestAllTypes)     899ns ± 1%     922ns ± 1%   +2.49%  (p=0.001 n=7+7)

Fixes golang/protobuf#950

Change-Id: I9393a87975ce09011d885a8af4a63a639ea8452f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210281
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-12-09 22:00:58 +00:00
Damien Neil
79571e90e2 internal/impl: improve extension fast path performance
Stash fast-path information for extensions on the ExtensionInfo. In
the usual case where an ExtensionType's underlying implementation is
an *ExtensionInfo, fetching the fast-path information becomes a type
assertion rather than a mutex-guarded map access.

Maintain a global sync.Map for the case where an ExtensionType isn't an
*ExtensionInfo.

Substantially improves performance for fast-path operations on
extensions:

Encode/MessageSet_type_id_before_message_content-12      267ns ± 1%   185ns ± 1%  -30.44%  (p=0.001 n=7+7)
Encode/basic_scalar_types_(*test.TestAllExtensions)-12  1.94µs ± 1%  0.40µs ± 1%  -79.32%  (p=0.000 n=8+7)

Change-Id: If048b521deb3665a090ea3d0a178c61691d4201e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210540
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-12-09 18:59:42 +00:00
Damien Neil
fe15dd4cdd all: don't allow invalid field numbers when legacy support is on
The deprecated messageset format permits extension fields with numbers
greater than the usual maximum (1<<29-1). To support this, the
internal/encoding/wire package has disabled field number validation when
legacy support is enabled.

We shouldn't skip validating all field numbers for validity just because
we support larger ones in messagesets.

This change drops range validation from the wire package (other than
checking that numbers fit in an int32) and adds it to the wire
unmarshalers instead. This gives us validation where we care
about it (when unmarshaling a wire-format message) and allows for
best-effort handling of out-of-range numbers everywhere else.

Fixes golang/protobuf#996

Change-Id: I4e11b8a8aa177dd60e89723570af074a317c2451
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210290
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-12-09 18:35:13 +00:00
Damien Neil
5366f825ad proto: consistently use non-nil, zero-length []bytes for empty bytes strings
The fast-path decoder decodes zero-length repeated bytes values as
non-nil, zero-length []bytes. Do the same in the reflection decoder.

This isn't really a correctness issue, since there's no ambiguity about what a
nil entry in a [][]byte means. Still a good idea for consistency, and
retains v1 behavior.

Change-Id: Icd2cb726d14ff1f2b9f142e65756777a359971f3
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210257
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-12-09 17:33:50 +00:00
Damien Neil
9f1165c3bf all: fix reflection behavior for empty lists and maps
The protoreflect documentation states that Get on an empty repeated or
map field returns an invalid (empty, read-only) List or Map. We weren't
doing this; fix it.

The documentation also states that an invalid List or Map may not be
assigned via Set. We were permitting Set with an invalid map; fix this.

Add tests for this behavior in prototest.

Change-Id: I4678af532e192210af0bde7c96a1439a4cd26efa
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/209019
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-11-26 22:58:51 +00:00
Damien Neil
82886da2b9 reflect/protoreflect: add {Message,List,Map}.IsValid
Various protoreflect methods can return an "empty, read-only" message,
list, or map value. Provide a method to test if a value is one of these.

Fixes golang/protobuf#966

Change-Id: I793d8426d6e2201755983c06f024412a7e09bc4c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/209018
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-11-26 22:56:18 +00:00
Joe Tsai
1c31032e00 testing/protocmp: add Ignore options and Any support
This CL adds the following helper options:
	func IgnoreEnums(...protoreflect.Enum) cmp.Option
	func IgnoreMessages(...proto.Message) cmp.Option
	func IgnoreFields(proto.Message, ...protoreflect.Name) cmp.Option
	func IgnoreOneofs(proto.Message, ...protoreflect.Name) cmp.Option
	func IgnoreDescriptors(...protoreflect.Descriptor) cmp.Option
	func IgnoreDefaultScalars() cmp.Option
	func IgnoreEmptyMessages() cmp.Option
	func IgnoreUnknown() cmp.Option

It also augments transformMessage to unmarshal and expand Any messages
with the value of the underlying message. At this moment in time
we do not provide an API to provide a custom type resolver.

Change-Id: I51e1d9ff0d56d71161e510f366a7dcc32236d760
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/204577
Reviewed-by: Damien Neil <dneil@google.com>
2019-11-14 09:49:19 +00:00
Joe Tsai
fab1c8d9a3 testing/protocmp: switch Enum.Number to be a method
Accessing Number as a method, rather than a field paves the
way to have Enum potentially implement protoreflect.Enum
in the future.

Change-Id: Iebe9c0ec12e067decf2121d12fe2fb1549477b32
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/207077
Reviewed-by: Damien Neil <dneil@google.com>
2019-11-13 18:38:37 +00:00
Joe Tsai
613285cf7a internal/impl: refactor makeStructInfo
Simplify the implementation of makeStructInfo by checking the
names of the internal fields in a switch statement.
Also, add "weakFields" as the unexported name for weak fields
in preparation for actually unexporting it.

Change-Id: Ide970a39e9caa5a24bc288ba3e3a0d223a6bfcb6
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/207057
Reviewed-by: Damien Neil <dneil@google.com>
2019-11-13 17:53:39 +00:00
Damien Neil
01c0e8d680 proto, internal/impl: make wire output more consistent with v1
The v1 wire marshaler sorts fields as follows:
  - All extensions, sorted by field number.
  - All non-oneof fields, sorted by field number.
  - All oneof fields, in indeterminate order.

We already make some steps toward supporting this ordering: The
fast path encoder places extensions in sorted order at the start
of the message.

This commit moves oneof fields to the end of the message, makes the
reflection-based encoder use this ordering when deterministic marshaling
is enabled, and adds a test to catch unintentional changes to the
ordering.

Users SHOULD NOT depend on stability of the marshal output. It is
subject to change over time. Without deterministic marshaling enabled,
it is subject to change over calls to Marshal.

Change-Id: I6cfd89090d790a3bb50785f32b94d2781d7d08db
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/206800
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-11-12 20:59:03 +00:00
Damien Neil
1605775be0 internal/impl: handle some dynamic legacy messages
When creating a MessageDescriptor for a legacy message with a
Descriptor method, we call that method on the type's zero value to
get the message's DescriptorProto. Some existing dynamic message
types have a Descriptor method which panics in this case.

Catch the panic and continue as if the Descriptor method wasn't present.

Change-Id: I98d4625d6917cc1ec25737e5670a443f5d02a404
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/206637
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-11-12 17:55:36 +00:00
Damien Neil
ce3384cd34 proto, internal/impl: store unknown MessageSet items in non-mset format
In the v1 implementation, unknown MessageSet items are stored in a
message's unknown fields section in non-MessageSet format. For example,
consider a MessageSet containing an item with type_id T and value V.
If the type_id is not resolvable, the item will be placed in the unknown
fields as a bytes-valued field with number T and contents V. This
conversion is then reversed when marshaling a MessageSet containing
unknown fields.

Preserve this behavior in v2.

One consequence of this change is that actual unknown fields in a
MessageSet (any field other than 1) are now discarded. This matches
the previous behavior.

Change-Id: I3d913613f84e0ae82481078dbc91cb25628651cc
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/205697
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-11-11 19:40:27 +00:00
Damien Neil
c975a7097d reflect/protoregistry: remove deprecated APIs
Remove previously deprecated types, functions, and methods.

Update github.com/golang/protobuf module version to one which does not
depend on any deprecated APIs.

Fixexs golang/protobuf#963

Change-Id: Ida451ef5ef3f34830808f737cc0d1c98f32ce76a
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/206017
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-11-08 01:03:52 +00:00
Damien Neil
96208278f9 cmd/protoc-gen-go-grpc: add Unimplemented...Server type
Also add deprecation comments on methods.

Forward port:
  https://github.com/golang/protobuf/pull/785
  https://github.com/golang/protobuf/pull/952

Fixes golang/protobuf#816

Change-Id: Id4d9f08b39fe16eaf57fb7a92fb8ada222b5cbf4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/205246
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-11-08 00:42:50 +00:00
Damien Neil
e9187326c3 internal/filedesc: move message options to L1 initialization
Avoid a deadlock when registering a legacy ExtensionType, caused by
initialization of the "internal/impl".ExtensionInfo calling IsMessageSet
on the MessageDescriptor of the type being extended.

We can avoid this deadlock either by initializing the ExtensionType
outside of the GlobalTypes mutex, or by moving IsMessageSet to L1
initialization of the MessageDescriptor so that it doesn't trigger lazy
init.

CL 204804 takes the former approach; this CL takes the latter.

Change-Id: Idfc1ed36a23a139839290ea32492142a17f68cf5
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/205957
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-11-08 00:29:25 +00:00
Damien Neil
c7f2beeef0 internal/impl: assume legacy Marshal method supports deterministic
The v1 implementation calls Marshal methods when deterministic
serialization is requested, even though it has no way to verify that the
method supports determinism. Preserve this behavior.

Change-Id: I383f2ec4bd4d5b996d96d604e92dfa43cb6f1bdc
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/205719
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-11-07 00:44:29 +00:00
Damien Neil
a2684f4b8a internal/impl: make resolverOnly explicitly implement file resolver
The resolverOnly type embeds a *protoregistry.Files and overrides one of
of its methods. When that method was renamed (Register -> RegisterFile),
the resolverOnly type continued to compile but no longer overrode the
desired method.

Update the method name, and change this type to explicitly forward
methods to the underlying *protoregistry.Files to catch errors of this
nature in the future.

Change-Id: I09a529034b6e2f310977bc635288fa15ce14cc7e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/205242
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-11-06 18:17:25 +00:00
Damien Neil
a0a54b8005 reflect/protoreflect: remove nullability from repeated extension fields
Remove repeated extension fields from the set of nullable fields,
so that Has reports false and Range does not visit a a zero-length
repeated extension field.

This corrects a fuzzer-detected case where unmarshaling and remarshaling
a wire-format message could result in a semantic change. For a repeated
extension field in non-packed encoding, unmarshaling a packed
representation of the field would result in a message which Has the
extension. Remarshaling it would discard the the field.

Fixes golang.org/protobuf#975

Change-Id: Ie836559c93d218db5b5201742a3b8ebbaacf54ed
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/204897
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-11-05 18:31:30 +00:00