Re: smppbox code questions

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 <[email protected]>:
> 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: [email protected] [mailto:[email protected]] On Behalf Of Victor Luchitz
> Sent: dinsdag 6 juli 2010 14:53
> To: [email protected]
> 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 <[email protected]>:
>> 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: [email protected] [mailto:[email protected]] On Behalf Of Victor Luchitz
>> Sent: maandag 5 juli 2010 20:36
>> To: [email protected]
>> 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

Index: smppbox.c
===================================================================
--- smppbox.c	(revision 15)
+++ smppbox.c	(working copy)
@@ -456,7 +456,7 @@
     List *pdulist = gwlist_create(), *parts;
     int validity, dlrtype, catenate;
     Msg *dlr;
-    char *text;
+    char *text, *tmps, err[4] = { '0', '0', '0', '\0' };
     Octstr *msgid, *msgid2, *dlr_status, *dlvrd;
     /* split variables */
     List *list;
@@ -604,10 +604,20 @@
 		dlr_status = octstr_imm("UNDELIV");
 		break;
 	}
+
 	text = octstr_get_cstr(msg->sms.msgdata);
-	if (strstr(text, "text:") != NULL) {
-		text = strstr(text, "text:") + (5 * sizeof(char));
+
+	tmps = strstr(text, "err:");
+	if (tmps != NULL) {
+		snprintf(err, sizeof(err), "%03.3s", tmps + (4 * sizeof(char)));
+		tmps = strstr(tmps, " ");
+		text = tmps ? tmps + (1 * sizeof(char)) : "";
 	}
+	tmps = strstr(text, "text:");
+	if (tmps != NULL) {
+		text = tmps + (5 * sizeof(char));
+	}
+
 	/* the msgids are in dlr->dlr_url as reported by Victor Luchitz */
 	gwlist_destroy(parts, octstr_destroy_item);
 	parts = octstr_split(dlr->sms.dlr_url, octstr_imm(";"));
@@ -626,14 +636,14 @@
 			pdu2->u.deliver_sm.source_addr = octstr_duplicate(pdu->u.deliver_sm.source_addr);
 			pdu2->u.deliver_sm.destination_addr = octstr_duplicate(pdu->u.deliver_sm.destination_addr);
 			pdu2->u.deliver_sm.service_type = octstr_duplicate(pdu->u.deliver_sm.service_type);
-			pdu2->u.deliver_sm.short_message = octstr_format("id:%S sub:001 dlvrd:%S submit date:%ld done date:%ld stat:%S err:000 text:%12s", msgid2, dlvrd, msg->sms.time, dlr->sms.time, dlr_status, text);
+			pdu2->u.deliver_sm.short_message = octstr_format("id:%S sub:001 dlvrd:%S submit date:%ld done date:%ld stat:%S err:%s text:%12s", msgid2, dlvrd, msg->sms.time, dlr->sms.time, dlr_status, err, text);
 			octstr_destroy(msgid2);
 			gwlist_append(pdulist, pdu2);
 		}
         	smpp_pdu_destroy(pdu);
 	}
 	else {
-		pdu->u.deliver_sm.short_message = octstr_format("id:%S sub:001 dlvrd:%S submit date:%ld done date:%ld stat:%S err:000 text:%12s", msgid, dlvrd, msg->sms.time, dlr->sms.time, dlr_status, text);
+		pdu->u.deliver_sm.short_message = octstr_format("id:%S sub:001 dlvrd:%S submit date:%ld done date:%ld stat:%S err:%s text:%12s", msgid, dlvrd, msg->sms.time, dlr->sms.time, dlr_status, err, text);
 		gwlist_append(pdulist, pdu);
 	}
 	octstr_destroy(msgid);