Emit future warning when mutating GetOptions() in OSS

PiperOrigin-RevId: 946657065

python/google/protobuf/descriptor.py

@@ -245,6 +245,12 @@ class DescriptorBase(metaclass=DescriptorMetaclass):
     # If either has been reset by gencode, reload options.
     if not self._options or not self._loaded_options:
       self._LazyLoadOptions()
+    if (
+        self._options
+        and hasattr(self._options, '_SetFrozen')
+        and not getattr(self._options, '_frozen', False)
+    ):
+      self._options._SetFrozen()
     return self._options
 
 

python/google/protobuf/internal/containers.py

@@ -19,6 +19,7 @@ are:
 import collections.abc
 import copy
 import pickle
+import warnings
 from typing import (
     Any,
     Iterable,
@@ -42,6 +43,16 @@ from google.protobuf.descriptor import FieldDescriptor
 from google.protobuf import message
 
 
+def _CheckFrozen(is_frozen: bool, msg: str) -> None:
+  if is_frozen:
+    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]):
   """Base container class."""
 
@@ -89,8 +100,7 @@ class BaseContainer(Sequence[_T]):
     self._frozen = True
 
   def _AssureWritable(self) -> 'BaseContainer[_T]':
-    if self._frozen:
-      raise message.FrozenInstanceError('Container is immutable')
+    _CheckFrozen(self._frozen, 'Container is immutable')
     return self
 
   def sort(self, *args, **kwargs) -> None:
@@ -447,8 +457,7 @@ class ScalarMap(MutableMapping[_K, _V]):
     self._frozen = True
 
   def _AssureWritable(self) -> 'ScalarMap[_K, _V]':
-    if self._frozen:
-      raise message.FrozenInstanceError('Map is immutable')
+    _CheckFrozen(self._frozen, 'Map is immutable')
     return self
 
   def __getitem__(self, key: _K) -> _V:
@@ -582,8 +591,7 @@ class MessageMap(MutableMapping[_K, _V]):
       val._SetFrozen()
 
   def _AssureWritable(self) -> 'MessageMap[_K, _V]':
-    if self._frozen:
-      raise message.FrozenInstanceError('Map is immutable')
+    _CheckFrozen(self._frozen, 'Map is immutable')
     return self
 
   def __getitem__(self, key: _K) -> _V:

python/google/protobuf/internal/python_message.py

@@ -1606,7 +1606,12 @@ def _SetFrozen(self):
 
 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
 
 

python/google/protobuf/pyext/descriptor.cc

@@ -313,9 +313,7 @@ static PyObject* GetOrBuildMessageInDefaultPool(
     }
   }
 
-#if PROTOBUF_PY_FUTURE_FREEZE_OPTIONS
   cmsg->state = MESSAGE_FROZEN;
-#endif
 
   // Cache the result.
   {

python/google/protobuf/pyext/message.cc

@@ -29,6 +29,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
@@ -494,6 +495,26 @@ PyObject* SetMessageFrozenError() {
   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;
+}
+
 // Format an error message for unexpected types.
 // Always return with an exception set.
 void FormatTypeError(PyObject* arg, const char* expected_types) {
@@ -834,8 +855,10 @@ Message* AssureWritable(CMessage* self) {
     case MESSAGE_MUTABLE:
       return const_cast<Message*>(self->message);
     case MESSAGE_FROZEN:
-      SetMessageFrozenError();
-      return nullptr;
+      if (CheckFrozen(self, "Message is immutable.") < 0) {
+        return nullptr;
+      }
+      return const_cast<Message*>(self->message);
     case MESSAGE_MUTABLE_DEFAULT:
       break;
   }
@@ -1025,8 +1048,7 @@ int DeleteRepeatedField(CMessage* self, const FieldDescriptor* field_descriptor,
 int CheckRepeatedFieldDeletion(CMessage* parent,
                                const FieldDescriptor* field_descriptor,
                                PyObject* slice) {
-  if (parent->state == python::MESSAGE_FROZEN) {
-    SetMessageFrozenError();
+  if (CheckFrozen(parent, "Message is immutable.") < 0) {
     return -1;
   }
 

python/google/protobuf/pyext/message.h

@@ -360,6 +360,12 @@ PyObject* SetFrozenError(const char* msg);
 // Sets a Python FrozenInstanceError with the default error message for messages
 // type and returns nullptr.
 PyObject* SetMessageFrozenError();
+// 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,
                         PyObject* py_message_factory);

python/google/protobuf/pyext/repeated_composite_container.cc

@@ -16,6 +16,7 @@
 #include "google/protobuf/dynamic_message.h"
 #include "google/protobuf/message.h"
 #include "google/protobuf/reflection.h"
+#include "google/protobuf/breaking_changes.h"
 #include "google/protobuf/pyext/descriptor.h"
 #include "google/protobuf/pyext/descriptor_pool.h"
 #include "google/protobuf/pyext/message.h"
@@ -28,9 +29,6 @@ namespace python {
 
 namespace repeated_composite_container {
 
-static PyObject* SetContainerFrozenError() {
-  return SetFrozenError("Container is immutable");
-}
 
 // ---------------------------------------------------------------------
 // len()
@@ -285,8 +283,8 @@ static PyObject* Remove(PyObject* pself, PyObject* value) {
   RepeatedCompositeContainer* self =
       reinterpret_cast<RepeatedCompositeContainer*>(pself);
 
-  if (self->parent->state == python::MESSAGE_FROZEN) {
-    return SetContainerFrozenError();
+  if (CheckFrozen(self->parent, "Container is immutable") < 0) {
+    return nullptr;
   }
 
   Py_ssize_t len = Length(reinterpret_cast<PyObject*>(self));
@@ -406,8 +404,8 @@ static PyObject* Sort(PyObject* pself, PyObject* args, PyObject* kwds) {
   RepeatedCompositeContainer* self =
       reinterpret_cast<RepeatedCompositeContainer*>(pself);
 
-  if (self->parent->state == python::MESSAGE_FROZEN) {
-    return SetContainerFrozenError();
+  if (CheckFrozen(self->parent, "Container is immutable") < 0) {
+    return nullptr;
   }
 
   // Support the old sort_function argument for backwards
@@ -455,8 +453,8 @@ static PyObject* Reverse(PyObject* pself) {
   RepeatedCompositeContainer* self =
       reinterpret_cast<RepeatedCompositeContainer*>(pself);
 
-  if (self->parent->state == python::MESSAGE_FROZEN) {
-    return SetContainerFrozenError();
+  if (CheckFrozen(self->parent, "Container is immutable") < 0) {
+    return nullptr;
   }
 
   // TODO: b/517235198 - Reify even for empty sequences.
@@ -499,8 +497,8 @@ static PyObject* Pop(PyObject* pself, PyObject* args) {
   RepeatedCompositeContainer* self =
       reinterpret_cast<RepeatedCompositeContainer*>(pself);
 
-  if (self->parent->state == python::MESSAGE_FROZEN) {
-    return SetContainerFrozenError();
+  if (CheckFrozen(self->parent, "Container is immutable") < 0) {
+    return nullptr;
   }
 
   Py_ssize_t index = -1;
@@ -602,7 +600,7 @@ PyTypeObject RepeatedCompositeContainer_Type = {
 #if PY_VERSION_HEX >= 0x03080000
     0,  //  tp_vectorcall_offset
 #else
-    nullptr,             //  tp_print
+    nullptr,  //  tp_print
 #endif
     nullptr,                                   //  tp_getattr
     nullptr,                                   //  tp_setattr

python/google/protobuf/pyext/repeated_scalar_container.cc

@@ -26,6 +26,7 @@
 #include "google/protobuf/descriptor.h"
 #include "google/protobuf/message.h"
 #include "google/protobuf/reflection.h"
+#include "google/protobuf/breaking_changes.h"
 #include "google/protobuf/pyext/message.h"
 #include "google/protobuf/pyext/safe_numerics.h"
 #include "google/protobuf/pyext/scoped_pyobject_ptr.h"
@@ -526,9 +527,6 @@ bool CallWithSpan(const Message* message,
 
 namespace repeated_scalar_container {
 
-static PyObject* SetContainerFrozenError() {
-  return SetFrozenError("Container is immutable");
-}
 
 static int InternalAssignRepeatedField(RepeatedScalarContainer* self,
                                        PyObject* list) {
@@ -1057,8 +1055,8 @@ static PyObject* Remove(PyObject* pself, PyObject* value) {
 
   // Even if the value doesn't exist in the container, raise immutability error
   // prior to value error if applicable.
-  if (self->parent->state == python::MESSAGE_FROZEN) {
-    return SetContainerFrozenError();
+  if (CheckFrozen(self->parent, "Container is immutable") < 0) {
+    return nullptr;
   }
 
   Py_ssize_t match_index = -1;
@@ -1306,8 +1304,8 @@ static PyObject* Sort(PyObject* pself, PyObject* args, PyObject* kwds) {
   RepeatedScalarContainer* self =
       reinterpret_cast<RepeatedScalarContainer*>(pself);
 
-  if (self->parent->state == python::MESSAGE_FROZEN) {
-    return SetContainerFrozenError();
+  if (CheckFrozen(self->parent, "Container is immutable") < 0) {
+    return nullptr;
   }
 
   // Support the old sort_function argument for backwards
@@ -1353,8 +1351,8 @@ static PyObject* Reverse(PyObject* pself) {
   RepeatedScalarContainer* self =
       reinterpret_cast<RepeatedScalarContainer*>(pself);
 
-  if (self->parent->state == python::MESSAGE_FROZEN) {
-    return SetContainerFrozenError();
+  if (CheckFrozen(self->parent, "Container is immutable") < 0) {
+    return nullptr;
   }
 
   // TODO: b/517235198 - Reify even for empty sequences.
@@ -1405,8 +1403,8 @@ static PyObject* Pop(PyObject* pself, PyObject* args) {
 
   // Even if the value doesn't exist in the container, raise immutability error
   // prior to value error.
-  if (self->parent->state == python::MESSAGE_FROZEN) {
-    return SetContainerFrozenError();
+  if (CheckFrozen(self->parent, "Container is immutable") < 0) {
+    return nullptr;
   }
 
   Py_ssize_t index = -1;