diff --git a/python/descriptor.c b/python/descriptor.c index c60c8f50ee..815157c7a9 100644 --- a/python/descriptor.c +++ b/python/descriptor.c @@ -165,7 +165,18 @@ oom: static PyObject* PyUpb_DescriptorBase_CopyToProto(PyObject* _self, PyUpb_ToProto_Func* func, const upb_MiniTable* layout, + const char* expected_type, PyObject* py_proto) { + if (!PyUpb_CMessage_Verify(py_proto)) return NULL; + const upb_MessageDef* m = PyUpb_CMessage_GetMsgdef(py_proto); + const char* type = upb_MessageDef_FullName(m); + if (strcmp(type, expected_type) != 0) { + PyErr_Format( + PyExc_TypeError, + "CopyToProto: message is of incorrect type '%s' (expected '%s'", type, + expected_type); + return NULL; + } PyObject* serialized = PyUpb_DescriptorBase_GetSerializedProto(_self, func, layout); if (!serialized) return NULL; @@ -319,7 +330,8 @@ static PyObject* PyUpb_Descriptor_CopyToProto(PyObject* _self, PyObject* py_proto) { return PyUpb_DescriptorBase_CopyToProto( _self, (PyUpb_ToProto_Func*)&upb_MessageDef_ToProto, - &google_protobuf_DescriptorProto_msginit, py_proto); + &google_protobuf_DescriptorProto_msginit, + "google.protobuf.DescriptorProto", py_proto); } static PyObject* PyUpb_Descriptor_EnumValueName(PyObject* _self, @@ -728,7 +740,8 @@ static PyObject* PyUpb_EnumDescriptor_CopyToProto(PyObject* _self, PyObject* py_proto) { return PyUpb_DescriptorBase_CopyToProto( _self, (PyUpb_ToProto_Func*)&upb_EnumDef_ToProto, - &google_protobuf_EnumDescriptorProto_msginit, py_proto); + &google_protobuf_EnumDescriptorProto_msginit, + "google.protobuf.EnumDescriptorProto", py_proto); } static PyGetSetDef PyUpb_EnumDescriptor_Getters[] = { @@ -1248,7 +1261,8 @@ static PyObject* PyUpb_FileDescriptor_CopyToProto(PyObject* _self, PyObject* py_proto) { return PyUpb_DescriptorBase_CopyToProto( _self, (PyUpb_ToProto_Func*)&upb_FileDef_ToProto, - &google_protobuf_FileDescriptorProto_msginit, py_proto); + &google_protobuf_FileDescriptorProto_msginit, + "google.protobuf.FileDescriptorProto", py_proto); } static PyGetSetDef PyUpb_FileDescriptor_Getters[] = { @@ -1354,7 +1368,8 @@ static PyObject* PyUpb_MethodDescriptor_CopyToProto(PyObject* _self, PyObject* py_proto) { return PyUpb_DescriptorBase_CopyToProto( _self, (PyUpb_ToProto_Func*)&upb_MethodDef_ToProto, - &google_protobuf_MethodDescriptorProto_msginit, py_proto); + &google_protobuf_MethodDescriptorProto_msginit, + "google.protobuf.MethodDescriptorProto", py_proto); } static PyGetSetDef PyUpb_MethodDescriptor_Getters[] = { @@ -1559,7 +1574,8 @@ static PyObject* PyUpb_ServiceDescriptor_CopyToProto(PyObject* _self, PyObject* py_proto) { return PyUpb_DescriptorBase_CopyToProto( _self, (PyUpb_ToProto_Func*)&upb_ServiceDef_ToProto, - &google_protobuf_ServiceDescriptorProto_msginit, py_proto); + &google_protobuf_ServiceDescriptorProto_msginit, + "google.protobuf.ServiceDescriptorProto", py_proto); } static PyObject* PyUpb_ServiceDescriptor_FindMethodByName(PyObject* _self, diff --git a/python/descriptor_pool.c b/python/descriptor_pool.c index 7428452760..06011d702e 100644 --- a/python/descriptor_pool.c +++ b/python/descriptor_pool.c @@ -249,7 +249,7 @@ done: static PyObject* PyUpb_DescriptorPool_DoAdd(PyObject* _self, PyObject* file_desc) { - if (!PyUpb_CMessage_Check(file_desc)) return NULL; + if (!PyUpb_CMessage_Verify(file_desc)) return NULL; const upb_MessageDef* m = PyUpb_CMessage_GetMsgdef(file_desc); const char* file_proto_name = "google.protobuf.FileDescriptorProto"; if (strcmp(upb_MessageDef_FullName(m), file_proto_name) != 0) { diff --git a/python/message.c b/python/message.c index 9bede8d42b..11b3a0828e 100644 --- a/python/message.c +++ b/python/message.c @@ -232,7 +232,7 @@ bool PyUpb_CMessage_TryCheck(PyObject* self) { return Py_TYPE(type) == state->message_meta_type; } -bool PyUpb_CMessage_Check(PyObject* self) { +bool PyUpb_CMessage_Verify(PyObject* self) { if (!PyUpb_CMessage_TryCheck(self)) { PyErr_Format(PyExc_TypeError, "Expected a message object, but got %R.", self); @@ -1395,7 +1395,7 @@ PyObject* PyUpb_CMessage_SerializeInternal(PyObject* _self, PyObject* args, PyObject* kwargs, bool check_required) { PyUpb_CMessage* self = (void*)_self; - if (!PyUpb_CMessage_Check((PyObject*)self)) return NULL; + if (!PyUpb_CMessage_Verify((PyObject*)self)) return NULL; static const char* kwlist[] = {"deterministic", NULL}; int deterministic = 0; if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|p", (char**)(kwlist), diff --git a/python/message.h b/python/message.h index 73bac46910..46155ac8af 100644 --- a/python/message.h +++ b/python/message.h @@ -50,7 +50,7 @@ PyObject* PyUpb_CMessage_Get(upb_Message* u_msg, const upb_MessageDef* m, // Verifies that a Python object is a message. Sets a TypeError exception and // returns false on failure. -bool PyUpb_CMessage_Check(PyObject* self); +bool PyUpb_CMessage_Verify(PyObject* self); // Gets the upb_Message* for this message object if the message is reified. // Otherwise returns NULL. diff --git a/python/pb_unit_tests/descriptor_test_wrapper.py b/python/pb_unit_tests/descriptor_test_wrapper.py index 92485e5b0b..758d86e974 100644 --- a/python/pb_unit_tests/descriptor_test_wrapper.py +++ b/python/pb_unit_tests/descriptor_test_wrapper.py @@ -26,10 +26,18 @@ from google.protobuf.internal import descriptor_test import unittest -descriptor_test.DescriptorCopyToProtoTest.testCopyToProto_TypeError.__unittest_expecting_failure__ = True +# Our behavior here matches pure-Python, which does not allow +# foo.enum_values_by_name.get([]). We reject it because we return a true +# dict (like pure Python), which does not allow hashing by a list. descriptor_test.GeneratedDescriptorTest.testDescriptor.__unittest_expecting_failure__ = True + +# These fail because they attempt to add fields with conflicting JSON names. +# We don't want to support this going forward. descriptor_test.MakeDescriptorTest.testCamelcaseName.__unittest_expecting_failure__ = True descriptor_test.MakeDescriptorTest.testJsonName.__unittest_expecting_failure__ = True + +# We pass this test, but the error message is slightly different. +# Our error message is better. descriptor_test.NewDescriptorTest.testImmutableCppDescriptor.__unittest_expecting_failure__ = True if __name__ == '__main__': diff --git a/python/repeated.c b/python/repeated.c index 3520b43a17..16407a508f 100644 --- a/python/repeated.c +++ b/python/repeated.c @@ -598,7 +598,7 @@ PyObject* PyUpb_RepeatedCompositeContainer_Add(PyObject* _self, PyObject* args, static PyObject* PyUpb_RepeatedCompositeContainer_Append(PyObject* _self, PyObject* value) { - if (!PyUpb_CMessage_Check(value)) return NULL; + if (!PyUpb_CMessage_Verify(value)) return NULL; PyObject* py_msg = PyUpb_RepeatedCompositeContainer_AppendNew(_self); if (!py_msg) return NULL; PyObject* none = PyUpb_CMessage_MergeFrom(py_msg, value);