From 98aab04c0bf846ad6b4af339d9dbc5cf77d6c03c Mon Sep 17 00:00:00 2001 From: Jie Luo Date: Tue, 7 Jan 2025 21:28:04 -0800 Subject: [PATCH] Fix bug that DiscardUnknownFields did not discard unknown fields of extensions in Py-upb and Php-upb and ruby-upb. PiperOrigin-RevId: 713147042 --- .github/workflows/test_php.yml | 1 + php/ext/google/protobuf/message.c | 3 ++- .../protobuf/internal/unknown_fields_test.py | 17 +++++++++++++++-- python/message.c | 3 ++- ruby/ext/google/protobuf_c/protobuf.c | 3 ++- upb/reflection/message.c | 14 +++++++------- upb/reflection/message.h | 4 +++- 7 files changed, 32 insertions(+), 13 deletions(-) diff --git a/.github/workflows/test_php.yml b/.github/workflows/test_php.yml index dcbd93818f1e1..11732310d6054 100644 --- a/.github/workflows/test_php.yml +++ b/.github/workflows/test_php.yml @@ -79,6 +79,7 @@ jobs: set -ex COMPOSER_HOME=/workspace/composer-cache BAZEL_FLAGS=${{ env.BAZEL_FLAGS }} + ./regenerate_stale_files.sh ${{ env.BAZEL_FLAGS }} pushd /workspace/php composer update ${{ matrix.command }} diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index 38c730dfa6faf..c1ada126d0afd 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -583,7 +583,8 @@ PHP_METHOD(Message, __construct) { */ PHP_METHOD(Message, discardUnknownFields) { Message* intern = (Message*)Z_OBJ_P(getThis()); - upb_Message_DiscardUnknown(intern->msg, intern->desc->msgdef, 64); + upb_Message_DiscardUnknown(intern->msg, intern->desc->msgdef, + DescriptorPool_GetSymbolTable(), 64); } /** diff --git a/python/google/protobuf/internal/unknown_fields_test.py b/python/google/protobuf/internal/unknown_fields_test.py index 33ad766537994..7494bfd3b90d9 100755 --- a/python/google/protobuf/internal/unknown_fields_test.py +++ b/python/google/protobuf/internal/unknown_fields_test.py @@ -13,6 +13,9 @@ import sys import unittest +from google.protobuf import descriptor +from google.protobuf import text_format +from google.protobuf import unknown_fields from google.protobuf.internal import api_implementation from google.protobuf.internal import encoder from google.protobuf.internal import message_set_extensions_pb2 @@ -21,8 +24,7 @@ from google.protobuf.internal import testing_refleaks from google.protobuf.internal import type_checkers from google.protobuf.internal import wire_format -from google.protobuf import descriptor -from google.protobuf import unknown_fields + from google.protobuf import map_unittest_pb2 from google.protobuf import unittest_mset_pb2 from google.protobuf import unittest_pb2 @@ -142,6 +144,17 @@ def testDiscardUnknownFields(self): b'', msg.map_int32_all_types[1].optional_nested_message.SerializeToString()) + def testUnknownFieldsInExtension(self): + msg = message_set_extensions_pb2.TestMessageSet() + ext3 = message_set_extensions_pb2.message_set_extension3 + sub_message = unittest_pb2.TestAllTypes() + sub_message.optional_string = 'discard' + msg.Extensions[ext3].ParseFromString(sub_message.SerializeToString()) + msg.DiscardUnknownFields() + self.assertNotIn( + 'discard', text_format.MessageToString(msg, print_unknown_fields=True) + ) + @testing_refleaks.TestCase class UnknownFieldsAccessorsTest(unittest.TestCase): diff --git a/python/message.c b/python/message.c index cece4ac2adc92..691ddcf5ac43a 100644 --- a/python/message.c +++ b/python/message.c @@ -1484,7 +1484,8 @@ static PyObject* PyUpb_Message_DiscardUnknownFields(PyUpb_Message* self, PyObject* arg) { PyUpb_Message_EnsureReified(self); const upb_MessageDef* msgdef = _PyUpb_Message_GetMsgdef(self); - upb_Message_DiscardUnknown(self->ptr.msg, msgdef, 64); + const upb_DefPool* ext_pool = upb_FileDef_Pool(upb_MessageDef_File(msgdef)); + upb_Message_DiscardUnknown(self->ptr.msg, msgdef, ext_pool, 64); Py_RETURN_NONE; } diff --git a/ruby/ext/google/protobuf_c/protobuf.c b/ruby/ext/google/protobuf_c/protobuf.c index e84395bf882c5..4df4886a6043e 100644 --- a/ruby/ext/google/protobuf_c/protobuf.c +++ b/ruby/ext/google/protobuf_c/protobuf.c @@ -286,7 +286,8 @@ VALUE ObjectCache_Get(const void *key) { static VALUE Google_Protobuf_discard_unknown(VALUE self, VALUE msg_rb) { const upb_MessageDef *m; upb_Message *msg = Message_GetMutable(msg_rb, &m); - if (!upb_Message_DiscardUnknown(msg, m, 128)) { + const upb_DefPool* ext_pool = upb_FileDef_Pool(upb_MessageDef_File(m)); + if (!upb_Message_DiscardUnknown(msg, m, ext_pool, 128)) { rb_raise(rb_eRuntimeError, "Messages nested too deeply."); } diff --git a/upb/reflection/message.c b/upb/reflection/message.c index 93300d6b862f4..5a29fabe4c249 100644 --- a/upb/reflection/message.c +++ b/upb/reflection/message.c @@ -192,7 +192,7 @@ bool upb_Message_Next(const upb_Message* msg, const upb_MessageDef* m, } bool _upb_Message_DiscardUnknown(upb_Message* msg, const upb_MessageDef* m, - int depth) { + const upb_DefPool* ext_pool, int depth) { UPB_ASSERT(!upb_Message_IsFrozen(msg)); size_t iter = kUpb_Message_Begin; const upb_FieldDef* f; @@ -203,7 +203,7 @@ bool _upb_Message_DiscardUnknown(upb_Message* msg, const upb_MessageDef* m, _upb_Message_DiscardUnknown_shallow(msg); - while (upb_Message_Next(msg, m, NULL /*ext_pool*/, &f, &val, &iter)) { + while (upb_Message_Next(msg, m, ext_pool, &f, &val, &iter)) { const upb_MessageDef* subm = upb_FieldDef_MessageSubDef(f); if (!subm) continue; if (upb_FieldDef_IsMap(f)) { @@ -217,7 +217,7 @@ bool _upb_Message_DiscardUnknown(upb_Message* msg, const upb_MessageDef* m, upb_MessageValue map_key, map_val; while (upb_Map_Next(map, &map_key, &map_val, &iter)) { if (!_upb_Message_DiscardUnknown((upb_Message*)map_val.msg_val, val_m, - depth)) { + ext_pool, depth)) { ret = false; } } @@ -227,13 +227,13 @@ bool _upb_Message_DiscardUnknown(upb_Message* msg, const upb_MessageDef* m, for (i = 0; i < n; i++) { upb_MessageValue elem = upb_Array_Get(arr, i); if (!_upb_Message_DiscardUnknown((upb_Message*)elem.msg_val, subm, - depth)) { + ext_pool, depth)) { ret = false; } } } else { if (!_upb_Message_DiscardUnknown((upb_Message*)val.msg_val, subm, - depth)) { + ext_pool, depth)) { ret = false; } } @@ -243,6 +243,6 @@ bool _upb_Message_DiscardUnknown(upb_Message* msg, const upb_MessageDef* m, } bool upb_Message_DiscardUnknown(upb_Message* msg, const upb_MessageDef* m, - int maxdepth) { - return _upb_Message_DiscardUnknown(msg, m, maxdepth); + const upb_DefPool* ext_pool, int maxdepth) { + return _upb_Message_DiscardUnknown(msg, m, ext_pool, maxdepth); } diff --git a/upb/reflection/message.h b/upb/reflection/message.h index fbe34f4d04217..53acc399535f9 100644 --- a/upb/reflection/message.h +++ b/upb/reflection/message.h @@ -78,7 +78,9 @@ UPB_API bool upb_Message_Next(const upb_Message* msg, const upb_MessageDef* m, // Clears all unknown field data from this message and all submessages. UPB_API bool upb_Message_DiscardUnknown(upb_Message* msg, - const upb_MessageDef* m, int maxdepth); + const upb_MessageDef* m, + const upb_DefPool* ext_pool, + int maxdepth); #ifdef __cplusplus } /* extern "C" */