[PATCH] Re: EMI Driver Bug

Hi All,

attached patch should solve the bug , make small cleanup and add additional sanity check...
please look in it and vote ...

Am Montag, 3. Februar 2003 19:08 schrieb Michael Mulcahy:
> > Michael you are right...
> > should we replace: dlrmsg->sms.msgdata =
> >  octstr_duplicate(emimsg->fields[E50_AMSG]); with:
> > dlrmsg->sms.msgdata =
> >  octstr_create("");
> > ?
>
> I think so, unless other people working with EMI have a better idea!
>
> ANAM Wireless Internet Solutions
> http://www.anam.com mailto:[email protected]
> +353 1 284 7555
> Castle Yard, Saint Patrick's Road, Dalkey, County Dublin, Ireland
>
> > -----Original Message-----
> > From: [email protected]
> > [mailto:[email protected]]On Behalf Of Alexander Malysh (by
> > way of Alexander Malysh <[email protected]>)
> > Sent: 03 February 2003 15:57
> > To: [email protected]
> > Subject: Re: EMI Driver Bug
> >
> >
> > Hi All,
> >
> > Michael you are right...
> > should we replace: dlrmsg->sms.msgdata =
> >  octstr_duplicate(emimsg->fields[E50_AMSG]); with:
> > dlrmsg->sms.msgdata =
> >  octstr_create("");
> > ?
> >
> > Am Montag, 3. Februar 2003 16:38 schrieb Michael Mulcahy:
> > > Hi All,
> > >
> > > more information for your understanding:
> > >
> > > When I submit a message with a dlr mask of 24, the
> >
> > bearerbox crashes,
> >
> > > Used following sendsms request:
> >
> > http://127.0.0.1:22022/cgi-bin/sendsms?username=testuser&passw
> > ord=password&
> >
> > >t o=12345676&from=987654321&text=Test Message (A) is being
> >
> > sent!&smsc=test&dlrmask=24&dlrurl=http://synge/test.asp?mask=%
> > 25d%26smsc_re
> >
> > >p ly=%25A%26receiver=%25p%26sender=%25P
> > >
> > > Here is a stack trace of the problem:
> > >
> > > seems_valid_real(Octstr * 0xdddddddd, const char *
> >
> > 0x10066ffc, long 289,
> >
> > > const char * 0x10066ff4)
> > > octstr_duplicate_real(Octstr * 0xdddddddd)
> > > emi2_handle_smscreq(smscconn * 0x00d706f0, Connection * 0x00d73420)
> > > emi2_send_loop(smscconn * 0x00d706f0, Connection * * 0x0188ff1c)
> > > emi2_sender(void * 0x00d706f0)
> > > new_thread(void * 0x00d727d0)
> > >
> > > dlrmsg->sms.msgdata = octstr_duplicate(emimsg->fields[E50_AMSG]);
> > > is the line in question within the emi2_handle_smscreq function.
> > >
> > > corresponding bearerbox trace:
> > >
> > > 2003-02-03 15:11:28 [22] DEBUG: boxc_receiver: sms received
> > > 2003-02-03 15:11:28 [8] DEBUG: EMI2[emi_smsc]: emi2 sending packet:
> >
> > <00/00138/O/51/12345676/20860000/////////////////3//5465737420
> > 4D65737361676
> >
> > > 5202841292030206973206265696E672073656E7421//////////020111///0F>
> > > 2003-02-03 15:11:28 [8] DEBUG: Adding DLR smsc=emi_smsc,
> > > ts=emi_smsc-0:45676, src=20860000, dst=12345676, mask=24,
> > > boxc=192.168.0.97:62982
> > > 2003-02-03 15:11:28 [8] DEBUG: EMI2[emi_smsc]: Got packet
> >
> > from the main
> >
> > > socket
> > > 2003-02-03 15:11:28 [8] DEBUG: EMI2[emi_smsc]: emi2 parsing packet:
> > > <00/00041/R/51/A//12345676:030203150828/D3>
> > > 2003-02-03 15:11:28 [8] DEBUG: Looking for DLR smsc=emi_smsc,
> > > ts=emi_smsc-0:45676, dst=12345676, type=8
> > > 2003-02-03 15:11:28 [8] DEBUG: created DLR message for URL
> >
> > <http://synge/test.asp?mask=%d&smsc_reply=%A&receiver=%p&sende
> > r=%P&message-
> >
> > >n umber=0>
> > >
> > > This occurs as the EMI driver is accessing a field
> >
> > (E50_AMSG) that is not
> >
> > > available in an EMI acknowledgement. It is surprising that
> >
> > no one has
> >
> > > experienced
> > > this problem in Kannel already with dlr_masks set to 24.
> > >
> > >
> > > ANAM Wireless Internet Solutions
> > > http://www.anam.com mailto:[email protected]
> > > +353 1 284 7555
> > > Castle Yard, Saint Patrick's Road, Dalkey, County Dublin, Ireland
> > >
> > >
> > > -----Original Message-----
> > > From: Andreas Fink [mailto:[email protected]]
> > > Sent: 03 February 2003 14:30
> > > To: [email protected]
> > > Subject: Re: EMI Driver Bug
> > >
> > >
> > >
> > > On Montag, Februar 3, 2003, at 03:22 Uhr, Michael Mulcahy wrote:
> > >
> > >
> > > Hi All,
> > >
> > > Comments Below:
> > >
> > >
> > > You're mixing up the SMSC response with the delivery report.
> > > When you send a message, you send a type 51 message. you will
> > > get a 50ACK back saying the SMSC has accepted the SMS.
> > >
> > >
> > > Is this a typo on your part? According to the EMI spec 4.0
> >
> > the response to
> >
> > > the submit short message operation 51 should contain an
> >
> > operation type of
> >
> > > 51 not 50.
> > >
> > >
> > > the ACK of all type 50 messages are the same. So this is of
> >
> > course a ACK
> >
> > > for 51.
> > >
> > >
> > >
> > >
> > > This is NOT the delivery report. The SMSC response is
> >
> > acknowledging that
> >
> > > the SMSC has accepted the message but it doesnt say that the message
> > > has been delivered to the handset.
> > >
> > >
> > > I think there is a misunderstanding of terminology here. I
> >
> > use the term
> >
> > > submission notification to mean that the message was
> >
> > submitted to the SMSC
> >
> > > not delivered to the handset.
> > >
> > >
> > > well this is not a delivery report then and I think the
> >
> > code snipped you
> >
> > > are showing is not executed in this case.
> > > note: in terms of DLR storage there are two things happening:
> > >
> > > when submission report is asked, it is adding a
> >
> > "temporarely" DLR record
> >
> > > with the sequence ID to the storage.
> > > when the submission is OK, then it adds a "final" DLR
> >
> > record with the full
> >
> > > data to the storage. This one is the one being searched
> >
> > when the message
> >
> > > finally gets delivered.
> > >
> > >
> > >
> > > The EMI2 driver does the following when a response to a
> >
> > submit operation is
> >
> > > received:
> > > Some code removed for brevity
> > >
> > > if (emimsg->ot == 51) {
> > > if (PRIVDATA(conn)->slots[emimsg->trn].dlr) {
> > > ...
> > > dlrmsg = dlr_find(octstr_get_cstr((conn->id ? conn->id :
> > > privdata->name)),
> > > octstr_get_cstr(ts), /* timestamp */
> > > octstr_get_cstr(origmsg->sms.receiver), /* destination */
> > > (octstr_get_char(emimsg->fields[0], 0) == 'A' ?
> > > DLR_SMSC_SUCCESS : DLR_SMSC_FAIL));
> > >
> > > octstr_destroy(ts);
> > > if (dlrmsg != NULL) {
> > > ...
> > >
> > > /*
> > > * Recode the msg structure with the given msgdata.
> > > * Note: the DLR URL is delivered in msg->sms.dlr_url
> > > already.
> > > */
> > > dlrmsg->sms.msgdata =
> > > octstr_duplicate(emimsg->fields[E50_AMSG]);
> > > octstr_hex_to_binary(dlrmsg->sms.msgdata);
> > > dlrmsg->sms.sms_type = report;
> > >
> > > This is the code for handling a response to a submit short message
> > > operation and checking if the user requested notification
> >
> > that the message
> >
> > > was submitted
> > > to the SMSC.
> > >
> > >
> > >
> > > I have to look into the code but it might be simply a
> >
> > non-fatal bug here.
> >
> > > So as per my original mail why does the driver try to reference the
> > > E50_AMSG field from the response of submitted message?
> > >
> > > I have checked CVS and it appears our emi2 module is up to
> >
> > date. I can
> >
> > > understand the driver doing this for a delivery report as
> >
> > the field exists
> >
> > > in a delivery
> > > report message but not for an acknowledgement to a
> >
> > submitted message. Does
> >
> > > your SMSC send
> > > acknowledgements that include the E50_AMSG field?
> > >
> > > We have being testing with an EMI emulator that claims to
> >
> > implement the EMI
> >
> > > standard, version 4.
> > >
> > >
> > > Well, while you are talking to an EMI emulator, does kannel
> >
> > fail in any
> >
> > > way? In other words, do you get a panic or such?
> > >
> > >
> > > Andreas Fink
> > > Global Networks Switzerland AG
> > >
> > > ------------------------------------------------------------------
> > > Tel: +41-61-6666333 Fax: +41-61-6666334 Mobile: +41-79-2457333
> > > Global Networks, Inc. Clarastrasse 3, 4058 Basel, Switzerland
> > > Web: http://www.global-networks.ch/  [email protected]
> > > ------------------------------------------------------------------
> > > Member of the GSM Association
> >
> > --
> > Best regards / Mit besten Grüßen aus Köln
> >
> > Dipl.-Ing.
> > Alexander Malysh
> > ___________________________________________
> >
> > Centrium GmbH
> > Ehrenstraße 2
> > 50672 Köln
> >
> > Fon: +49 (0221) 277 49 240
> > Fax: +49 (0221) 277 49 109
> >
> > email: [email protected]
> > web: www.centrium.de
> > msn: [email protected]

-- 
Best regards / Mit besten Grüßen aus Köln

Dipl.-Ing.
Alexander Malysh
___________________________________________

Centrium GmbH
Ehrenstraße 2
50672 Köln

Fon: +49 (0221) 277 49 240
Fax: +49 (0221) 277 49 109

email: [email protected]
web: www.centrium.de
msn: [email protected]

Index: gw/smsc/smsc_emi2.c
===================================================================
RCS file: /opt/cvsroot/kannel/gateway/gw/smsc/smsc_emi2.c,v
retrieving revision 1.1.1.1.2.23
diff -a -u -r1.1.1.1.2.23 smsc_emi2.c
--- gw/smsc/smsc_emi2.c	12 Dec 2002 18:02:40 -0000	1.1.1.1.2.23
+++ gw/smsc/smsc_emi2.c	3 Feb 2003 18:12:00 -0000
@@ -1109,25 +1109,23 @@
 
 			octstr_destroy(ts);
 			if (dlrmsg != NULL) {
-			    Octstr *moretext;
+			    /*
+			     * Note: the DLR URL is delivered in msg->sms.dlr_url already.
+			    */
+			    dlrmsg->sms.sms_type = report;
 
-				/*
-				* Recode the msg structure with the given msgdata.
-				* Note: the DLR URL is delivered in msg->sms.dlr_url already.
-				*/
-				dlrmsg->sms.msgdata = octstr_duplicate(emimsg->fields[E50_AMSG]);
-				octstr_hex_to_binary(dlrmsg->sms.msgdata);
-				dlrmsg->sms.sms_type = report;
-
-			    moretext = octstr_create("");
+			    dlrmsg->sms.msgdata = octstr_create("");
 			    if (octstr_get_char(emimsg->fields[0], 0) == 'N') {
-				octstr_append(moretext, emimsg->fields[1]);
-				octstr_append_char(moretext, '-');
-				octstr_append(moretext, emimsg->fields[2]);
+				octstr_append(dlrmsg->sms.msgdata, emimsg->fields[1]);
+				octstr_append_char(dlrmsg->sms.msgdata, '-');
+				/* system message is optional */
+				if (emimsg->fields[2] != NULL)
+				    octstr_append(dlrmsg->sms.msgdata, emimsg->fields[2]);
+				octstr_append_cstr(dlrmsg->sms.msgdata, "/NACK");
+			    }
+			    else {
+			        octstr_append_cstr(dlrmsg->sms.msgdata, "/ACK");
 			    }
-			    octstr_append_char(moretext, '/');
-			    octstr_insert(dlrmsg->sms.msgdata, moretext, 0);
-			    octstr_destroy(moretext);
 
 			    bb_smscconn_receive(conn, dlrmsg);
 			}
@@ -1152,13 +1150,13 @@
 				if(m == NULL) {
 				    info(0,"EMI2[%s]: uhhh m is NULL, very bad",
 					 octstr_get_cstr(privdata->name));
-				} else if ((m->sms.dlr_mask & 0x7)) {
+				} else if ((m->sms.dlr_mask & (DLR_SUCCESS|DLR_FAIL|DLR_BUFFERED))) {
 				    dlr_add(octstr_get_cstr((conn->id ? conn->id : privdata->name)),
 					    octstr_get_cstr(ts),
 					    octstr_get_cstr(m->sms.sender),
 					    octstr_get_cstr(adc),
-					    octstr_get_cstr(m->sms.service),
-					    octstr_get_cstr(m->sms.dlr_url),
+					    m->sms.service ? octstr_get_cstr(m->sms.service):NULL,
+					    m->sms.dlr_url ? octstr_get_cstr(m->sms.dlr_url):NULL,
 					    m->sms.dlr_mask,
 					    (m->sms.boxc_id ? octstr_get_cstr(m->sms.boxc_id) : NULL));
 				}