Skip to content

Commit

Permalink
Fix bug that DiscardUnknownFields did not discard unknown fields of e…
Browse files Browse the repository at this point in the history
…xtensions in Py-upb and Php-upb and ruby-upb.

PiperOrigin-RevId: 713147042
  • Loading branch information
anandolee authored and copybara-github committed Jan 8, 2025
1 parent dce578d commit 98aab04
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 13 deletions.
1 change: 1 addition & 0 deletions .github/workflows/test_php.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
3 changes: 2 additions & 1 deletion php/ext/google/protobuf/message.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
17 changes: 15 additions & 2 deletions python/google/protobuf/internal/unknown_fields_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand Down
3 changes: 2 additions & 1 deletion python/message.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
3 changes: 2 additions & 1 deletion ruby/ext/google/protobuf_c/protobuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}

Expand Down
14 changes: 7 additions & 7 deletions upb/reflection/message.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)) {
Expand All @@ -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;
}
}
Expand All @@ -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;
}
}
Expand All @@ -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);
}
4 changes: 3 additions & 1 deletion upb/reflection/message.h
Original file line number Diff line number Diff line change
Expand Up @@ -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" */
Expand Down

0 comments on commit 98aab04

Please sign in to comment.