On a side note, why does smppbox use boxc_id as the first parameter passed to dlr_add and dlr_find? Both functions take smsc_id as the first argument and boxc_id value is obtained from the sms struct.
2010/7/8 Rene Kluwen <rene.klu...@chimit.nl>: > Done. > Current revision is 17. > > -----Original Message----- > From: devel-boun...@kannel.org [mailto:devel-boun...@kannel.org] On Behalf Of > Victor Luchitz > Sent: donderdag 8 juli 2010 15:06 > To: devel@kannel.org > Subject: Re: smppbox code questions > > Yeah, you committed the proposed change to boxc->boxc_id in revision > 15. What I'm asking about is the suggestion and patch I posted here: > http://www.kannel.org/pipermail/devel/2010-July/003653.html > > 2010/7/8 Rene Kluwen <rene.klu...@chimit.nl>: >> It's already in the code. >> Current revision is 16. >> >> == Rene >> >> -----Original Message----- >> From: devel-boun...@kannel.org [mailto:devel-boun...@kannel.org] On Behalf >> Of Victor Luchitz >> Sent: donderdag 8 juli 2010 7:52 >> To: devel@kannel.org >> Subject: Re: smppbox code questions >> >> Any hope this will be reviewed and committed? >> >> I'm also working on a patch that adds TLV support to smppbox but I'd >> like to get this one included first. >> >> 2010/7/6 Victor Luchitz <vluch...@gmail.com>: >>> Yup, it's working fine now. Noticed there's another memleak though: >>> >>> another octstr_destroy(msgid); call is needed right after the: >>> /* we could not find a corresponding dlr; nothing to send */ >>> line. >>> >>> I'm also attaching another patch which allows transmission of custom >>> error codes in DLR's in the same manner as the message text bit. >>> >>> 2010/7/6 Rene Kluwen <rene.klu...@chimit.nl>: >>>> I have no way of testing this here. But since either way cannot harm I >>>> changed it. >>>> Current smppbox revision is now 15. >>>> Could you please check out and see if this fixes your problem? >>>> >>>> == Rene >>>> >>>> -----Original Message----- >>>> From: devel-boun...@kannel.org [mailto:devel-boun...@kannel.org] On Behalf >>>> Of Victor Luchitz >>>> Sent: dinsdag 6 juli 2010 14:53 >>>> To: devel@kannel.org >>>> Subject: Re: smppbox code questions >>>> >>>> 1) I think this assumption is incorrect. I have the routing set up >>>> this way in bearerbox: >>>> group = smsbox-route >>>> smsbox-id = vma >>>> smsc-id = HTTP >>>> >>>> So all messages on the 'HTTP' smsc get routed to smppbox. However, the >>>> custom HTTP protocol in the above layer does not use dlr_find to route >>>> messages to a specific box for two reasons: >>>> >>>> a) wrong smsc-id is used in the query (bearerbox doesn't know that >>>> smppbox overrides the smsc id with system-type) so dlr_find always >>>> fails >>>> b) dlr_find removes DLR from the table and then subsequently readds >>>> it, which is rather stressful on the DB for no sane reason >>>> >>>> What it does instead is simply setting the sms_type to report_mo, >>>> leaving box_id empty as in regular MO messages. >>>> >>>> 2010/7/6 Rene Kluwen <rene.klu...@chimit.nl>: >>>>> To start with the last thing: >>>>> >>>>> 2) You are right. It should use the msgid's in the dlr_url from the dlr >>>>> instance. I changed it. >>>>> >>>>> About 1): We assume msg->boxc_id and box->boxc_id are the same in this >>>>> case. Otherwise the message wouldn't have ended up there. >>>>> >>>>> == Rene >>>>> >>>>> >>>>> -----Original Message----- >>>>> From: devel-boun...@kannel.org [mailto:devel-boun...@kannel.org] On >>>>> Behalf Of Victor Luchitz >>>>> Sent: maandag 5 juli 2010 20:36 >>>>> To: devel@kannel.org >>>>> Subject: smppbox code questions >>>>> >>>>> Hello, >>>>> >>>>> I have a few questions for you regarding the handling of DLR's by >>>>> smppbox, which might also turn out to be bugs. >>>>> >>>>> 1) >>>>> In msg_to_pdu function there's a line which reads: >>>>> dlr = dlr_find(msg->sms.boxc_id, msgid, msg->sms.receiver, dlrtype); >>>>> >>>>> I think it's incorrect because when a DLR is stored by smppbox in >>>>> handle_pdu, the boxc_id it uses is that from smpp_logins file >>>>> (system_type). That in turn may cause dlr_find to always fail. So in >>>>> my opinion the correct dlr_find call is this: >>>>> dlr = dlr_find(box->boxc_id, msgid, msg->sms.receiver, dlrtype); >>>>> >>>>> 2) another thing I find not quite correct is the way smppbox splits >>>>> message ids for concatenated DLR's. Basically, what smppbox does is >>>>> this: >>>>> >>>>> parts = octstr_split(msg->sms.dlr_url, octstr_imm(";")); >>>>> msgid = gwlist_extract_first(parts); >>>>> ... >>>>> Then it loops through all elements of the 'parts' list and here is >>>>> where the potential problem lies. smppbox assumes that msgid for the >>>>> concatenated DLR is always equal to dlr_url which is not always true. >>>>> In fact, I think it's never true for concatenated DLR's stored by the >>>>> dlr_add call in handle_pdu. Also, for example, the 'msgid' and >>>>> 'dlrurls' columns in the storage table can have different maxiumum >>>>> lengths, allowing truncation of the msgid. Here's my proposed fix - >>>>> add the following bit of code to msg_to_pdu: >>>>> >>>>> gwlist_destroy(parts, octstr_destroy_item); >>>>> parts = octstr_split(dlr->sms.dlr_url, octstr_imm(";")); >>>>> gwlist_extract_first(parts); >>>>> >>>>> right above the following bit: >>>>> if (gwlist_len(parts) > 0) { >>>>> while ((msgid2 = gwlist_extract_first(parts)) != NULL) { >>>>> >>>>> >>>>> -- >>>>> Best regards, >>>>> Victor Luchitz >>>>> >>>>> >>>>> >>>>> >>>> >>>> >>>> >>>> -- >>>> Best regards, >>>> Victor Luchitz >>>> >>>> >>>> >>>> >>> >>> >>> >>> -- >>> Best regards, >>> Victor Luchitz >>> >> >> >> >> -- >> Best regards, >> Victor Luchitz >> >> >> >> > > > > -- > Best regards, > Victor Luchitz > > > > -- Best regards, Victor Luchitz
