This was previously fixed in C++ (https://github.com/protocolbuffers/protobuf/issues/16549), but not ported to other languages. Delimited field encoding can be inherited by fields where it's invalid, such as non-messages and maps. In these cases, the encoding should be ignored and length-prefixed should be used.
PiperOrigin-RevId: 642792988
This updates all our text parsers and serializers to better handle tag-delimited fields under editions. Under proto2, groups were the only tag-delimited fields possible, and the group name (i.e. the message type) was guaranteed to be unique. Text-format and various generators used this instead of the synthetic field name (lower-cased group name) to represent these fields.
Under editions, we've removed group syntax and allowed any message field to be tag-delimited. This breaks those cases when adding new tag-delimited fields where the message type might not be unique or correspond to the field name. Code generators have already been fixed to treat "group-like" fields using the old behavior, and treat new fields like any other sub-message.
This change addresses the text-format issue. Text parsers will accept *either* the type or field name for "group-like" fields, and only the field name for every other message field. Text serializers will continue to emit the message name for "group-like" fields, but also use the field name for everything else.
This creates some awkward capitalization behavior for fields that happen to *look* like proto2 groups, but it won't lead to any conflicts or invalid encodings. A feature will likely be added to edition 2024 to allow for migration off this legacy behavior.
PiperOrigin-RevId: 622260327
This also fixes a few minor bugs in the editions implementation that were caught in python/conformance tests, and adds a new SetFeatureSetDefaults API to the def pool for consistency with C++ and other python implementations.
PiperOrigin-RevId: 581384108
This required enabling the feature in the code generator and fixing a few edge cases around label and type.
Also added tests to verify the special cases, and to verify that required fields work as expected.
PiperOrigin-RevId: 580263087
This simplifies the logic when the type of a field is only known at a later time (when the field is resolved).
This brings upb in line with the C++ behavior.
PiperOrigin-RevId: 573338934
This change moves almost everything in the `upb/` directory up one level, so
that for example `upb/upb/generated_code_support.h` becomes just
`upb/generated_code_support.h`. The only exceptions I made to this were that I
left `upb/cmake` and `upb/BUILD` where they are, mostly because that avoids
conflict with other files and the current locations seem reasonable for now.
The `python/` directory is a little bit of a challenge because we had to merge
the existing directory there with `upb/python/`. I made `upb/python/BUILD` into
the BUILD file for the merged directory, and it effectively loads the contents
of the other BUILD file via `python/build_targets.bzl`, but I plan to clean
this up soon.
PiperOrigin-RevId: 568651768
Prior to this CL, upb MiniDescriptors only allowed per-message control over UTF-8. This CL adds a new field-level modifier to upb MiniDescriptors that can flip a field to the opposite of the message default, like we already have for packed, allowing per-field control over UTF-8 validation.
PiperOrigin-RevId: 566638331
A couple weeks ago we moved upb into the protobuf Git repo, and this change
continues the merger of the two repos by making them into a single Bazel repo.
This was mostly a matter of deleting upb's WORKSPACE file and fixing up a bunch
of references to reflect the new structure.
Most of the changes are pretty mechanical, but one thing that needed more
invasive changes was the Python script for generating CMakeLists.txt,
make_cmakelists.py. The WORKSPACE file it relied on no longer exists with this
change, so I updated it to hardcode the information it needed from that file.
PiperOrigin-RevId: 564810016
This is the second attempt to fix our Git history. This should allow
"git blame" to work correctly in the upb/ directory even though our
automation unexpectedly blew away that directory.
When build a FileDescriptorProto into pool, FieldDescriptorProto.type may not set if type_name is set. Runtime Change. To make copybara happy, tests will be added in a separate change
PiperOrigin-RevId: 553881407
When build a FileDescriptorProto into pool, FieldDescriptorProto.type may not set if type_name is set. Runtime Change. To make copybara happy, tests will be added in a separate change
PiperOrigin-RevId: 553598520
When build a FileDescriptorProto into pool, FieldDescriptorProto.type may not set if type_name is set. Runtime Change. To make copybara happy, tests will be added in a separate change
PiperOrigin-RevId: 552521056
After this change, `mini_table` only has MiniTable definitions themselves. Everything having to do with the MiniDescriptor wire format is in `mini_descriptor`.
Also rearranged some of the files in mini_table to have better structure for `internal/`.
This CL contains no functional change.
PiperOrigin-RevId: 543529112
The upb convention is that "_Build()" means to also allocate, which this function does not do, so rename it as "_Init()" to free up the name for a future function that does allocate.
PiperOrigin-RevId: 510282736
Prior to this CL we were allocating a MiniTable for each message and then overwriting it later. This could lead to an inconsistent state, and is unnecessary. This CL adds an extra phase to initialization so that the MiniTable is assigned only one time for each message.
PiperOrigin-RevId: 507617479
The initial motivation for this change was to fix a bug found by fuzzing. The old fuzz test (built on `cc_fuzz_target()`) detected an infinite loop if a bytes field default has an unterminated `\x` escape.
To fix this bug while expanding fuzz coverage, I created a fuzz test that verifies that we can do a lossless round trip from descriptor -> DefPool -> descriptor. We use C++ as the source of truth for whether a descriptor is valid or not, and what the canonical serialization back to protobuf form should be.
I wrote the new fuzz test using go/FuzzTest, which makes it easier and more readable to use an arbitrary `FileDescriptorSet` as input, while adding test cases for regressions.
The fuzz test highlighted a handful of errors that I subsequently fixed and added regression tests for:
1. The aforementioned unterminated `\x` bug.
2. We were not propagating the `edition` field.
3. We were missing the CheckIdent() check in a few places.
4. We were rejecting files with empty name, whereas C++ allows this.
5. There were a few bugs with escaping string defaults.
Since FuzzTest is Clang-only, I split the `FUZZ_TEST()` invocation from the regression tests, since the latter are portable and should be run on all platforms. Only `FUZZ_TEST()` itself is in a google3/Clang-only file.
PiperOrigin-RevId: 506997362
Slight optimization that frees us from needing to backtrack up to the owning file def to extract the proto syntax bit. Costs zero additional storage since we already have available unused bits. Also makes the enum def the single source of truth for determining enum syntax - upb_FieldDef_IsClosedEnum() now just passes off to upb_EnumDef_IsClosed() instead of replicating that code.
PiperOrigin-RevId: 505513429
We have previously been using Copybara to rewrite these names, but for bootstrapping we will want to be able to sometimes use OSS names inside google3.
PiperOrigin-RevId: 500294974
upb_MiniTable_BuildEnum() -> upb_MiniTableEnum_Build()
upb_MiniTable_BuildExtension() -> upb_MiniTableExtension_Build()
also make the status pointer argument optional for the mini table builders
PiperOrigin-RevId: 490992866
Prior to this CL, there were several different code paths for reading/writing message data. Generated code, MiniTable accessors, and reflection all performed direct manipulation of the bits and bytes in a message, but they all had distinct implementations that did not share much of any code. This divergence meant that they could easily have different behavior, bugs could creep into one but not another, and we would need three different sets of tests to get full test coverage. This also made it very difficult to change the internal representation in any way, since it would require updating many places in the code.
With this CL, the three different APIs for accessing message data now all share a common set of functions. The common functions all take a `upb_MiniTableField` as the canonical description of a field's type and layout. The lowest-level functions are very branchy, as they must test for every possible variation in the field type (field vs oneof, hasbit vs no-hasbit, different field sizes, whether a nonzero default value exists, extension vs. regular field), however these functions are declared inline and designed to be very optimizable when values are known at compile time.
In generated accessors, for example, we can declare constant `upb_MiniTableField` instances so that all values can constant-propagate, and we can get fully specialized code even though we are calling a generic function. On the other hand, when we use the generic functions from reflection, we get runtime branches since values are not known at compile time. But even the function is written to still be as efficient as possible even when used from reflection. For example, we use memcpy() calls with constant length so that the compiler can optimize these into inline loads/stores without having to make an out-of-line call to memcpy().
In this way, this CL should be a benefit to both correctness and performance. It will also make it easier to change the message representation, for example to optimize the encoder by giving hasbits to all fields.
Note that we have not completely consolidated all access in this CL:
1. Some functions outside of get/set such as clear and hazzers are not yet unified.
2. The encoder and decoder still touch the message without going through the common functions. The encoder and decoder require a bit more specialized code to get good performance when reading/writing fields en masse.
PiperOrigin-RevId: 490016095
Joshua Haberman
Mike Kruskal
Hong Shin
Eric Salo
Adam Cozzette
Sandy Zhang
Protobuf Team Bot
Jie Luo