-
Notifications
You must be signed in to change notification settings - Fork 16.1k
Description
What version of protobuf and what language are you using?
Version: main (commit 18da8e1)
Language: Python (C extension / pyext)
What operating system (Linux, Windows, ...) and version?
Linux (Debian, kernel 6.x) - bug is platform-independent, pure C code logic
What runtime / compiler are you using (e.g., python version or gcc version)
Python 3.13.5, GCC 14.2.0
What did you do?
While reviewing python/google/protobuf/pyext/message.cc after the recent FixupMessageAfterMerge fix (ab14c0f), I noticed the same pattern of missing error handling across the file.
AssureWritable() returns -1 on failure but 13 call sites discard the return value:
InitWKTOrMerge() — line 1025
InitAttributes() — line 1234
ClearFieldByDescriptor() — line 1629
ClearField() — line 1641
Clear() — line 1663
MergeFrom() — line 1883
CopyFrom() — line 1929
MergeFromString() — line 1962
SetInParent() — line 2028
DiscardUnknownFields() — line 2136
SetFieldValue() — line 2726, 2741
PyMessage_GetMutableMessagePointer() — line 2924
The only caller that checks the return value is AssureWritable() itself at line 845.
What did you expect to see
Each site should propagate the error, e.g.:
if (AssureWritable(self) < 0) return nullptr; // or return -1What did you see instead?
When AssureWritable() fails, self->message still points to the shared read-only default instance. The caller proceeds to mutate it via MergeFrom, CopyFrom, ClearField, etc. This corrupts the global default instance , all future messages of that type see wrong defaults
Anything else we should know about your project / environment
Triggered under OOM. Same file, same error handling pattern as ab14c0f