Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MergeFromString() does not work with any buffer protocol type. #18651

Open
pettni opened this issue Oct 8, 2024 · 0 comments
Open

MergeFromString() does not work with any buffer protocol type. #18651

pettni opened this issue Oct 8, 2024 · 0 comments
Assignees

Comments

@pettni
Copy link

pettni commented Oct 8, 2024

What version of protobuf and what language are you using?
Python protobuf 5.28.2. I believe this affects all versions after the upb migration.

What operating system (Linux, Windows, ...) and version?
Linux (Amazon Linux 2)

What runtime / compiler are you using (e.g., python version or gcc version)
Python 3.12

What did you do?

python -c 'from google.protobuf import timestamp_pb2; from array import array; timestamp_pb2.Timestamp.FromString(array("l"))'

What did you expect to see
The program should run without error.

What did you see instead?

Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: expected bytes, array.array found

Workaround
Wrap the argument to MergeFromString in a memoryview().


The documentation says that MergeFromString() should work on

Any object that allows us to call `memoryview(serialized)` to access a string of 
bytes using the buffer interface.

However, I had trouble with a custom type that implements the buffer interface. I reproduced it with an array which also implements the buffer interface in the example above.

From reading the source it appears that serialized needs to be one of {memoryview, bytearray, bytes}, but does not support arbitrary buffer interface types. I checked with version 3.19.2 (before upb) and did not have the same issue there.

Would it be a good idea to to instead use PyMemoryView_FromObject (with a NULL check) instead? That seems closer to the old Python-native logic and the documentation.

Note: this is similar to #16691 (issue #15911) that added support for bytearray's, but that is still not as general as any buffer protocol type.

@pettni pettni added the untriaged auto added to all issues by default when created. label Oct 8, 2024
@JasonLunn JasonLunn added python upb and removed untriaged auto added to all issues by default when created. labels Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants