Skip to content

Commit

Permalink
Make Python/C++ reject unmatched end-group tag.
Browse files Browse the repository at this point in the history
This brings it into conformance with our spec and other languages.

PiperOrigin-RevId: 702914786
  • Loading branch information
mkruskal-google authored and copybara-github committed Dec 5, 2024
1 parent ab3adc2 commit 482752a
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 17 deletions.
4 changes: 0 additions & 4 deletions conformance/failure_list_python_cpp.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,3 @@
# TODO: insert links to corresponding bugs tracking the issue.
# Should we use GitHub issues or the Google-internal bug tracker?
Required.*.JsonInput.Int32FieldQuotedExponentialValue.* # Failed to parse input or produce output.
Required.*.ProtobufInput.UnmatchedEndGroup # Should have failed to parse, but didn't.
Required.*.ProtobufInput.UnmatchedEndGroupUnknown # Should have failed to parse, but didn't.
Required.*.ProtobufInput.UnmatchedEndGroupWithData # Should have failed to parse, but didn't.
Required.*.ProtobufInput.UnmatchedEndGroupWrongType # Should have failed to parse, but didn't.
14 changes: 5 additions & 9 deletions python/google/protobuf/internal/message_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,12 @@ def testParseErrors(self, message_module):
msg = message_module.TestAllTypes()
self.assertRaises(TypeError, msg.FromString, 0)
self.assertRaises(Exception, msg.FromString, '0')
# TODO: Fix cpp extension to raise error instead of warning.
# b/27494216
end_tag = encoder.TagBytes(1, 4)
if (api_implementation.Type() == 'python' or
api_implementation.Type() == 'upb'):
with self.assertRaises(message.DecodeError) as context:
msg.FromString(end_tag)
if api_implementation.Type() == 'python':
# Only pure-Python has an error message this specific.
self.assertEqual('Unexpected end-group tag.', str(context.exception))
with self.assertRaises(message.DecodeError) as context:
msg.FromString(end_tag)
if api_implementation.Type() != 'upb':
# upb raises a less specific exception.
self.assertRegex(str(context.exception), 'Unexpected end-group tag.*')

# Field number 0 is illegal.
self.assertRaises(message.DecodeError, msg.FromString, b'\3\4')
Expand Down
7 changes: 3 additions & 4 deletions python/google/protobuf/pyext/message.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1900,10 +1900,9 @@ static PyObject* MergeFromString(CMessage* self, PyObject* arg) {
// ctx has an explicit limit set (length of string_view), so we have to
// check we ended at that limit.
if (!ctx.EndedAtLimit()) {
// TODO: Raise error and return NULL instead.
// b/27494216
PyErr_Warn(nullptr, "Unexpected end-group tag: Not all data was converted");
return PyLong_FromLong(data.len - ctx.BytesUntilLimit(ptr));
PyErr_Format(DecodeError_class,
"Unexpected end-group tag: Not all data was converted");
return nullptr;
}
return PyLong_FromLong(data.len);
}
Expand Down

0 comments on commit 482752a

Please sign in to comment.