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>
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.
Fixesgolang/protobuf#1021.
Change-Id: I45567df3fd6c8dc9a5caafdb55654827f6fb1941
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215338
Reviewed-by: Joe Tsai <joetsai@google.com>
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>
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>
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>
Move the build script from OSS-Fuzz's repo into ours, allowing us to
make changes without sending them a PR.
Change-Id: I557c3be2b6d9fd221ac7e6b1331bf3d53fd3ca51
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/217768
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
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>
Change the protoc flags such that when one of our test .proto files
imports another, the filename is consistently specified relative to the
module root.
Change-Id: I690282795cef23347c8794c1c6357e4fe9560d8a
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/217762
Reviewed-by: Joe Tsai <joetsai@google.com>
Look up a ProtoFile via a message's ParentFile method. Makes it easier
to run the test in environments (blaze, bazel) where the file path may
have changed.
Change-Id: I824f8412ef7db8299961e8df6edac14a13f3e263
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/217761
Reviewed-by: Joe Tsai <joetsai@google.com>
Tweak the "nested unknown extension" test case's resolver to not depend
on the exact message being tested. Useful for if/when we want to run
these tests on other message implementations.
Change-Id: Id1722afd8e094ddb59cb3e5440f7994c20cfa681
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/217760
Reviewed-by: Joe Tsai <joetsai@google.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Required field validation populates a bitmask of observed required
fields. Store a uint64 containing the bit to set in the validationInfo
rather than the index of the bit. Provides a noticeable speed increase
in validation.
name old time/op new time/op delta
EmptyMessage/Wire/Unmarshal 40.2ns ± 1% 40.2ns ± 2% ~ (p=0.860 n=35+37)
EmptyMessage/Wire/Unmarshal-12 7.13ns ± 5% 7.12ns ± 1% ~ (p=0.112 n=37+37)
RepeatedInt32/Wire/Unmarshal 6.57µs ± 1% 6.46µs ± 1% -1.56% (p=0.000 n=39+35)
RepeatedInt32/Wire/Unmarshal-12 1.05µs ± 2% 1.05µs ± 2% ~ (p=0.659 n=37+33)
Required/Wire/Unmarshal 258ns ± 1% 251ns ± 1% -2.87% (p=0.000 n=32+38)
Required/Wire/Unmarshal-12 44.3ns ± 2% 42.4ns ± 1% -4.36% (p=0.000 n=36+37)
Change-Id: Ib1cb74d3e348355a6a2f66aecf8fdc4b58cd84d4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216420
Reviewed-by: Joe Tsai <joetsai@google.com>
Add a place to put microbenchmarks used to justify performance-related changes.
Change-Id: I6e90a3500594b3f6297cee0b8e321a50d0a556ca
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216480
Reviewed-by: Joe Tsai <joetsai@google.com>
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>
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>
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>
The fast-path unmarshal funcs return the number of bytes consumed.
Change these functions to return an unmarshalOutput struct instead, to
make it easier to add to the results. This is groundwork for allowing
the fast-path unmarshaler to indicate when the unmarshaled message is
known to be initialized.
Change-Id: Ia8c44731a88f5be969a55cd98ea26282f412c7ae
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215720
Reviewed-by: Joe Tsai <joetsai@google.com>
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>
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>
