Handle invalid NULL at the end of a PDU

[jerith]

We've seen at least one provider that sends a NULL byte after the short_message field in a deliver_sm PDU.

This is a violation of the SMPP spec, but we should probably handle it more gracefully than assuming it's the start of an optional parameter and exploding because it's too short to be a 16-bit integer.

[justinvdm]

Can't we do any kind of log thing here instead?

[jerith]

Not without a bunch of other work that's out of scope for this PR.

[jerith]

We have similar prints in other places in this codebase. We really should fix it, but it hasn't been a priority and I don't want it to block this unnecessarily while we have production issues.

[nofeet]

I submitted a pull request a few months ago to replace the print statements with logging: #3

It sounds like you're working on some other issues, but let me know later if you're interested in setting up logging and need help.

[jerith]

I saw that, thanks. Unfortunately, the logging situation is a little more complicate than it appears at first glance -- we're using Twisted's logging in the code that calls this, and the interactions with Python's logging require some careful thought.

Also, this project is something that we generally prefer not to touch unless it's absolutely necessary. It currently (mostly) works for us in production and the effort and risk required to clean it up properly are fairly high for the value we'd get from doing it.

Those are embarrassingly bad reasons for basically ignoring your contributions, for which I apologise, but it's the situation we're currently in. :-/

I do plan to look at the logging stuff again when I get some time. Hopefully in the next few weeks.

[nofeet]

I understand. Thanks again for all your work on this package.

[justinvdm]

👍 though I probably don't know enough about SMPP things to review this properly.