Re: [PATCH] Fix bug in at2 7bit encoding with offset

Hi Riku,

thanks for the patch and great explanation! I was able to understand and 
reproduce this issue.
Patch commited to CVS.

Thanks,
Alex

Riku Palomäki schrieb:
> Hi,
> 
> On Thu, 11 Dec 2008 17:04:09 +0100
> Alexander Malysh <[email protected]> wrote:
> 
>> could you please explain in more detail why this patch is needed?
>> Unfortunately I'm unable to reproduce or understand the issue...
>> some examples maybe?
> 
> Since the PDU/7bit -bug seems not to be so obvious even after 5 months,
> some explaining with example could be in order.
> 
> Lets send a message with the lenght of 161 + n*8 to number 0123456789
> using the HTTP API: curl 'http://localhost:13013/cgi-bin/sendsms?
> username=*&password=*&to=0123456789&text=abcdefghijklmnopqrstuvwxyzABC
> DEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJK
> LMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRS'
> 
> 
> Current Kannel version does this according to the bearerbox.log:
> 
> # part 2
> AT+CMGS=33^M
> 0051000A8110325476980000A7170500030202028845E31199542E994DE7131A954E
> 
> # part 1
> AT+CMGS=153^M
> 0051000A8110325476980000A7A0050003020201C2E231B96C3EA3D3EA35BB
> ED7EC3E3F239BD6EBFE3F3FAA070482C1A8FC8A472C96C3A9FD0A8744AAD5A
> AFD8AC161693CD6835DB0D970B8BC7E4B2F98C4EABD7ECB6FB0D8FCBE7F4BA
> FD8ECFEB83C221B1683C2293CA25B3E97C42A3D229B56ABD62B35A584C36A3
> D56C375C2E2C1E93CBE6333AAD5EB3DBEE373C2E9FD3EBF63B3EAF0F0A87
> 
> 
> The first part (that is actually the big latter one, why does Kannel
> sends those messages in 'wrong' order?) is ok, but the second part is
> one octet too short. It should be 34 octets but it's only 33.
> 
> To be sure, lets decode the data manually (by blindly trusting the
> specs here: http://dreamfabric.com/sms/). 
> 
> # part 1
> 00 - use default SMSC
> 51 - SMS-SUBMIT: TP-UDHI (User data has header) |
>      TP-VPF (validity period is in realtive format == one octet) |
>      TP-MTI (SMS-SUBMIT)
> 00 - auto msg ref number
> 0A - length of the phone number (10 digits)
> 81 - phone address type (unknown type of number, telephone numbering
> plan) 1032547698 - the phone number (read by swapping the nibbles, 10
> 32 54.. -> 01 23 45..)
> 00 - short message type 0
> 00 - data is coded in 7 bits (important!)
> A7 - validity period is 24 hours
> A0 - length of the msg in septets == 160 7bit chars. That would be
>      exactly 140 (160*7/8) bytes.
> 050004020201...
>    - remaining 140 bytes of data, everything is ok. The
>      data has some kind of another header within the message, i think
>      it at least defines the timestamp to the message. However, that
>      part is in the data section in PDU, so we are not interested.
> 
> 
> # part 2
> # Most of the header is the same than in part 1:
> 00 51 00 0A 81 1032547698 00 00 A7
> 
> # Now the interesting part:
> 17 - the data length, 23 septets == 161 bits. That needs 21 bytes ...
> 0500030202028845E31199542E994DE7131A954E
>    - ... but the rest of the data is only 20 bytes (40 nibbles), so
>      we just lost one last bit of the data.
> 
> 
> So now to my patch. destRemain was calculated incorrectly, because it
> didn't take into account the offset value (in bits) in function
> Octstr *at2_encode7bituncompressed(Octstr *source, int offset):
> 
> -int destRemain = (int)ceil ((octstr_len(source) * 7.0) / 8.0);
> +int destRemain = (int)ceil ((octstr_len(source) * 7.0 + offset) / 8.0);
> 
> 
> With my patch, the PDU is generated one byte longer with the second
> part, and the data now has the last bit there correctly:
> 
> # part 2, with patch
> AT+CMGS=34^M
> 0051000A8140658790740000A7170500032C02028845E31199542E994DE7131A954E01
> 
> (the timestamp is different, that's why some bytes in the beginning
> are different to the old one)
> 
> 
> The patch has been in production use since July 2008, and without this
> Kannel makes Nokia 30 GSM Modem to crash. Kannel restart was required
> to recover from that state.
> 
>