Fixing pdu.generate() encoding - #132
Conversation
Sorry, something went wrong.
|
I think, all chr calls should be replaced with six.b(chr(...)), all isinstance checks are redundant. Good catch anyway! Perhaps add a test for that? |
Sorry, something went wrong.
| value = chr(0).encode() | ||
|
|
||
| setattr(self, field, field_value) | ||
| if isinstance(value, bytes): |
There was a problem hiding this comment.
| if isinstance(value, bytes): | |
| if isinstance(value, six.binary_type): |
hm, this check is required though
Sorry, something went wrong.
There was a problem hiding this comment.
Probably right :) You are trying to keep compatibility with 2.7
Sorry, something went wrong.
There was a problem hiding this comment.
for the test that was happening for me for every received pdu, as I was testing the generated PDU equals to the received.
pdu = parse_pdu(raw_pdu, client=self) gen_bin = pdu.generate()
And so, the exception occurs. Seems nobody had long not using that part, as I was fixing that over a year ago for myself and realized the bug is still there just yesterday after checking in a new version :)
Sorry, something went wrong.
There was a problem hiding this comment.
can you please add a new function or a new file with a new function to the tests/ directory?
Sorry, something went wrong.
Co-authored-by: Konstantin <kpp.live+github@gmail.com>
code-of-kpp
left a comment
There was a problem hiding this comment.
It also worth looking on line 174 and in general, why field_value is ever not bytes. Looks like the bug is actually happens because self contains a string in the field-attribute, while it should have bytes
Sorry, something went wrong.
Reviewers
Requested changes must be addressed to merge this pull request.
pdu.generate()
will lead to the following exception, so this is the proposed fix for it.
File "/opt/smpp_proxy_server/smpplib/pdu.py", line 136, in generate body = self.generate_params() File "/opt/smpp_proxy_server/smpplib/command.py", line 144, in generate_params value = self._generate_string(field) File "/opt/smpp_proxy_server/smpplib/command.py", line 180, in _generate_string value = field_value + chr(0) TypeError: can't concat str to bytes