search

pmarti / python-messaging

Pure python SMS/MMS encoder/decoder
Other
225 stars 115 forks source link

Add support for decoding the MMS Delivery-Time header #18

Closed mikaelhg closed 13 years ago

mikaelhg commented 13 years ago

The library broke when decoding messages with this header, but not anymore.

andrewbird commented 13 years ago

Hi mikaelhg, Your patch looks clean and understandable, do you have a PDU generated by another application that contains the Delivery-Time header, if so I'd like to include it in the test suite?

Thanks, Andrew

mikaelhg commented 13 years ago

Alas, the test case which broke the library is a MMS message sent by a consumer, which I cannot share, and which I'm unable to reproduce.

andrewbird commented 13 years ago

No worries, thanks anyway. In the patch think we need to handle the case where the token is not 128 or 129. I'm not sure what the correct way to handle that is, either set a default value or raise, but currently the token_type variable would be undefined.

mikaelhg commented 13 years ago

As soon as I get my local database server restarted, we'll run the library against all of our archived MMS messages.

andrewbird commented 13 years ago

Thanks, that would be very useful

andrewbird commented 13 years ago

Almost there, could you change the exception to type 'wsp_pdu.DecodeError', then I'll apply? Thanks.

mikaelhg commented 13 years ago

Looks like most of the cases in which the library fails to parse a message, it's just invalid. Do you sanity-check the length of binary values before you reserve memory for them?

andrewbird commented 13 years ago

Not sure about sanity checking, Pablo's the best one to ask about that. In the invalid message case, how is that error seen / handled by your client?

mikaelhg commented 13 years ago

I suspect that the invalid or possibly maliciously crafted messages trick the library to reserve significantly more memory than is available on any reasonable host platform. I'll have to investigate further.

andrewbird commented 13 years ago

Ahh I see, python is supposed to release use on loss of reference, then garbage collect memory as required. We don't specifically allocate and deallocate memory, but I believe there are methods to specifically force the deallocation, though I've never had cause to do it myself. See this discussion http://stackoverflow.com/questions/1035489/python-garbage-collection

pmarti commented 13 years ago

You don't need to care of those details usually on Python. Changing it to xrange suffices. Good job!