RE: [PATCH] custom MO Parameters for smsc-http

"Franck LAMASUTA" <[email protected]>
Newsgroups gmane.comp.mobile.kannel.devel
Message-ID <[email protected]> 
Here is the new version...


 

> -----Original Message-----
> From: [email protected] 
> [mailto:[email protected]] On Behalf Of Franck LAMASUTA
> Sent: Monday, June 22, 2009 4:53 PM
> To: [email protected]
> Subject: RE: [PATCH] custom MO Parameters for smsc-http
> 
> Thanks for your comments.
> 
> First comment regarding the variable name:
> The new variable is closely related to status-success-regex 
> which doesn't start with "generic-".
> But ok, I will change it if you think it's better...
> 
> 
> Second comment regarding pmatch[1].rm_eo:
> The test seems to be useless according to regexec() documentation.
> But it's cheap so it can't hurt.
> 
> I will update userguide.xml also.
> 
> Franck
> 
> 
> 
> > -----Original Message-----
> > From: Alejandro Guerrieri [mailto:[email protected]] 
> > Sent: Monday, June 22, 2009 12:37 PM
> > To: Alexander Malysh
> > Cc: [email protected];[email protected]
> > Subject: Re: [PATCH] custom MO Parameters for smsc-http
> > 
> > +1 with the changes Alex proposed. I'd rename 
> > ack_foreign_id_regex as  
> > well, for clarity's sake.
> > 
> > Regards,
> > --
> > Alejandro Guerrieri
> > [email protected]
> > 
> > On 22/06/2009, at 12:26, Alexander Malysh wrote:
> > 
> > > Hi,
> > >
> > > patch looks ok. Here some minor comments:
> > >
> > > +    OCTSTR(ack-foreign-id-regex)
> > > would it be better to name it something like generic-foreign-id- 
> > > regex? so user know it belongs only to generic http...
> > >
> > > +                if (pmatch[1].rm_so != -1) {
> > > better to check the end as well -> if (pmatch[1].rm_eo != -1 &&  
> > > pmatch[1].rm_so != -1)
> > >
> > > Otherwise patch looks good.
> > > Please make patch to userguide and I will commit it to CVS.
> > >
> > > Thanks,
> > > Alex
> > >
> > > Am 19.06.2009 um 14:40 schrieb Franck LAMASUTA:
> > >
> > >> Here it is, implemented with second idea...
> > >>
> > >> I did a small optimization of previous gw_regex_exec() calls in  
> > >> generic_parse_reply().
> > >> I have also fixed 3 debug() calls in generic_receive_sms()  
> > >> ("smsc.http.kannel" => "smsc.http.generic").
> > >>
> > >> Waiting for your comments...
> > >>
> > >> Franck
> > >>
> > >>
> > >>
> > >>> -----Original Message-----
> > >>> From: Alejandro Guerrieri [mailto:[email protected]]
> > >>> Sent: Wednesday, June 10, 2009 3:08 PM
> > >>> To: Alexander Malysh
> > >>> Cc: [email protected];[email protected]
> > >>> Subject: Re: [PATCH] custom MO Parameters for smsc-http
> > >>>
> > >>> Me too, second one is far more flexible and clean.
> > >>> --
> > >>> Alejandro Guerrieri
> > >>> [email protected]
> > >>>
> > >>>
> > >>>
> > >>> On 10/06/2009, at 14:31, Alexander Malysh wrote:
> > >>>
> > >>>> Hi,
> > >>>>
> > >>>> I'm for the second idea, regex rocks ;)
> > >>>>
> > >>>> Thanks,
> > >>>> Alex
> > >>>>
> > >>>> Am 10.06.2009 um 12:37 schrieb Franck LAMASUTA:
> > >>>>
> > >>>>> It's not so urgent but I could try, it could be interesting...
> > >>>>>
> > >>>>> I have 2 ideas so far...
> > >>>>>
> > >>>>>
> > >>>>> First idea
> > >>>>> ==========
> > >>>>>
> > >>>>> I could add 2 new config parameters to define 2 regex:
> > >>>>> - the first one, to find the start of the foreign id.
> > >>>>> - the second one, to find the end of the foreign id.
> > >>>>>
> > >>>>> For Claro, they could be defined like this:
> > >>>>> foreign-id-regex-begin = "<transaction-id>"
> > >>>>> foreign-id-regex-end = "</transaction-id>"
> > >>>>>
> > >>>>> For Clickatell, they could be defined like this:
> > >>>>> foreign-id-regex-begin = "ID: "
> > >>>>> foreign-id-regex-end = " "
> > >>>>>
> > >>>>> For Brunet, they could be defined like this:
> > >>>>> foreign-id-regex-begin = "MessageId="
> > >>>>> foreign-id-regex-end = " "
> > >>>>>
> > >>>>> In generic_parse_reply(), only in case of success,
> > >>> regex-begin will
> > >>>>> be used to find the beginning of the id. Starting from
> > >>> there, regex-
> > >>>>> end will be used to find the end of the id. If regex-end
> > >>> can't find
> > >>>>> a match, the id will be read until the end of text 
> (body) and an
> > >>>>> ending CRLF will be removed.
> > >>>>> Then, the id will be used to call dlr_add() (like done in
> > >>>>> clickatell_parse_reply()).
> > >>>>>
> > >>>>>
> > >>>>> Second idea
> > >>>>> ===========
> > >>>>>
> > >>>>> It's basically the same idea except that only one regex
> > >>> will be used.
> > >>>>>
> > >>>>> For Claro, it could be defined like this:
> > >>>>> foreign-id-regex = "<transaction-id>(.*)</transaction-id>"
> > >>>>>
> > >>>>> For Clickatell, it could be defined like this (if an 
> id contains
> > >>>>> only digits):
> > >>>>> foreign-id-regex = "ID: ([0-9]+)"
> > >>>>>
> > >>>>> The offsets given by gw_regex_exec() in pmatch[1] could be
> > >>> used to
> > >>>>> extract the id.
> > >>>>> However, it could be more difficult to understand for 
> people who
> > >>>>> are not regex experts...
> > >>>>>
> > >>>>>
> > >>>>> Any comment, opinion, suggestion or better idea? :)
> > >>>>>
> > >>>>> Franck
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>> From: Alexander Malysh
> > >>>>>> Sent: Tuesday, June 09, 2009 6:05 PM
> > >>>>>>
> > >>>>>>
> > >>>>>> Am 09.06.2009 um 17:55 schrieb Alejandro Guerrieri:
> > >>>>>>
> > >>>>>>> Franck,
> > >>>>>>>
> > >>>>>>> The dlr part is inherited from the "kannel" smsc 
> > format. In the
> > >>>>>>> actual implementation there's no way to grab the remote
> > >>> message id
> > >>>>>>> from a request, and it'd make a lot of sense to be 
> > able to do  
> > >>>>>>> it.
> > >>>>>>>
> > >>>>>>> I can try adding it, but don't refrain to do it so
> > >>> yourself if you
> > >>>>>>> feel the urge (you'll learn a lot about the code in the
> > >>> process!).
> > >>>>>>
> > >>>>>> yep, Alex is right here :) and we need more contributors :)
> > >>>>>>
> > >>>>>>>
> > >>>>>>> Opinions?
> > >>>>>>> --
> > >>>>>>> Alejandro Guerrieri
> > >>>>>>> [email protected]
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> On 09/06/2009, at 17:47, Franck LAMASUTA wrote:
> > >>>>>>>
> > >>>>>>>> Hi Alejandro ,
> > >>>>>>>>
> > >>>>>>>> I work with Hervé who posted to this list the 25th of May  
> > >>>>>>>> (Claro
> > >>>>>>>> http communication).
> > >>>>>>>> It seems the patch you've done on the generic part 
> could be  
> > >>>>>>>> very
> > >>>>>>>> useful for us to manage our communications with Claro. It
> > >>>>>> could be
> > >>>>>>>> better than a specific implementation for Claro.
> > >>>>>>>>
> > >>>>>>>> However, I don't understand how your patch manages a DLR:
> > >>>>>>>> In generic_receive_sms(), you call dlr_find() (line 
> > 1674 of the
> > >>>>>>>> patched source) to retrieve a previous record stored 
> > in the DLR
> > >>>>>>>> storage.
> > >>>>>>>> It seems fine except that dlr_add() is never called in
> > >>>>>>>> generic_parse_reply(), so I guess dlr_find() will always
> > >>>>>> return NULL.
> > >>>>>>>> Am I wrong???
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>> In fact, when we submit a MT SMS to Claro with this request
> > >>>>>>>>
> > >>>>>> 
> http://retail.mds.claro.com.br/MSE/api?profile=xxxx&pwd=xxxx&m
> > >>>>> ode=assync-delivery&ANUM=3333&BNUM=1234567&TEXT=test
> > >>>>>>>>
> > >>>>>>>> we get a HTTP code 200 with this body:
> > >>>>>>>>
> > >>>>>>>> <?xml version="1.0" encoding="UTF-8" ?>
> > >>>>>>>> <mse-response>
> > >>>>>>>> <status-code>0</status-code>
> > >>>>>>>> <profile>profleID</profile>
> > >>>>>>>> <transaction-id>1020606201622668099</transaction-id>
> > >>>>>>>> </mse-response>
> > >>>>>>>>
> > >>>>>>>> So I guess that the transaction-id should be used in
> > >>>>>>>> generic_parse_reply() to call dlr_add().
> > >>>>>>>> Unfortunately, the current implementation does not allow
> > >>>>>> to manage
> > >>>>>>>> such an id, it only allows to search if the request was
> > >>>>>> successful
> > >>>>>>>> or not through the regex.
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>> If all my hypothesis are right... :-)
> > >>>>>>>> I could try to patch generic_parse_reply() to manage 
> > also the  
> > >>>>>>>> id
> > >>>>>>>> through another regex. It will be my first Kannel 
> patch! ;-)
> > >>>>>>>> If you prefer to do it, no problem, just let me know.
> > >>>>>>>>
> > >>>>>>>> If I'm wrong, please clarify how it works.
> > >>>>>>>>
> > >>>>>>>> Regards,
> > >>>>>>>> Franck
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>
> > >>>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>
> > >>>>>
> > >>>>
> > >>>>
> > >>>
> > >>>
> > >> <kannel_http-generic.patch>
> > >
> > >
> > 
> > 
> > 
> 
> 
> 
>
kannel_http-generic_2.patch (application/octet-stream, 10.4 KB) 
Index: gwlib/cfg.def
===================================================================
RCS file: /home/cvs/gateway/gwlib/cfg.def,v
retrieving revision 1.140
diff -u -b -r1.140 cfg.def
--- gwlib/cfg.def	9 Jun 2009 17:05:08 -0000	1.140
+++ gwlib/cfg.def	22 Jun 2009 16:24:37 -0000
@@ -408,6 +408,7 @@
     OCTSTR(generic-message-sent)
     OCTSTR(generic-status-sent)
     OCTSTR(generic-status-error)
+    OCTSTR(generic-foreign-id-regex)
 )
 
 
Index: gw/smsc/smsc_http.c
===================================================================
RCS file: /home/cvs/gateway/gw/smsc/smsc_http.c,v
retrieving revision 1.64
diff -u -b -r1.64 smsc_http.c
--- gw/smsc/smsc_http.c	10 Jun 2009 13:03:35 -0000	1.64
+++ gw/smsc/smsc_http.c	22 Jun 2009 16:24:37 -0000
@@ -185,6 +185,10 @@
     regex_t *permfail_regex;
     regex_t *tempfail_regex;
 
+    /* Compiled regex for the 'generic' type to get the foreign message id
+    * from the HTTP response body */
+    regex_t *generic_foreign_id_regex;
+
     /* callback functions set by HTTP-SMSC type */
     void (*send_sms) (SMSCConn *conn, Msg *msg);
     void (*parse_reply) (SMSCConn *conn, Msg *msg, int status,
@@ -237,6 +241,8 @@
         gw_regex_destroy(conndata->permfail_regex);
     if (conndata->tempfail_regex)
         gw_regex_destroy(conndata->tempfail_regex);
+    if (conndata->generic_foreign_id_regex)
+        gw_regex_destroy(conndata->generic_foreign_id_regex);
     fieldmap_destroy(conndata->fieldmap);
     octstr_destroy(conndata->allow_ip);
     octstr_destroy(conndata->send_url);
@@ -1532,6 +1538,7 @@
  *  status-success-regex = "ok"
  *  status-permfail-regex = "failure"
  *  status-tempfail-regex = "retry later"
+ *  generic-foreign-id-regex = "<id>(.+)</id>"
  *  generic-param-from = "phoneNumber"
  *  generic-param-to = "shortCode"
  *  generic-param-text = "message"
@@ -1655,7 +1662,7 @@
     if (tmp_string) {
         sscanf(octstr_get_cstr(tmp_string),"%d", &dlrmask);
     }
-    debug("smsc.http.kannel", 0, "HTTP[%s]: Received an HTTP request",
+    debug("smsc.http.generic", 0, "HTTP[%s]: Received an HTTP request",
           octstr_get_cstr(conn->id));
 
     if ((conndata->username != NULL && conndata->password != NULL) &&
@@ -1679,7 +1686,7 @@
         if (dlrmsg != NULL) {
             dlrmsg->sms.sms_type = report_mo;
 
-            debug("smsc.http.kannel", 0, "HTTP[%s]: Received DLR for DLR-URL <%s>",
+            debug("smsc.http.generic", 0, "HTTP[%s]: Received DLR for DLR-URL <%s>",
                   octstr_get_cstr(conn->id), octstr_get_cstr(dlrmsg->sms.dlr_url));
 
             Msg *resp = msg_duplicate(dlrmsg);
@@ -1752,7 +1759,7 @@
         account = http_cgi_variable(cgivars, octstr_get_cstr(fm->account));
         binfo = http_cgi_variable(cgivars, octstr_get_cstr(fm->binfo));
 
-        debug("smsc.http.kannel", 0, "HTTP[%s]: Constructing new SMS",
+        debug("smsc.http.generic", 0, "HTTP[%s]: Constructing new SMS",
               octstr_get_cstr(conn->id));
 
         /* convert character encoding if required */
@@ -1785,7 +1792,7 @@
 
     reply_headers = gwlist_create();
     http_header_add(reply_headers, "Content-Type", "text/plain");
-    debug("smsc.http.kannel", 0, "HTTP[%s]: Sending reply",
+    debug("smsc.http.generic", 0, "HTTP[%s]: Sending reply",
           octstr_get_cstr(conn->id));
     http_send_reply(client, retstatus, reply_headers, retmsg);
 
@@ -1819,8 +1826,8 @@
                                 List *headers, Octstr *body)
 {
     ConnData *conndata = conn->data;
-    size_t n_match = 1;
-    regmatch_t p_match[10];
+    regmatch_t pmatch[2];
+    Octstr *msgid = NULL;
     
     /* 
      * Our generic type checks only content on the HTTP reponse body.
@@ -1828,17 +1835,31 @@
      * This is the most generic criteria (at the moment). 
      */
     if ((conndata->success_regex != NULL) && 
-        (gw_regex_exec(conndata->success_regex, body, n_match, p_match, 0) == 0)) {
+        (gw_regex_exec(conndata->success_regex, body, 0, NULL, 0) == 0)) {
+        /* SMSC ACK... the message id should be in the body */
+        if ((conndata->generic_foreign_id_regex != NULL) && DLR_IS_ENABLED_DEVICE(msg->sms.dlr_mask)) {
+            if (gw_regex_exec(conndata->generic_foreign_id_regex, body, sizeof(pmatch) / sizeof(regmatch_t), pmatch, 0) == 0) {
+                if (pmatch[1].rm_so != -1 && pmatch[1].rm_eo != -1) {
+                    msgid = octstr_copy(body, pmatch[1].rm_so, pmatch[1].rm_eo - pmatch[1].rm_so);
+                    debug("smsc.http.generic", 0, "HTTP[%s]: Found foreign message id <%s> in body.", octstr_get_cstr(conn->id), octstr_get_cstr(msgid));
+                    dlr_add(conn->id, msgid, msg);
+                }
+            }
+            if (msgid == NULL)
+                warning(0, "HTTP[%s]: Can't get the foreign message id from the HTTP body.", octstr_get_cstr(conn->id));
+            else
+                octstr_destroy(msgid);
+        }
         bb_smscconn_sent(conn, msg, NULL);
     } 
     else if ((conndata->permfail_regex != NULL) &&        
-        (gw_regex_exec(conndata->permfail_regex, body, n_match, p_match, 0) == 0)) {
+        (gw_regex_exec(conndata->permfail_regex, body, 0, NULL, 0) == 0)) {
         error(0, "HTTP[%s]: Message not accepted.", octstr_get_cstr(conn->id));
         bb_smscconn_send_failed(conn, msg,
             SMSCCONN_FAILED_MALFORMED, octstr_duplicate(body));
     }
     else if ((conndata->tempfail_regex != NULL) &&        
-        (gw_regex_exec(conndata->tempfail_regex, body, n_match, p_match, 0) == 0)) {
+        (gw_regex_exec(conndata->tempfail_regex, body, 0, NULL, 0) == 0)) {
         warning(0, "HTTP[%s]: Message temporary not accepted, will retry.", 
                 octstr_get_cstr(conn->id));
         bb_smscconn_send_failed(conn, msg,
@@ -1933,6 +1954,7 @@
     conndata->http_ref = NULL;
     conndata->success_regex = 
         conndata->permfail_regex = conndata->tempfail_regex = NULL;
+    conndata->generic_foreign_id_regex = NULL;
 
     conndata->allow_ip = cfg_get(cfg, octstr_imm("connect-allow-ip"));
     conndata->send_url = cfg_get(cfg, octstr_imm("send-url"));
@@ -2011,18 +2033,28 @@
 
         /* pre-compile regex expressions */
         if (os != NULL) {   /* this is implicite due to the above if check */
-            if ((conndata->success_regex = gw_regex_comp(os, REG_EXTENDED)) == NULL)
-                panic(0, "Could not compile regex pattern '%s'", octstr_get_cstr(os));
+            if ((conndata->success_regex = gw_regex_comp(os, REG_EXTENDED|REG_NOSUB)) == NULL)
+                panic(0, "Could not compile pattern '%s' defined for variable 'status-success-regex'", octstr_get_cstr(os));
             octstr_destroy(os);
         }
         if ((os = cfg_get(cfg, octstr_imm("status-permfail-regex"))) != NULL) {
-            if ((conndata->permfail_regex = gw_regex_comp(os, REG_EXTENDED)) == NULL)
-                panic(0, "Could not compile regex pattern '%s'", octstr_get_cstr(os));
+            if ((conndata->permfail_regex = gw_regex_comp(os, REG_EXTENDED|REG_NOSUB)) == NULL)
+                panic(0, "Could not compile pattern '%s' defined for variable 'status-permfail-regex'", octstr_get_cstr(os));
             octstr_destroy(os);
         }
         if ((os = cfg_get(cfg, octstr_imm("status-tempfail-regex"))) != NULL) {
-            if ((conndata->tempfail_regex = gw_regex_comp(os, REG_EXTENDED)) == NULL)
-                panic(0, "Could not compile regex pattern '%s'", octstr_get_cstr(os));
+            if ((conndata->tempfail_regex = gw_regex_comp(os, REG_EXTENDED|REG_NOSUB)) == NULL)
+                panic(0, "Could not compile pattern '%s' defined for variable 'status-tempfail-regex'", octstr_get_cstr(os));
+            octstr_destroy(os);
+        }
+        if ((os = cfg_get(cfg, octstr_imm("generic-foreign-id-regex"))) != NULL) {
+            if ((conndata->generic_foreign_id_regex = gw_regex_comp(os, REG_EXTENDED)) == NULL)
+                panic(0, "Could not compile pattern '%s' defined for variable 'generic-foreign-id-regex'", octstr_get_cstr(os));
+            else {
+                /* check quickly that at least 1 group seems to be defined in the regex */
+                if (octstr_search_char(os, '(', 0) == -1 || octstr_search_char(os, ')', 0) == -1)
+                    warning(0, "HTTP[%s]: No group defined in pattern '%s' for variable 'generic-foreign-id-regex'", octstr_get_cstr(conn->id), octstr_get_cstr(os));
+            }
             octstr_destroy(os);
         }
     }
Index: doc/userguide/userguide.xml
===================================================================
RCS file: /home/cvs/gateway/doc/userguide/userguide.xml,v
retrieving revision 1.352
diff -u -b -r1.352 userguide.xml
--- doc/userguide/userguide.xml	19 Jun 2009 13:14:09 -0000	1.352
+++ doc/userguide/userguide.xml	22 Jun 2009 16:24:38 -0000
@@ -4114,21 +4114,29 @@
      <entry><literal>POSIX regular expression</literal></entry>
      <entry valign="bottom">
        Regular expression to match against HTTP
-       response body conent, indicating a successfull submission. 
+       response body content, indicating a successful submission.
      </entry></row>
      
    <row><entry><literal>status-permfail-regex (o)</literal></entry>
      <entry><literal>POSIX regular expression</literal></entry>
      <entry valign="bottom">
        Regular expression to match against HTTP
-       response body conent, indicating a permanent failure. 
+       response body content, indicating a permanent failure.
      </entry></row>
 
    <row><entry><literal>status-tempfail-regex (o)</literal></entry>
      <entry><literal>POSIX regular expression</literal></entry>
      <entry valign="bottom">
        Regular expression to match against HTTP
-       response body conent, indicating a temporary failure. 
+       response body content, indicating a temporary failure.
+     </entry></row>
+
+   <row><entry><literal>generic-foreign-id-regex (o)</literal></entry>
+     <entry><literal>POSIX regular expression</literal></entry>
+     <entry valign="bottom">
+       Regular expression to match against HTTP
+       response body content to get the foreign message id
+       in case of successful submission.
      </entry></row>
     
    <row><entry><literal>generic-param-username (o)</literal></entry>
@@ -4299,6 +4307,7 @@
 status-success-regex = "ok"
 status-permfail-regex = "failure"
 status-tempfail-regex = "retry later"
+generic-foreign-id-regex = "&lt;id&gt;(.+)&lt;/id&gt;"
 generic-param-from = "phoneNumber"
 generic-param-to = "shortCode"
 generic-message-sent = "Message sent with ID: %I"
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.