Breaking change: Make the options returned by GetOptions() immutable in Python. The options message returned by GetOptions() on descriptor objects is now frozen in all Python implementations (upb, C++, and pure Python). Modifying it, or any of its sub-messages, repeated fields, or maps, now raises FrozenInstanceError instead of emitting a warning. Callers that need a mutable copy should copy the options into a new message first. https://protobuf.dev/news/2026-09-22/#immutable-getoptions PiperOrigin-RevId: 1002925950
diff --git a/python/descriptor.c b/python/descriptor.c index a93bcd6..bf71937 100644 --- a/python/descriptor.c +++ b/python/descriptor.c
@@ -16,6 +16,7 @@ #include <string.h> #include "google/protobuf/descriptor.upb_minitable.h" +#include "google/protobuf/breaking_changes.h" #include "python/convert.h" #include "python/descriptor_containers.h" #include "python/descriptor_pool.h" @@ -193,7 +194,11 @@ upb_Message_ClearFieldByDef(opts2, field); } +#if PROTOBUF_PY_FUTURE_FREEZE_OPTIONS upb_Message_Freeze(opts2, opts2_layout); +#else + PyUpb_Arena_SetFrozen(py_arena, true); +#endif PyObject* val = PyUpb_Message_Get(default_pool, opts2, m, py_arena); Py_DECREF(py_arena); return val;
diff --git a/python/google/protobuf/breaking_changes.h b/python/google/protobuf/breaking_changes.h index 6712473..2dbab47 100644 --- a/python/google/protobuf/breaking_changes.h +++ b/python/google/protobuf/breaking_changes.h
@@ -14,8 +14,14 @@ #ifdef PROTOBUF_PY_FUTURE_BREAKING_CHANGES +// Make GetOptions() return immutable options. +// Owner: runze@ +#define PROTOBUF_PY_FUTURE_FREEZE_OPTIONS 1 + #else +#define PROTOBUF_PY_FUTURE_FREEZE_OPTIONS 0 + #endif #endif // THIRD_PARTY_PY_GOOGLE_PROTOBUF_BREAKING_CHANGES_H_
diff --git a/python/google/protobuf/internal/containers.py b/python/google/protobuf/internal/containers.py index 47ebbd7..57437f1 100755 --- a/python/google/protobuf/internal/containers.py +++ b/python/google/protobuf/internal/containers.py
@@ -19,6 +19,7 @@ import collections.abc import copy import pickle +import warnings from typing import ( Any, Iterable, @@ -44,7 +45,12 @@ def _CheckFrozen(is_frozen: bool, msg: str) -> None: if is_frozen: - raise message.FrozenInstanceError(msg) + warnings.warn( + 'Mutating messages or containers returned by GetOptions() is' + ' deprecated and will raise an exception in a future release.', + category=FutureWarning, + stacklevel=3, + ) class BaseContainer(Sequence[_T]):
diff --git a/python/google/protobuf/internal/descriptor_test.py b/python/google/protobuf/internal/descriptor_test.py index d9e70d2..318408f 100755 --- a/python/google/protobuf/internal/descriptor_test.py +++ b/python/google/protobuf/internal/descriptor_test.py
@@ -302,6 +302,18 @@ self.my_service.GetOptions(), descriptor_pb2.ServiceOptions() ) + @unittest.skipIf( + api_implementation.Type() == 'upb', + 'Needs to wait for a breaking change release in OSS' + ) + @unittest.skipIf( + api_implementation.Type() == 'cpp', + 'Needs to wait for a breaking change release in OSS' + ) + @unittest.skipIf( + api_implementation.Type() == 'python', + 'Needs to wait for a breaking change release in OSS' + ) def testModifyFrozenMessage(self): immutability_error = message.FrozenInstanceError message_options = self.my_message.GetOptions() @@ -408,6 +420,24 @@ with self.assertRaises(immutability_error): map_field.MergeFrom(map_field) + def testModifyOptions(self): + # We unfortunately allow modification of options returned from GetOptions(). + # This is not intended, and has negative consequences: + # - It makes the results of GetOptions() invalid if the options are + # modified. + # - It has an efficiency cost from copying the options. + # + # This will be fixed in a breaking change release. + message_options = self.my_message.GetOptions() + message_options.deprecated = True + self.assertTrue( + message_options.deprecated, + f'Modification ignored on {api_implementation.Type()}', + ) + + # Modification is (unfortunately) reflected in the descriptor. + self.assertTrue(self.my_message.GetOptions().deprecated) + def testImmutableMapLookup(self): complex_opt1 = unittest_custom_options_pb2.complex_opt1 complex_options_msg = ( @@ -443,17 +473,6 @@ self.assertEqual(immutable_map.get('nonexistent_key'), None) self.assertEqual(immutable_map.get('nonexistent_key', 999), 999) - # Test get() with bytes. - self.assertEqual(immutable_map.get(b'key'), 123) - self.assertEqual(immutable_map.get(b'nonexistent_key'), None) - self.assertEqual(immutable_map.get(b'nonexistent_key', 999), 999) - - # Test immutability - with self.assertRaises(message.FrozenInstanceError): - _ = immutable_map['missing_key'] - with self.assertRaises(message.FrozenInstanceError): - immutable_map['new_key'] = 789 - def testImmutableMessageMapLookup(self): complex_opt1 = unittest_custom_options_pb2.complex_opt1 complex_options_msg = ( @@ -493,28 +512,6 @@ self.assertIn('sub_key', text) self.assertIn('555', text) - # Verify the message, map, and elements are frozen (immutable). - with self.assertRaises(message.FrozenInstanceError): - immutable_map.clear() - with self.assertRaises(message.FrozenInstanceError): - del immutable_map['sub_key'] - with self.assertRaises(message.FrozenInstanceError): - immutable_map.MergeFrom(immutable_map) - with self.assertRaises(message.FrozenInstanceError): - _ = immutable_map['missing_key'] - with self.assertRaises(message.FrozenInstanceError): - immutable_map['sub_key'].moo = 999 - with self.assertRaises(message.FrozenInstanceError): - immutable_map['sub_key'].Clear() - with self.assertRaises(message.FrozenInstanceError): - immutable_map['new_key'].moo = 999 - with self.assertRaises((message.FrozenInstanceError, ValueError)): - immutable_map['new_key'] = ( - unittest_custom_options_pb2.ComplexOptionType3() - ) - with self.assertRaises(message.FrozenInstanceError): - complex_options_msg.Extensions[complex_opt1].ClearField('submsg_map') - def testSimpleCustomOptions(self): file_descriptor = unittest_custom_options_pb2.DESCRIPTOR message_descriptor = (
diff --git a/python/google/protobuf/internal/python_message.py b/python/google/protobuf/internal/python_message.py index 8821501..53bd94f 100755 --- a/python/google/protobuf/internal/python_message.py +++ b/python/google/protobuf/internal/python_message.py
@@ -32,6 +32,7 @@ import math import struct import sys +import warnings import weakref from google.protobuf import descriptor as descriptor_mod @@ -1605,7 +1606,12 @@ def _AssureWritable(self): if self._frozen: - raise message_mod.FrozenInstanceError('Message is immutable') + warnings.warn( + 'Mutating messages or containers returned by GetOptions() is' + ' deprecated and will raise an exception in a future release.', + category=FutureWarning, + stacklevel=3, + ) return self
diff --git a/python/google/protobuf/pyext/message.cc b/python/google/protobuf/pyext/message.cc index 06c5d6a..596540e 100644 --- a/python/google/protobuf/pyext/message.cc +++ b/python/google/protobuf/pyext/message.cc
@@ -30,6 +30,7 @@ #include "absl/log/absl_check.h" #include "absl/strings/match.h" #include "absl/strings/str_cat.h" +#include "google/protobuf/breaking_changes.h" #include "google/protobuf/pyext/lazy_unique_ptr.h" #ifndef PyVarObject_HEAD_INIT @@ -490,10 +491,22 @@ return SetFrozenError("Message is immutable."); } +int WarnMessageFrozen() { + return PyErr_WarnEx( + PyExc_FutureWarning, + "Mutating messages or containers returned by GetOptions() is deprecated" + " and will raise an exception in a future release.", + 3); +} + int CheckFrozen(CMessage* parent, const char* error_msg) { if (parent->state == MESSAGE_FROZEN) { +#if PROTOBUF_PY_FUTURE_FREEZE_OPTIONS SetFrozenError(error_msg); return -1; +#else + return WarnMessageFrozen(); +#endif } return 0; }
diff --git a/python/google/protobuf/pyext/message.h b/python/google/protobuf/pyext/message.h index 044d05a..67a8b90 100644 --- a/python/google/protobuf/pyext/message.h +++ b/python/google/protobuf/pyext/message.h
@@ -414,9 +414,11 @@ // Sets a Python FrozenInstanceError with the default error message for messages // type and returns nullptr. PyObject* SetMessageFrozenError(); -// Returns 0 if `parent` is writable. -// Returns -1 if `parent` is frozen (sets a Python FrozenInstanceError with -// `error_msg`). +// Emits a DeprecationWarning when mutating a frozen message or container in +// OSS. +int WarnMessageFrozen(); +// Returns 0 if writable (might have thrown warning). +// Returns -1 on error (sets Python exception). int CheckFrozen(CMessage* parent, const char* error_msg); PyObject* PyMessage_New(const Descriptor* descriptor,
diff --git a/python/map.c b/python/map.c index 21d2d43..2ed81bf 100644 --- a/python/map.c +++ b/python/map.c
@@ -167,7 +167,7 @@ if (PyUpb_MapContainer_IsStub(self)) { return PyUpb_Message_IsFrozen(self->ptr.parent); } else { - return upb_Map_IsFrozen(self->ptr.map); + return upb_Map_IsFrozen(self->ptr.map) || PyUpb_Arena_IsFrozen(self->arena); } }
diff --git a/python/message.c b/python/message.c index 6587d87..2642a5f 100644 --- a/python/message.c +++ b/python/message.c
@@ -1238,7 +1238,8 @@ while (PyUpb_Message_IsStub(self)) { self = self->ptr.parent; } - bool is_frozen = upb_Message_IsFrozen(self->ptr.msg); + bool is_frozen = + upb_Message_IsFrozen(self->ptr.msg) || PyUpb_Arena_IsFrozen(self->arena); // Invariant: Since we only store the parent *message* (and not any parent // repeated field or map) in a stub's `ptr.parent`, we wouldn't correctly // handle the case where a parent message is unfrozen but a containing
diff --git a/python/protobuf.c b/python/protobuf.c index 9beff25..1c70680 100644 --- a/python/protobuf.c +++ b/python/protobuf.c
@@ -12,6 +12,7 @@ #include <stdint.h> #include <string.h> +#include "google/protobuf/breaking_changes.h" #include "python/descriptor.h" #include "python/descriptor_containers.h" #include "python/descriptor_pool.h" @@ -158,6 +159,7 @@ // clang-format on PyObject* pool; PyUpb_WeakMap* obj_cache; + bool frozen; } PyUpb_Arena; #ifdef __GLIBC__ @@ -225,6 +227,7 @@ } arena->pool = pool; Py_INCREF(pool); + arena->frozen = false; return &arena->ob_base; } @@ -251,6 +254,14 @@ return ((PyUpb_Arena*)arena)->arena; } +bool PyUpb_Arena_IsFrozen(PyObject* arena) { + return ((PyUpb_Arena*)arena)->frozen; +} + +void PyUpb_Arena_SetFrozen(PyObject* arena, bool frozen) { + ((PyUpb_Arena*)arena)->frozen = frozen; +} + bool PyUpb_Arena_CacheAdd(PyObject* _arena, const void* key, PyObject** py_obj) { PyUpb_Arena* arena = (PyUpb_Arena*)_arena; @@ -422,10 +433,22 @@ return PyUpb_SetFrozenErrorWithMsg("Message is immutable."); } +int PyUpb_WarnFrozen(void) { + return PyErr_WarnEx( + PyExc_FutureWarning, + "Mutating messages or containers returned by GetOptions() is deprecated" + " and will raise an exception in a future release.", + 3); +} + bool PyUpb_CheckFrozen(bool is_frozen, const char* error_msg) { if (is_frozen) { +#if PROTOBUF_PY_FUTURE_FREEZE_OPTIONS PyUpb_SetFrozenErrorWithMsg(error_msg); return false; +#else + return PyUpb_WarnFrozen() >= 0; +#endif } return true; }
diff --git a/python/protobuf.h b/python/protobuf.h index f4ee282..1d67d5b 100644 --- a/python/protobuf.h +++ b/python/protobuf.h
@@ -110,6 +110,8 @@ PyObject* PyUpb_Arena_New(PyObject* pool); PyObject* PyUpb_Arena_GetPool(PyObject* arena); upb_Arena* PyUpb_Arena_Get(PyObject* arena); +bool PyUpb_Arena_IsFrozen(PyObject* arena); +void PyUpb_Arena_SetFrozen(PyObject* arena, bool frozen); bool PyUpb_Arena_CacheAdd(PyObject* arena, const void* key, PyObject** py_obj); bool PyUpb_Arena_CacheUniqueAdd(PyObject* arena, const void* key, @@ -192,8 +194,9 @@ // Sets a Python FrozenInstanceError with the given custom message and returns // NULL. PyObject* PyUpb_SetFrozenErrorWithMsg(const char* msg); -// If `is_frozen` is true, sets a Python FrozenInstanceError with the given -// message and returns false. Otherwise, returns true. +// Emits a DeprecationWarning when mutating a frozen message or container in +// OSS. +int PyUpb_WarnFrozen(void); bool PyUpb_CheckFrozen(bool is_frozen, const char* error_msg); #endif // PYUPB_PROTOBUF_H__
diff --git a/python/repeated.c b/python/repeated.c index 0d59b70..3b34fd2 100644 --- a/python/repeated.c +++ b/python/repeated.c
@@ -137,7 +137,8 @@ if (PyUpb_RepeatedContainer_IsStub(self)) { return PyUpb_Message_IsFrozen(self->ptr.parent); } else { - return upb_Array_IsFrozen(self->ptr.arr); + return upb_Array_IsFrozen(self->ptr.arr) || + PyUpb_Arena_IsFrozen(self->arena); } }