272 Commits

Author SHA1 Message Date
Joe Tsai
d347099942 internal/cmd/generate-types: use ConsumeBytes instead of ConsumeString
The protowire.{ConsumeBytes,ConsumeString} funcs are identical except
that the latter allocates a string by implicitly converting the []byte.
Avoid using ConsumeString since we can do the conversion ourselves
at a latter point and sometimes avoid the allocation.

Change-Id: Idf31edc013b72ee5ee8461a68d10e57ad461d95c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/263628
Trust: Joe Tsai <joetsai@digital-static.net>
Reviewed-by: Damien Neil <dneil@google.com>
2020-10-20 20:17:50 +00:00
Joe Tsai
41134b7e06 internal/cmd/generate-types: name the options variable
This trivially adds a name for marshalOptions and unmarshalOptions
instead of always having it be underscore.
This assists the use of patches that rely on this variable.

Change-Id: I4c23901c93f0362aff412339912522f92749d659
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/263627
Trust: Joe Tsai <joetsai@digital-static.net>
Reviewed-by: Damien Neil <dneil@google.com>
2020-10-20 19:16:57 +00:00
Joe Tsai
44626bc40b internal/cmd/generate-types: move variable declaration
Move the declaration of the slice variable
as close to its actual use as possible.

Change-Id: Ibe26460b5ca81edbed1073b32faf1c5f5d2c922a
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/263719
Trust: Joe Tsai <joetsai@digital-static.net>
Trust: Joe Tsai <joetsai@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
2020-10-20 18:59:00 +00:00
Joe Tsai
5d63473da8 internal/impl: add runtime support for *[]byte unknown representation
This CL adds runtime support for unknown fields to be represented
as *[]byte in addition to the current representation as []byte.
This CL does not change generated code to use *[]byte.

Comparison between using *[]byte and []byte:

• Every message supports unknown fields, so use of []byte
  expands a message size by 24B (for 64-bit systems).
  In contrast, *[]byte only expands a message by 8B.
  This has significant memory implications for small messages.

• If unknown fields are encountered, *[]byte has extra overhead
  allocating the 24B slice header. However, it is assumed
  that messages rarely see any unknown fields at runtime,
  or generally do so for a temporary period of time.

Change-Id: I81935e4ea7394166e61ff4579f76f59fa792dfc9
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/244937
Reviewed-by: Damien Neil <dneil@google.com>
2020-07-28 16:36:39 +00:00
Joe Tsai
4194195e12 internal/impl: rename legacyProtoMethods as aberrantProtoMethods
This variable is only used by the aberrantMessage type,
rename it to properly reflect what it is used for.

Change-Id: I081e8954eb1b282f01f6d2d764bffaf31c6677e9
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/245077
Reviewed-by: Damien Neil <dneil@google.com>
2020-07-27 20:52:20 +00:00
Damien Neil
a0f95d5b14 internal/impl: make errInvalidUTF8 be a proto.Error
Fixes golang/protobuf#1174.

Change-Id: If95f9f79fff76dc0c44f7ee96e484f59fa287e58
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/244718
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-07-24 21:43:34 +00:00
Joe Tsai
1a290e9a0e reflect/protoreflect: add MessageFieldTypes
The MessageFieldTypes interface (if implemented by a MessageType)
provides Go type information about the fields if they are
an enum or message type.

Change-Id: I68b20f5726377f6b0f2c20a8b6e45f9802b43f67
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/236777
Reviewed-by: Damien Neil <dneil@google.com>
2020-07-23 21:24:58 +00:00
Damien Neil
b5523e32b3 all: return less-specific, but more informative wire unmarshal errors
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>
2020-07-23 20:09:00 +00:00
Joe Tsai
e14d6b3cdc reflect/protoreflect: add FieldDescriptor.TextName
Add a new TextName accessor that returns the field name that should
be used for the text format. It is usually just the field name, except:
1) it uses the inlined message name for groups,
2) uses the full name surrounded by brackets for extensions, and
3) strips the "message_set_extension" for well-formed extensions
to the proto1 MessageSet.

We make similar adjustments to the JSONName accessor so that it applies
similar semantics for extensions.

The two changes simplifies all logic that wants the humanly readable
name for a field.

Change-Id: I524b6e017fb955146db81819270fe197f8f97980
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/239838
Reviewed-by: Herbie Ong <herbie@google.com>
2020-07-08 23:23:57 +00:00
Joe Tsai
92679665d7 internal/order: add a package for ordered iteration over messages and maps
The order package replaces the mapsort and fieldsort packages.
It presents a common API for ordered iteration over message fields
and map fields.

It has a number of pre-defined orderings.

Change-Id: Ie6cd423da30b4757864c352cb04454f21fe07ee2
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/239837
Reviewed-by: Herbie Ong <herbie@google.com>
2020-07-01 18:57:38 +00:00
Joe Tsai
1726b83dc4 internal/impl: introduce instability to protoreflect.Message.Range order
Change-Id: I6f53aaf4071b4cbf596f3438a83cf3e5c93f1712
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/240017
Reviewed-by: Herbie Ong <herbie@google.com>
2020-07-01 17:34:30 +00:00
Joe Tsai
f49fd502d3 all: implement first-class WKT support
This CL introduces generation of specialized APIs directly into the
generated packages for certain well-known types. This follows the pattern
set forth by the other language implementations that have specialized
generated support for certain well-known types.

Overview of new API:
  package anypb
    func MarshalFrom(*Any, proto.Message, proto.MarshalOptions) error
    func UnmarshalTo(*Any, proto.Message, proto.UnmarshalOptions) error
    func UnmarshalNew(*Any, proto.UnmarshalOptions) (proto.Message, error)
    func (*Any) MessageIs(proto.Message) bool
    func (*Any) MessageName() protoreflect.FullName
    func (*Any) MarshalFrom(proto.Message) error
    func (*Any) UnmarshalTo(proto.Message) error
    func (*Any) UnmarshalNew() (proto.Message, error)

  package timestamppb
    func Now() *Timestamp
    func New(time.Time) *Timestamp
    func (*Timestamp) AsTime() time.Time
    func (*Timestamp) IsValid() bool
    func (*Timestamp) CheckValid() error

  package durationpb
    func New(time.Duration) *Duration
    func (*Duration) AsDuration() time.Duration
    func (*Duration) IsValid() bool
    func (*Duration) CheckValid() error

  package structpb
    func NewStruct(map[string]interface{}) (*Struct, error)
    func (*Struct) AsMap() map[string]interface{}
    func (*Struct) MarshalJSON() ([]byte, error)
    func (*Struct) UnmarshalJSON(b []byte) error

    func NewList([]interface{}) (*ListValue, error)
    func (*ListValue) AsSlice() []interface{}
    func (*ListValue) MarshalJSON() ([]byte, error)
    func (*ListValue) UnmarshalJSON(b []byte) error

    func NewValue(interface{}) (*Value, error)
    func NewNullValue() *Value
    func NewBoolValue(bool) *Value
    func NewNumberValue(float64) *Value
    func NewStringValue(string) *Value
    func NewStructValue(*Struct) *Value
    func NewListValue(*ListValue) *Value
    func (*Value) AsInterface() interface{}
    func (*Value) MarshalJSON() ([]byte, error)
    func (*Value) UnmarshalJSON(b []byte) error

  package fieldmaskpb
    func New(proto.Message, ...string) (*FieldMask, error)
    func Union(*FieldMask, *FieldMask, ...*FieldMask) *FieldMask
    func Intersect(*FieldMask, *FieldMask, ...*FieldMask) *FieldMask
    func (*FieldMask) IsValid(proto.Message) bool
    func (*FieldMask) Append(proto.Message, ...string) error
    func (*FieldMask) Normalize()

  package wrapperspb
    func Bool(bool) *BoolValue
    func Int32(int32) *Int32Value
    func Int64(int64) *Int64Value
    func UInt32(uint32) *UInt32Value
    func UInt64(uint64) *UInt64Value
    func Float(float32) *FloatValue
    func Double(float64) *DoubleValue
    func String(string) *StringValue
    func Bytes([]byte) *BytesValue

This functionality expands upon and supersedes the
older github.com/golang/protobuf/ptypes package,
which provided helpers for Any, Timestamp, and Duration.

Comparison with older ptypes package:

* ptypes.AnyMessageName is replaced by anypb.Any.MessageName.
  The former returned an error for malformed type URLs,
  while the latter simply returns an empty string.

* ptypes.Is is replaced by anypb.Any.MessageIs.

* ptypes.Empty has no direct replacement as it is equivalent to:
	mt, err := protoregistry.GlobalTypes.FindMessageByURL(any.GetTypeUrl())
	if err != nil {
		return nil, err
	}
	return mt.New().Interface(), nil
  Analysis of user code revealed that this function is seldom used.

* ptypes.MarshalAny is replaced by anypb.Any.MarshalFrom.
  The former creates a new Any message and returns it,
  while the latter is a method that modifies the receiver.

* ptypes.UnmarshalAny is replaced by anypb.Any.UnmarshalTo.

* ptypes.DynamicAny is loosely replaced by anypb.Any.UnmarshalNew.
  The DynamicAny type is a custom proto.Message that is special
  to ptypes.UnmarshalAny where it would allocate a new message
  and store it into the DynamicAny instance. The UnmarshalNew method
  accomplishes the equivalent functionality in a more direct fashion.

* ptypes.TimestampNow is replaced by timestamppb.Now.

* ptypes.TimestampProto is replaced by timestamppb.New.
  The former returned an error if the timestamp was outside the
  10000-year range recommended by timestamp.proto,
  while the latter always succeeded. To preserve the behavior of
  the former validation check, the replacement can additionally
  call the timestamppb.Timestamp.CheckValid method.

* ptypes.Timestamp is replaced by timestamppb.Timestamp.AsTime.
  The former returned an error if the timestamp was outside the
  10000-year range recommended by timestamp.proto,
  while the latter always succeeded. To preserve the behavior of
  the former validation check, the replacement can additionally
  call the timestamppb.Timestamp.CheckValid method.

* ptypes.TimestampString has no direct replacement as it is equivalent to:
	ts.AsTime().Format(time.RFC3339Nano)

* ptypes.DurationProto is replaced by durationpb.New.

* ptypes.Duration is replaced by durationpb.Duration.AsDuration.
  The former returned an error if the duration would overflow
  when converting to a time.Duration, while the latter uses
  saturation arithmetic (similiar to the time package itself).
  Underflow resulted in time.Duration(math.MinInt64), while
  overflow resulted in time.Duration(math.MaxInt64).
  To preserve the behavior of former validation checks,
  the replacement can call the durationpb.Duration.CheckValid method
  and check whether the duration is fixed to one of the overflow values.

Change-Id: Ia996b1037a1fcafced7c7e10e9408ef7fa22863a
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/225298
Reviewed-by: Damien Neil <dneil@google.com>
2020-06-09 21:21:54 +00:00
Joe Tsai
e0b77db13b internal/genid: add new package for generated identifiers
The genid package unifies the genname, fieldnum, and detectknown
packages into a single package.

Whenever possible use the generated constants rather than
hard-coded literals. This makes it easier to search the entire
module for special logic that deal with well-known types.

Change-Id: I13beff1f4149444a0c0b9e607ebf759657f000f4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/235301
Reviewed-by: Herbie Ong <herbie@google.com>
2020-05-29 07:08:23 +00:00
Joe Tsai
3034025d28 internal/impl: avoid inlining fixed coderFieldInfo array
Any attempt at guessing the size for a fixed coderFieldInfo array
will always get it wrong in some cases, either by under-estimating
or over-estimating the count. The former causes worse caching behavior,
while the latter causes memory waste.

As a middle ground, just pre-allocate a slice of the exact length.
Each element will have memory locality with each other, but not
be guaranteed to have memory locality with the parent coderMessageInfo.

name                            old time/op  new time/op  delta
EmptyMessage/Wire/Marshal-8     43.1ns ±11%  42.6ns ± 8%  -1.32%  (p=0.036 n=50+49)
EmptyMessage/Wire/Unmarshal-8   18.6ns ±10%  18.9ns ±12%    ~     (p=0.054 n=50+50)
EmptyMessage/Wire/Validate-8    15.0ns ± 9%  14.7ns ±10%  -2.44%  (p=0.002 n=50+45)
EmptyMessage/Clone-8             163ns ±20%   149ns ±19%  -8.58%  (p=0.000 n=48+53)
RepeatedInt32/Wire/Marshal-8    4.27µs ±12%  4.24µs ±13%    ~     (p=0.612 n=48+52)
RepeatedInt32/Wire/Unmarshal-8  3.47µs ±14%  3.50µs ±11%    ~     (p=0.217 n=50+53)
RepeatedInt32/Wire/Validate-8   2.12µs ±12%  2.09µs ± 9%    ~     (p=0.121 n=50+51)
RepeatedInt32/Clone-8           3.04µs ±18%  2.98µs ±36%    ~     (p=0.289 n=51+54)
Required/Wire/Marshal-8          281ns ±14%   276ns ±11%    ~     (p=0.059 n=48+55)
Required/Wire/Unmarshal-8        117ns ±14%   118ns ±11%    ~     (p=0.358 n=49+53)
Required/Wire/Validate-8        87.6ns ± 9%  88.0ns ±12%    ~     (p=0.373 n=48+53)
Required/Clone-8                 533ns ±12%   507ns ±15%  -4.71%  (p=0.000 n=49+54)

Change-Id: I4cf3134e424130bee728b7591127e5c80f07e2db
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/232937
Reviewed-by: Damien Neil <dneil@google.com>
2020-05-13 03:30:14 +00:00
Joe Tsai
1f5b6fe645 all: improve panic messages for better debugability
Change-Id: If3e505e715d5ce2c9a81249c868d26226a25b724
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/232339
Reviewed-by: Damien Neil <dneil@google.com>
2020-05-05 21:10:54 +00:00
Joe Tsai
d0a499bc65 internal/impl: validate UTF-8 for proto3 optional strings
Change-Id: I090e7c5adac47818831c63d3d999cb7fea5ac696
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/231357
Reviewed-by: Damien Neil <dneil@google.com>
2020-05-01 17:23:32 +00:00
Joe Tsai
3ebaa92e92 all: use v2 Message interface for weak fields
Cleanup the generated logic by having the implementation be backed
by protoimpl rather that directly generated.

Weak fields are a deprecated feature of protobufs and
have entirely be superceded by extensions.
Unfortunately, there are still some usages of it.

Change-Id: Ie1a4b7da253e2ccf5e56627775d9b2fb4090d59a
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/229717
Reviewed-by: Damien Neil <dneil@google.com>
2020-04-29 22:34:29 +00:00
Joe Tsai
387873dd53 all: implement support for proto3 optional semantics
In the upcoming 3.12.x release of protoc, the proto3 language will be
amended to support true presence for scalars. This CL adds support
to both the generator and runtime to support these semantics.

Newly added public API:
	protogen.Plugin.SupportedFeatures
	protoreflect.FieldDescriptor.HasPresence
	protoreflect.FieldDescriptor.HasOptionalKeyword
	protoreflect.OneofDescriptor.IsSynthetic

Change-Id: I7c86bf66d0ae56642109beb5f2132184593747ad
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/230698
Reviewed-by: Damien Neil <dneil@google.com>
2020-04-29 20:02:24 +00:00
Joe Tsai
b57aae9def all: improve extension validation
Changes made:
* Ensure protoreflect.ExtensionType.IsValidInterface never panics,
especially if given a nil interface value.
* Have protoreflect.ExtensionType.IsValid{Interface,Value} only
perform type-checks. It does not do value checks (i.e., whether the
value itself is valid). Value validity is left to when an actual
protoreflect.Message.Set operation is performed.
* Add special-casing on proto.SetExtension to treat an invalid
message or list as functionally equivalent to Clear. This is to
be more consistent with the legacy SetExtension implementation
which never panicked when given such values.
* Add special-casing on proto.HasExtension to treat a mismatched
extension descriptor as simply not being present in the message.
This is also to be more consistent with the legacy HasExtension
implementation which did the same thing.

Change-Id: Idf0419abf27b9f85d9b92bd2ff8088e25b7990cc
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/229558
Reviewed-by: Damien Neil <dneil@google.com>
2020-04-23 06:01:13 +00:00
Damien Neil
a5526f0129 internal/impl: fix off-by-one error in message initialization
This particular off-by-one can never happen in practice. We will crash
if a field has a number one greater than the maximum field number in the
dense array, but if field number N is in the array we will always put
N+1 in it as well.

Fix the off-by-one anyway.

Change-Id: I8c1304f2fc0d7b91036bde3f7ddb7115c21781ca
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/229278
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-04-21 21:23:04 +00:00
Damien Neil
98f56d1bd5 internal/impl: inline coderInfoFields for better cache locality
Microbenchmarks are inconclusive/noisy, but shows a small but noticeable
improvement on internal benchmarks.

Change-Id: Ic46c6aac8a42c4dc749c4f3583d8c8c95e9548b7
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/229277
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-04-21 21:03:31 +00:00
Joe Tsai
7dfcffe5a7 internal/impl: handle extremely old messages
At some point in time, protoc-gen-go actually emitted an XXX_OneofFuncs
with a different signature. Adjust the logic for handling XXX_OneofFuncs
to not assume that the return arguments are in a specific order.

Change-Id: Idd9c09231c4129c655d4a635bb1ae094896a1ff4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/226980
Reviewed-by: Damien Neil <dneil@google.com>
2020-04-02 21:12:23 +00:00
Joe Tsai
a75c9146b7 internal/impl: minor refactoring
Change-Id: I1a5f4ca31fec72a39ba6690af06f4ae8576408cc
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/226897
Reviewed-by: Damien Neil <dneil@google.com>
2020-04-02 00:23:08 +00:00
Joe Tsai
28cb1e4f28 internal/impl: remove deprecated ExtensionField methods
These were originally needed by the legacy implementation,
but not anymore now that it fully wraps the new implementation.

Change-Id: I950958ebfcb7883fc4b72128d22eaba2da5cc62f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/224583
Reviewed-by: Damien Neil <dneil@google.com>
2020-03-23 18:37:32 +00:00
Joe Tsai
cd108d00a8 encoding/protowire: make package publicly available
Change-Id: I95e293c208e787a91d50e29817620535dfeaa7f2
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/219838
Reviewed-by: Damien Neil <dneil@google.com>
2020-03-20 07:42:18 +00:00
Joe Tsai
92af527de9 internal/impl: remove Export.ExtensionDescFromType
This is no longer needed by the old implementation.

Change-Id: I3ba02d37f35f599ec790ec4e627258273883a308
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/223279
Reviewed-by: Damien Neil <dneil@google.com>
2020-03-13 23:34:19 +00:00
Joe Tsai
e05f789c09 proto: fix signature of UnmarshalState and MarshalState
The pseudo-internal MarshalState and UnmarshalState method should
not have a seperate Message argument since it is passed in through
the extensible MarshalInput and UnmarshalInput values.

Change-Id: I838aadaee30e91cdf888ab024e65348c73c1cd7e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/222678
Reviewed-by: Damien Neil <dneil@google.com>
2020-03-09 20:02:17 +00:00
Damien Neil
075e0741aa internal/impl: fix non-atomic access of MessageState.mi
MessageState's mi field must only be accessed via atomic operations.
Rename the field to 'atomicMessageInfo' to make it incorrect access
obvious. Fix one incorrect non-atomic access.

Change-Id: If80343fb1b82186416f007ca5b340a4926b1cd3c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/221419
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-27 20:50:32 +00:00
Damien Neil
5698f90d86 internal/impl: fix messageset validation bug
The validator was not ensuring the the MessageInfo for messageset
items was initialized. Fixed.

One or more of the existing messageset tests fail when run in isolation
due to this bug, but running all of them in sequence passes due to an
earlier test initializing the MessageInfo first.

Change-Id: Ifa7bd525c6d1cef9d1bed7bf761b0380907e35ee
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/221023
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-26 18:56:34 +00:00
Joe Tsai
e0daf31d84 all: trivial formatting changes
Changes:
* import grouping for third-party dependencies
* import grouping for generated protobufs
* blank space removal

Change-Id: I2950b0606bb2064046d79a23a78b05c23147cbfe
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/221017
Reviewed-by: Damien Neil <dneil@google.com>
2020-02-25 21:59:54 +00:00
Damien Neil
37cbbeb558 internal/impl: check for size cache overflow
The size cache is an int32. Store a -1 in it if the message size
overflows, and fall back to recomputing the size if the value is
negative. This means lamentable O(N^2) costs in marshaling,
but that's better than silently producing invalid output.

Also considered: Return an error. Avoids O(N^2) behavior, but gives the
user no good choices if they don't care the output being slow. Encoding
costs of messages this large are likely to be dominated by copying the
bytes rather than the size operation anyway, so slow-but-correct seems
like the most generally useful option.

We could store valid values for the range (0x7fffffff,0xfffffffe)
reserving only 0xffffffff as the overflow sentinel, but optimizing this
case seems less important than the code being obviously correct.

Fixes golang/protobuf#970.

Change-Id: I44f59ff81fdfbc8672dd5aec959d5153a081aab9
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/220593
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-02-24 20:12:47 +00:00
Joe Tsai
fb5fde41cd internal/impl: remove blocking TODO
This TODO doesn't need to be addressed for v2 release.
Addressing this now would probably need to go through global testing,
which is not worth the effort now.

Change-Id: I44bf45e077c00e9a6462905193efb0bce41e9dd2
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/220345
Reviewed-by: Damien Neil <dneil@google.com>
2020-02-20 22:43:06 +00:00
Joe Tsai
f26a9e7e30 all: rename IsInitialized as CheckInitialized
An Is prefix implies it returns a boolean.
A Check prefix better suggests that it could return an error.

Change-Id: I6ffcb32099a944c656c07654c294a0980efb2d0e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/220338
Reviewed-by: Damien Neil <dneil@google.com>
2020-02-20 20:45:40 +00:00
Damien Neil
0232edc1d7 all: remove stray "." from license headers
Remove a stray bit of punctuation that crept into one of the license
headers and got copied around everywhere.

Change-Id: Iebe4e882650ab6dab28f132b5e324e2ab0b99a73
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/220339
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-02-20 18:54:38 +00:00
Damien Neil
466dd77288 all: fast-path method refactoring
Move all fast-path inputs and outputs into the Input/Output structs.
Collapse all booleans into bitfields.

Change-Id: I79ebfbac9cd1d8ef5ec17c4f955311db007391ca
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/219505
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-19 22:01:50 +00:00
Damien Neil
3dbd95a558 reflect/protoreflect: add List.AppendMutable and Map.Mutable
Add methods to add a new, mutable message to a list or map, matching the
existing Message.Mutable.

These methods are purely a convenience, as each can be implemented in
terms of the existing interface.

Change-Id: I889c20fe37ea0f2a566555212e99e6378fb9fe1d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/220117
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-19 20:35:39 +00:00
Damien Neil
725bfeaf40 internal/impl: support legacy Merger interface
Change-Id: I796be0bb1bb40605073228446364f3a2f1073ef1
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/219557
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-18 17:00:39 +00:00
Damien Neil
e8e8875f94 proto, runtime/protoiface, internal/impl: add fast-path Merge
Comparing -tags=protoreflect to fast-path:

name                              old time/op    new time/op    delta
pkg:google.golang.org/protobuf/internal/benchmarks goos:linux goarch:amd64
/Clone/google_message1_proto2-12    1.70µs ± 1%    0.30µs ± 1%  -82.64%  (p=0.001 n=7+7)
/Clone/google_message1_proto3-12    1.01µs ± 1%    0.19µs ± 1%  -80.77%  (p=0.000 n=7+8)
/Clone/google_message2-12            818µs ± 8%     141µs ± 6%  -82.78%  (p=0.000 n=8+8)
pkg:google.golang.org/protobuf/internal/benchmarks/micro goos:linux goarch:amd64
EmptyMessage/Clone-12               51.1ns ± 1%    39.3ns ± 3%  -23.03%  (p=0.000 n=7+8)
RepeatedInt32/Clone-12              24.5µs ± 1%     1.1µs ± 3%  -95.64%  (p=0.000 n=8+8)
Required/Clone-12                    978ns ± 1%     132ns ± 2%  -86.46%  (p=0.000 n=8+8)

name                              old alloc/op   new alloc/op   delta
pkg:google.golang.org/protobuf/internal/benchmarks goos:linux goarch:amd64
/Clone/google_message1_proto2-12    1.08kB ± 0%    0.74kB ± 0%  -31.85%  (p=0.000 n=8+8)
/Clone/google_message1_proto3-12      872B ± 0%      544B ± 0%  -37.61%  (p=0.000 n=8+8)
/Clone/google_message2-12            602kB ± 0%     411kB ± 0%  -31.65%  (p=0.000 n=8+8)
pkg:google.golang.org/protobuf/internal/benchmarks/micro goos:linux goarch:amd64
EmptyMessage/Clone-12                96.0B ± 0%     64.0B ± 0%  -33.33%  (p=0.000 n=8+8)
RepeatedInt32/Clone-12              25.4kB ± 0%     3.2kB ± 0%  -87.33%  (p=0.000 n=8+8)
Required/Clone-12                     416B ± 0%      256B ± 0%  -38.46%  (p=0.000 n=8+8)

name                              old allocs/op  new allocs/op  delta
pkg:google.golang.org/protobuf/internal/benchmarks goos:linux goarch:amd64
/Clone/google_message1_proto2-12      52.0 ± 0%      21.0 ± 0%  -59.62%  (p=0.000 n=8+8)
/Clone/google_message1_proto3-12      33.0 ± 0%       3.0 ± 0%  -90.91%  (p=0.000 n=8+8)
/Clone/google_message2-12            22.3k ± 0%      7.5k ± 0%  -66.41%  (p=0.000 n=8+8)
pkg:google.golang.org/protobuf/internal/benchmarks/micro goos:linux goarch:amd64
EmptyMessage/Clone-12                 3.00 ± 0%      2.00 ± 0%  -33.33%  (p=0.000 n=8+8)
RepeatedInt32/Clone-12               1.51k ± 0%     0.00k ± 0%  -99.80%  (p=0.000 n=8+8)
Required/Clone-12                     51.0 ± 0%      18.0 ± 0%  -64.71%  (p=0.000 n=8+8)

Change-Id: Ife9018097c34cb025dc9c4fdd9a61b2f947853c6
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/219147
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-02-14 21:47:10 +00:00
Damien Neil
316febd1ab internal/impl: pass *coderFieldInfo into fast-path functions
Refactor the fast-path size, marshal, unmarshal, and isinit functions to
take the *coderFieldInfo for the field as input.

This replaces a number of closures capturing field-specific information
with functions taking that information as an explicit parameter.

Change-Id: I8cb39701265edb7b673f6f04a0152d5f4dbb4d5d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/218937
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-02-12 18:34:15 +00:00
Joe Tsai
93bccf763e all: scrub all TODOs
TODOs that we do not intend to address have been deleted.
Those that are blocking v2 release are marked with "blocks".

Change-Id: I7efa9e546d0637b562101d0edc7009893d762722
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/218878
Reviewed-by: Damien Neil <dneil@google.com>
2020-02-10 19:28:48 +00:00
Damien Neil
01b51b4f96 proto, internal/errors: add Error sentinel, errors.Wrap
Add a sentinel proto.Error error which matches all errors returned by
packages in this module.

Document that protoregistry.NotFound is an exact sentinel value for
performance reasons.

Add a Wrap function to the internal/errors package and use it to wrap
errors from outside sources (resolvers). Wrapped errors match
proto.Error.

Fixes golang/protobuf#1021.

Change-Id: I45567df3fd6c8dc9a5caafdb55654827f6fb1941
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215338
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-07 21:09:48 +00:00
Damien Neil
9afe9bb78b internal/impl: validate messagesets
Change-Id: Id90bb386e7481bb9dee5a07889f308f1e1810825
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/218438
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-07 20:06:04 +00:00
Damien Neil
f9d4fdf054 internal/impl: fix validation of required group fields
Change-Id: I3c3b5cfbea599dc08096aa5992b7829c2e50f25d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/218578
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-07 20:05:54 +00:00
Damien Neil
6fb29949b8 all: tests, tweaks for lazy extension decoding
Add a test to confirm that extensions are lazily decoded when we expect.

Drop the UnmarshalDefaultResolver flag. I added it thinking for some
reason that internal/impl couldn't depend on protoregistry; since it can
(and does), it's simpler to just test if the resolver is the expected
value.

Use a default set of options when lazily unmarshaling extensions.

Change-Id: Ied7666ffdc3bf90630260a80c9568d9a945048bc
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/218038
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-02-06 19:43:25 +00:00
Damien Neil
4eefd77886 internal/impl: init map value MessageInfos in validator
I'm not sure how to write a good test for this one, since it's so
specific to both the code and the ordering of initialization. Just
sticking the fuzzer-provided case into our standard test message set
doesn't do it, because something else has initialized the MessageInfo by
the time the test gets there.

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20543
Change-Id: I508222b43e52287f73e2ed32ce9b954a5f81717b
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/218257
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-06 18:58:27 +00:00
Damien Neil
40cba14b26 internal/impl: fix for lazy decoding of groups
Bit of a weird case in why this wasn't caught by tests: When validating
extension groups, we were validating an empty buffer rather than the
message content. For groups, this validation always fails due to a lack
of a group end tag. We'd then skip lazy decoding of the extension field
and proceed with eager decoding, which would behave correctly.

Change extension validation to report an error immediately on an invalid
result from the validator, which is both safe (assuming we trust the
validator) and would have caught this problem (by failing to decode the
extension field, rather than silently failing to eager decoding).

Change-Id: Id6c2d21fb687062bc74d9eb93760a1c24a6fe883
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/217767
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-05 22:49:38 +00:00
Damien Neil
0f783d864b internal/impl: fix off-by-one in varint validation
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20532
Change-Id: I670698a1ef780f341f336929384132febe2b40a1
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/217766
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-05 17:16:50 +00:00
Damien Neil
cadb4ab3b1 internal/impl: refactor validation a bit
Return the size of the field read from the validator, permitting us to
avoid an extra parse when skipping over groups.

Return an UnmarshalOutput from the validator, since it already combines
two of the validator outputs: bytes read and initialization status.

Remove initialization status from the ValidationStatus enum, since it's
covered by the UnmarshalOutput.

Change-Id: I3e684c45d15aa1992d8dc3bde0f608880d34a94b
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/217763
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-05 05:32:50 +00:00
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