164 Commits

Author SHA1 Message Date
Damien Neil
d025c95110 proto, internal/protobuild: add test proto template builder
The proto package tests often test several variations of messages with a
similar shape. For example, most tests are performed with a proto2
message with a regular field, a proto2 message with an extension field,
and a proto3 message.

Add a protobuild package which can initialize all these variations from
a single template. For example, these three messages:

	&testpb.TestAllTypes{OptionalInt32: proto.Int32(1)}

	&test3pb.TestAllTypes{OptionalInt32: 1}

	m := &testpb.TestAllExtensions{}
	proto.SetExtension(m, &testpb.E_OptionalInt32, 1)

can all be constructed from the template:

	protobuild.Message{"optional_int32": 1}

This reduces redundancy in tests and will make it more practical to
test alternative code generators.

Change-Id: I3245a4bf74ee1bce957bc772fed513d427720677
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/217457
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-02-03 19:14:55 +00:00
Damien Neil
4d918167a9 internal/impl: catch varint overflow in validator
Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20477

Change-Id: I6afe82e3818f8b4e9cf5eded2125317eae8be49d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/217309
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-02-03 18:21:31 +00:00
Joe Tsai
74b1460c5b encoding: add Format helper function and method
The Format function and MarshalOptions.Format method are helper
functions for directly obtaining the formatted string for a message
without having to deal with errors or convert a []byte to string.
It is only intended for human consumption (e.g., debugging or logging).

We also add a MarshalOptions.Multiline option to specify that the output
should use some default indentation in a multiline output.

This assists in the v1 to v2 migration where:
	protoV1.CompactTextString(m) => prototext.MarshalOptions{}.Format(m)
	protoV1.MarshalTextString(m) => prototext.Format(m)

At Google, there are approximately 10x more usages of MarshalTextString than
CompactTextString, so it makes sense that the top-level Format function
does multiline expansion by default.

Fixes #850

Change-Id: I149c9e190a6d99b985d3884df675499a3313e9b3
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213460
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Herbie Ong <herbie@google.com>
2020-01-30 07:50:58 +00:00
Damien Neil
1887ff702c internal/impl: better fast-path init checks for extensions
Unknown extensions are initialized.

Valid extensions with no isInit func are initialized.

Change-Id: I2975c7ef85d2b777eca467d3b1861d20de8e24fc
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216960
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-30 05:43:23 +00:00
Damien Neil
6f2977906d internal/impl: fix validator bytes field length decoding
Missing a bounds check on the first byte.

Change-Id: I089fa8dcc1a14d11faca1acba758b6b811b16ac4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216957
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-30 00:26:49 +00:00
Damien Neil
c70f5d59d1 internal/impl: avoid redundant lazy extension inits
After taking the lock on a lazy extension's state, check to see if it
was initialized while we were waiting for the lock.

Change-Id: I1cbd52e9d655eec6c9142c97689ae36f219a28f2
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216898
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-01-29 23:04:37 +00:00
Damien Neil
ca6f40c880 proto: make use of fast-path initialization checks
Lost the check for the fast-path's Initialized output somewhere. Put it
back in.

name                        old time/op  new time/op  delta
Required/Wire/Unmarshal-12  37.8ns ± 1%  30.5ns ± 1%  -19.36%  (p=0.000 n=8+8)

Change-Id: Ica733366d00efba41023339a2bbd68167ab0df53
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216897
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-01-29 23:04:27 +00:00
Damien Neil
0ae1c9789a internal/impl: lazy extension decoding
Historically, extensions have been placed in the unknown fields section
of the unmarshaled message and decoded lazily on demand. The current
unmarshal implementation decodes extensions eagerly at unmarshal time,
permitting errors to be immediately reported and correctly detecting
unset required fields in extension values.

Add support for validated lazy extension decoding, where the extension
value is fully validated at initial unmarshal time but the fully
unmarshaled message is only created lazily.

Make this behavior conditional on the protolegacy flag for now.

Change-Id: I9d742496a4bd4dafea83fca8619cd6e8d7e65bc3
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216764
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-29 21:35:31 +00:00
Damien Neil
a522d5fa0c internal/impl: fix tag decoding when field num doesn't fit in int32
Discoverd by OSS-Fuzz.

Change-Id: Ie2feefacee4ae632802fa920ac9694b525690eb2
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216619
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-29 20:57:21 +00:00
Damien Neil
524c60670a runtime/protoiface: use more efficient options representation
Change the representation of option flags in protoiface from bools to a
bitfield. This brings the representation of options in protoiface in
sync with that in internal/impl.

This change has several benefits:

1. We will probably find that we need to add more option flags over time.
Converting to the more efficient representation of these flags as high
in the call stack as possible minimizes the performance implication of
the struct growing.

2. On a similar note, this avoids the need to convert from the compact
representation to the larger one when passing from internal/impl to
proto, since the {Marshal,Unmarshal}State methods take the compact form.

3. This removes unused options from protoiface. Instead of documenting
that AllowPartial is always set, we can just not include an AllowPartial
flag in the protoiface options.

4. Conversely, this provides a way to add option flags to protoiface
that we don't want to expose in the proto package.

name                             old time/op    new time/op    delta
EmptyMessage/Wire/Marshal-12       11.1ns ± 7%    10.1ns ± 1%   -9.35%  (p=0.000 n=8+8)
EmptyMessage/Wire/Unmarshal-12     7.07ns ± 0%    6.74ns ± 1%   -4.58%  (p=0.000 n=8+8)
EmptyMessage/Wire/Validate-12      4.30ns ± 1%    3.80ns ± 8%  -11.45%  (p=0.000 n=7+8)
RepeatedInt32/Wire/Marshal-12      1.17µs ± 1%    1.21µs ± 7%   +4.09%  (p=0.000 n=8+8)
RepeatedInt32/Wire/Unmarshal-12     938ns ± 0%     942ns ± 3%     ~     (p=0.178 n=7+8)
RepeatedInt32/Wire/Validate-12      521ns ± 4%     543ns ± 7%     ~     (p=0.157 n=7+8)
Required/Wire/Marshal-12           97.2ns ± 1%    95.3ns ± 1%   -1.98%  (p=0.001 n=7+7)
Required/Wire/Unmarshal-12         41.0ns ± 9%    38.6ns ± 3%   -5.73%  (p=0.048 n=8+8)
Required/Wire/Validate-12          25.4ns ±11%    21.4ns ± 3%  -15.62%  (p=0.000 n=8+7)

Change-Id: I3ac1b00ab36cfdf61316ec087a5dd20d9248e4f6
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216760
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-28 23:33:31 +00:00
Damien Neil
212b05b808 internal/testprotos: make TestAllExtensions recursive
Tweak the test message to allow creating messages with extensions that
contain extensions that contain extensions, etc.

Change-Id: I41844ae699c88ab96bf0d30db3a3fbaf09616161
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216761
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-28 23:28:36 +00:00
Damien Neil
a60e709ac8 proto: fix DiscardUnknown
UnmarshalOptions.DiscardUnknown was simply not working. Oops. Fix it.
Add a test.

Change-Id: I76888eae1221d99a007f0e9cdb711d292e6856b1
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216762
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-28 23:27:58 +00:00
Damien Neil
0bf97b7e36 internal/impl: messageset validation and isinit fixes
Recognize messagesets in the validator. Currently, this just gives
up and reports an unknown validity rather than trying to descend
into the messageset.

Plumb fast-path initialization checks through messageset decoding.

Change-Id: Ice55f28e8555764e4ce2720251830e8cf475c133
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216245
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-24 20:27:57 +00:00
Damien Neil
c600d6c086 all: do best-effort initialization check on fast path unmarshal
Add a fast check for required fields to the fast path unmarshal.
This is best-effort and will fail to detect some initialized
messages: Messages with more than 64 required fields, messages
split across multiple tags, possibly other cases.

In the cases where it works (which is most of them in practice),
this permits us to skip the IsInitialized check.

Change-Id: I6b70953a333033a5e64fb7ca37a59786cb0f75a0
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215878
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-22 20:57:14 +00:00
Damien Neil
d30e561d9e proto: add MarshalState, UnmarshalState
Add functions to the proto package which plumb through the fast-path state.

As a sample use case: A followup CL adds an Initialized field to
protoiface.UnmarshalOutput, permitting the unmarshaller to report back
when it can confirm that a message is fully initialized. We want to
preserve that information when an unmarshal operation threads through
the proto package (such as when unmarshaling extensions).

To allow these functions to be added as methods of MarshalOptions and
UnmarshalOptions rather than top-level functions, separate the options
from the input structs.

Also update options passed to fast-path methods to set AllowPartial and
Merge to reflect the expected behavior of those methods. (Always allow
partial, never merge.)

Change-Id: I482477b0c9340793be533e75a86d0bb88708716a
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215877
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-22 20:52:17 +00:00
Damien Neil
61781dd92f all: abstract fast-path marshal and unmarshal inputs and outputs
We may want to make changes to the inputs and outputs of the fast-path
functions in the future. For example, we likely want to add the ability
for the fast-path unmarshal to report back whether the unmarshaled
message is known to be initialized.

Change the signatures of these functions to take in and return struct
types which can be extended with whatever fields we want in the future.

Change-Id: Idead360785df730283a4630ea405265b72482e62
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215719
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-22 00:22:46 +00:00
Damien Neil
f12fb45fd6 all: add ProtoMethods method to protoreflect.Message
Promote the fast-path magic ProtoMethods method to first-class citizen
of the protoreflect.Message interface.

To avoid polluting the protoreflect package with the various types
required by this method, make the necessary protoiface types unnamed and
duplicate them in protoreflect.

Updates golang/protobuf#1022.

Change-Id: I9595bae40b3bc7536d727fb6f99b3bce8f73da87
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215718
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-21 21:05:54 +00:00
Damien Neil
6635e7d00a internal/impl: recognized required bytes fields in validation
Add a missed case in validation so we correctly validate bytes fields.
Fixes a case where we would report required bytes fields as potentially
missing.

Change-Id: I3dc4196d6995942d32a795a64214b3679d60ab6c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215000
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-15 23:51:51 +00:00
Damien Neil
2ae60936c2 internal/impl: fix unmarshal of group containing their own field number
The fast-path unmarshal was getting confused when parsing a group
containing a field with a number the same as the group's own field
number. Separate the handling of EndGroup tags.

Change-Id: I637702b42c94a26102e693ee29a55e80b37d7f28
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/214737
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-14 19:43:24 +00:00
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
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
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
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
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
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
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
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
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
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
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
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
Joe Tsai
8e9d5f6e8a internal/protolegacy: add stub v1 proto package for testing purposes
The protolegacy package is a minimal version of the v1 proto package.
This allows us to use this stub version as the dependency for
internal/testprotos/legacy packages and avoid a dependency
on the real v1 proto package.

The implementation of most v1 functionality will panic if called.
This way, we know if we the v2 code depends on one of those
unimplemented functions.

Updates golang/protobuf#962

Change-Id: I20b4091706fd456e4b01ae0931cce30a872639b0
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/199297
Reviewed-by: Damien Neil <dneil@google.com>
2019-10-05 23:12:32 +00:00
Damien Neil
6e40b32926 internal/impl: weak field bugfixes
Fix a reversed error check in impl.Export{}.WeakNil.

Check to see if we have a type for the weak field on marshal/size.

Treat a typed nil valued in XXX_Weak as not indicating presence for
the field.

Change-Id: Id667ac7eb4f53236be9e181017082bd8cd21d115
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/198717
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-10-04 17:07:45 +00:00
Damien Neil
47d5893acf internal/impl: support non-struct-pointer legacy message types
Support, to some limited degree, types which implement protoV1.Message
but which are not struct pointers. Our ability to work with these types
is largely limited to calling Marshal or Unmarshal methods, when
present.

Change-Id: Ie1b851d9e753e2b2cb189b17ffeefebe2d8b3a8f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/198237
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-10-04 16:13:03 +00:00
Damien Neil
1e5516a4c2 proto: improve slice growth in MarshalAppend
When allocating more space for the destination message in MarshalAppend,
use the same slice growth algorithm as the Go runtime's append rather
than allocating precisely the desired space.

Change-Id: If6033f6f7abdca473bc5188c4d3938ce57d3bdd2
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/197758
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-09-28 01:00:09 +00:00
Damien Neil
37ef691e6b internal/impl: call Marshal/Unmarshal methods on legacy types
Call the Marshal or Unmarshal method on legacy messages implementing
protoV1.Marshaler or protoV2.Unmarshaler.

We do this in the impl package by creating an appropriate function in
the protoiface.Methods struct for legacy messages.

In proto.MarshalAppend, return the bytes provided by the fast-path
marshal function even when the returned error is non-nil.

Fixes golang/protobuf#955

Change-Id: I36924af9ff959a946c43f2295ef3202216e81b32
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/197357
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-09-26 20:54:39 +00:00
Joe Tsai
f2c4ddc7a1 proto/equal: equate nil
Modify Equal to treat nil messages as equal iff both are nil.
Of special note, a typed nil pointer to T is equal to a new(T)
since they are indistinguishable from a protobuf reflection.

Change-Id: Ibf90b43a982e7376e07b4159be198f06230ec194
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/196618
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-23 16:43:47 +00:00
Joe Tsai
641611d984 proto: fix self-merging
While odd, it is possible to merge a message into itself.
In such a situation, the material impact is that repeated
and unknown fields are duplicated. The previous logic would
inifinite loop since the list iteration logic uses the current
length, but since the current length is ever growing, this loop
will never terminate. Instead, record the list length once
and iterate exactly that many times.

Change-Id: Ief98afa1b20bd950a9c2422d4462b170dbe6fa11
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/196857
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-23 16:14:39 +00:00
Joe Tsai
c908144c88 proto: fix race in Merge
Some existing targets (whether correctly or not) rely on it Merge
being safe to call concurrently so long as the set of fields being
merged are disjoint.

Change-Id: I4db9e64efccc7a2d44a5f9b52261b611cce461b0
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/196737
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-20 19:55:42 +00:00
Joe Tsai
6e095998ae proto, internal/impl: implement support for weak fields
Change-Id: I0a3ff79542a3316295fd6c58e1447e597be97ab9
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189923
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-19 22:41:12 +00:00
Joe Tsai
84177c9bf3 all: use typed variant of protoreflect.ValueOf
Change-Id: I7479632b57e7c8efade12a2eb2b855e9c321adb1
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/196037
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-17 21:33:16 +00:00