This removes an unnecessary branch in validation code and will allow reducing
the size of ExtensionInfo in a future change.
PiperOrigin-RevId: 967326524
Provide dummy argument for variadic macro to satisfy pre-C23 requirements.
Remove trailing `;` that duplicates `;` generated by macro expansion.
PiperOrigin-RevId: 967288052
0-length packed fields occur but are fairly rare since our serializers will emit nothing instead of emitting a 0-length packed field. But, there's little reason to leave inequality case behind since we're checking against <=0 anyway.
PiperOrigin-RevId: 967271476
We previously had a hack in Python/C++ Protobuf to account for the fact that LazyField did not properly remember a custom DescriptorPool or MessageFactory that was set in the ParseContext at parse time.
The code has since been fixed to properly handle the case where ParseContext contains a custom DescriptorPool/MessageFactory. Removing the hack removes the data race under free threading.
PiperOrigin-RevId: 967236272
Place "hidden" attribute behind macro conditioned on compiler support.
Disable UPB_FASTTABLE on Windows, as MSVC doesn't support required compiler features. (Existing detection can be confounded on Windows for clang-cl, which may run in normal mode during codegen but compile final binaries in MSVC compat mode.)
PiperOrigin-RevId: 966832345
The unit tests added in the initial implementation in cl/925012500 and
cl/922729754 were fairly minimal. I took some inspiration from our C++
and Java unit tests for JSON's custom enum name parsing to come up with
these new test cases.
This change also adds a python Benchmark that we can use to verify that
our optimizations in cl/952170251 actually works.
Benchmark results across all three proto implementations (cpp, python, upb):
=== C++ Extension Protos (json_format_benchmark_cpp_protos) ===
[BENCHMARK] ParseJsonDefault: med 8.70 us/op | p99 9.56 us/op | mean 8.74 ± 0.15 us/op
[BENCHMARK] ParseJsonCustom: med 17.39 us/op | p99 21.64 us/op | mean 17.43 ± 0.51 us/op
[BENCHMARK] ParseJsonUnknownIgnored: med 26.97 us/op | p99 32.04 us/op | mean 27.06 ± 0.65 us/op
[BENCHMARK] ParseRepeatedJsonDefault: med 1.71 us/item | p99 1.83 us/item | mean 1.71 ± 0.02 us/item
[BENCHMARK] ParseRepeatedJsonCustom: med 9.18 us/item | p99 9.88 us/item | mean 9.20 ± 0.11 us/item
[BENCHMARK] ParseRepeatedJsonUnknownIgnored: med 18.79 us/item | p99 19.19 us/item | mean 18.81 ± 0.13 us/item
=== UPB Extension Protos (json_format_benchmark_upb_protos) ===
[BENCHMARK] ParseJsonDefault: med 8.70 us/op | p99 9.56 us/op | mean 8.74 ± 0.15 us/op
[BENCHMARK] ParseJsonCustom: med 17.39 us/op | p99 21.64 us/op | mean 17.43 ± 0.51 us/op
[BENCHMARK] ParseJsonUnknownIgnored: med 26.97 us/op | p99 32.04 us/op | mean 27.06 ± 0.65 us/op
[BENCHMARK] ParseRepeatedJsonDefault: med 1.71 us/item | p99 1.83 us/item | mean 1.71 ± 0.02 us/item
[BENCHMARK] ParseRepeatedJsonCustom: med 9.18 us/item | p99 9.88 us/item | mean 9.20 ± 0.11 us/item
[BENCHMARK] ParseRepeatedJsonUnknownIgnored: med 18.79 us/item | p99 19.19 us/item | mean 18.81 ± 0.13 us/item
=== Pure Python Protos (json_format_benchmark_python_protos) ===
[BENCHMARK] ParseJsonDefault: med 10.26 us/op | p99 11.20 us/op | mean 10.30 ± 0.25 us/op
[BENCHMARK] ParseJsonCustom: med 16.59 us/op | p99 18.35 us/op | mean 16.70 ± 0.45 us/op
[BENCHMARK] ParseJsonUnknownIgnored: med 25.09 us/op | p99 28.22 us/op | mean 25.27 ± 0.65 us/op
[BENCHMARK] ParseRepeatedJsonDefault: med 1.94 us/item | p99 3.25 us/item | mean 1.97 ± 0.14 us/item
[BENCHMARK] ParseRepeatedJsonCustom: med 7.16 us/item | p99 10.35 us/item | mean 7.24 ± 0.39 us/item
[BENCHMARK] ParseRepeatedJsonUnknownIgnored: med 15.43 us/item | p99 19.54 us/item | mean 15.54 ± 0.49 us/item
PiperOrigin-RevId: 966820541
When fasttable is enabled, generated minitable headers transitively include internal fasttable headers (such as field_parsers.h / combinations.h / types.h). Adding field_parsers to upb_cinterop's deps under fasttable_enabled_setting ensures that these declarations are provided centrally by upb_cinterop.klib and not duplicated across individual proto libraries.
PiperOrigin-RevId: 966816120
The repeated field destructor caches the element destructor and calls it on each element of a repeated field, rather than going through full dynamic dispatch for each element. Currently, if the elements of a repeated field are of different types, this code may segfault in non-obvious ways (e.g. mismatch size-delete, or memory corruption). The DCHECK gives a better error message when you have repeated fields of mixed element types.
This code is only active with `enable_custom_vtable`.
PiperOrigin-RevId: 966610091
The version number is set to 7.1.0 and the contents of the example would never work on 7.x.
Our CI always sets a version which is why this not mattering, but it appears that if you hold it exactly right bazelisk will use this version if present, but fall back to latest bazel if not. Just deleting the file so we don't let it rot again.
PiperOrigin-RevId: 966558235
Update _message to use rules_python's py_extension macro (which
automatically injects required toolchain headers and manages shared library
extensions across platforms), and update _api_implementation.so to depend
directly on `@rules_python//python/cc:current_py_cc_headers` instead of
`@system_python`.
Using `py_extension` and `current_py_cc_headers` decouples Python C
extensions from the custom system_python.bzl script while maintaining
current host-interpreter behavior during the migration.
#test-continuous
PiperOrigin-RevId: 966345187
This change eliminates redundant debug info from being generated for `Clear` / `ByteSizeLong` / `_InternalSerialize` outside the TU compiling `.pb.cc` for a message. It does so via [constructor homing](https://blog.llvm.org/posts/2021-04-05-constructor-homing-for-debug-info/) on the `Helpers_` class.
This is only in effects when `:enable_custom_vtable=true`.
PiperOrigin-RevId: 966258037
## Summary
Follow-up to #28667, which added character validation for `php_namespace`, `php_metadata_namespace`, `ruby_package` and `csharp_namespace` because they are emitted directly into generated source without validation.
Two options in the same family were not covered and are still unvalidated on `main`:
| option | read at | validated before this PR |
|---|---|---|
| `php_class_prefix` | `php/names.cc` (`ClassNamePrefixImpl`) | no |
| `objc_class_prefix` | `objectivec/names.cc` (`FileClassPrefix`) | no |
`php_class_prefix` is easy to miss by inspection: the guard added in #28667 and both of its call sites are in `php_generator.cc`, while `php_class_prefix` is read from `names.cc`.
## What happens today
**PHP.** The generator emits `class <php_class_prefix><Name> extends ...`, so a prefix that closes the declaration and opens its own is emitted verbatim:
```proto
option php_class_prefix = "Pfx{} class Injected { public static function pwn() { return 'INJECTED'; } } class ";
```
`protoc --php_out` exits 0 and the generated file contains:
```php
class Pfx{} class Injected { public static function pwn() { return 'INJECTED'; } } class M extends \Google\Protobuf\Internal\Message
```
That is valid PHP (`php -l`: no syntax errors), the injected class is defined and its code runs, and `Tp\M` is still generated correctly — so nothing fails to build and nothing looks wrong.
**Objective-C.** `FileClassPrefix` returns the option verbatim, so with `objc_class_prefix = "A\";void pwn(){}//"` the value lands in `@interface` / `@implementation` declarations, a `typedef struct` tag and the `.prefix = "..."` string literal. I have not built the resulting Objective-C, so I am claiming unvalidated emission here rather than a working payload.
Worth noting separately: a prefix check already exists, but it validates style rather than characters, so it warns about a legitimate short prefix and says nothing about a hostile one:
```
objc_class_prefix = "AB" -> warning: Invalid 'option objc_class_prefix = "AB";'
objc_class_prefix = "A\";void pwn(){}//" -> no diagnostic at all
```
## Changes
- **PHP generator** — generalised `IsValidPhpNamespace` to `IsValidPhpOption(value, option_name, error)` so the error names the offending option, and applied it to `php_class_prefix` alongside the two existing options. The message still begins `Invalid character`, so the tests added in #28667 are unaffected.
- **Objective-C generator** — new `ValidateObjCClassPrefixChars()` rejects prefixes containing anything outside `[A-Za-z0-9_]`, run from `ValidateObjCClassPrefixes()` over every file ahead of the expected-prefixes opt-outs.
- Tests for both, in the reject/accept pattern used by #28667.
## Notes for review
Three things I'd rather flag than have you find:
1. **The Objective-C check is an allowlist, not a denylist.** The prefix is pasted into C identifiers, where nothing outside `[A-Za-z0-9_]` is ever valid, so an allowlist closes the class rather than chasing characters. I checked every `objc_class_prefix` in this repo (including the deliberately-empty one) and none would be rejected. Happy to switch to the `absl::CharSet` denylist used for PHP/Ruby/C# if you'd prefer consistency over precision.
2. **Placement matters.** The character check runs in `ValidateObjCClassPrefixes()` over every file, ahead of both documented opt-outs (`expected_prefixes_path=-` and `expected_prefixes_suppressions`), since disabling the naming *policy* should not disable a correctness check. There is a regression test per opt-out.
3. **Java is gated, but to a much narrower bar** — `java/file.cc` rejects only `;`, `\r`, `\n` and space, and its comment says it checks "only a subset". Since Java accepts `/**/` as a whitespace substitute, `java_outer_classname = "Evil/**/implements/**/Cloneable"` and `"Evil/*"` are both emitted into the class declaration on current `main`. I could not get code execution out of that (`;` blocks statement injection); the demonstrated effect is generated code that will not compile. **Not changed in this PR** — mentioning it in case you want the bar aligned, and happy to do it in a follow-up.
Also, for whatever it's worth to release planning: #28667 itself is not in any released binary yet — the validation string is absent from both `protoc 35.1` and `protoc 36.0-rc2`, since it landed after the v36 branch was cut.
## Testing
`src/google/protobuf/compiler/php/generator_unittest.cc` — `InvalidPhpClassPrefixRejected`, `ValidPhpClassPrefixAccepted`.
`src/google/protobuf/compiler/objectivec/generator_unittest.cc` (new file, plus its `cc_test` target) — `InvalidObjCClassPrefixRejected`, `InvalidObjCClassPrefixRejectedWithExpectedPrefixesDisabled`, `InvalidObjCClassPrefixRejectedWhenSuppressed`, `ValidObjCClassPrefixAccepted`, `ObjCClassPrefixWithUnderscoreAccepted`.
The two opt-out tests fail with the character check in its original location inside `ValidateObjCClassPrefix()`, and pass with it moved.
Both directions are covered, so the tests fail if the validation is removed *and* if it is made too strict.
---
_Both tests were run locally against this branch (`bazel test //src/google/protobuf/compiler/php:generator_unittest //src/google/protobuf/compiler/objectivec:generator_unittest`), and re-run with the two guards reverted to confirm they fail without the fix._
Closes#29130
COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/29130 from BarakSrour:validate-class-prefix-options a2bbd8f2ea
PiperOrigin-RevId: 966161028
Fixes#10486.
When an input message passed to `protoc --encode` or `--decode` is missing required fields, protoc prints `warning: Input message is missing required fields: ...`. That warning was written straight to stderr in `EncodeOrDecode` and never influenced the exit code, so protoc returned 0 even with `--fatal_warnings` set, defeating its `-Werror`-like purpose.
This makes `EncodeOrDecode` return a non-zero exit when `--fatal_warnings` is set and the message is missing required fields. The check happens after the output is written, matching the compile path (which also produces its output before failing). When `--fatal_warnings` is not set, behavior is unchanged: the output and the warning are still produced and the exit code stays 0.
A maintainer confirmed this is a valid bug and invited a PR on the issue.
Test: added `EncodeDecodeTest.PartialWithFatalWarnings` asserting a non-zero exit (and that the warning is still emitted) when encoding a message with missing required fields under `--fatal_warnings`.
./build/tests --gtest_filter='*EncodeDecodeTest.*'
Result: 18 tests pass, including the new one.
Closes#28033
COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/28033 from yashanil98:fix/issue-10486-fatal-warnings-encode-decode d0bb423371
PiperOrigin-RevId: 965995070
