RE: [PATCH] custom MO Parameters for smsc-http
"Franck LAMASUTA" <[email protected]>
| Newsgroups | gmane.comp.mobile.kannel.devel |
|---|---|
| Message-ID | <[email protected]> |
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
(application/octet-stream, 8.3 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 19 Jun 2009 12:22:41 -0000
@@ -408,6 +408,7 @@
OCTSTR(generic-message-sent)
OCTSTR(generic-status-sent)
OCTSTR(generic-status-error)
+ OCTSTR(ack-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 19 Jun 2009 12:22:42 -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 *ack_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->ack_foreign_id_regex)
+ gw_regex_destroy(conndata->ack_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"
+ * ack-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->ack_foreign_id_regex != NULL) && DLR_IS_ENABLED_DEVICE(msg->sms.dlr_mask)) {
+ if (gw_regex_exec(conndata->ack_foreign_id_regex, body, sizeof(pmatch) / sizeof(regmatch_t), pmatch, 0) == 0) {
+ if (pmatch[1].rm_so != -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->ack_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("ack-foreign-id-regex"))) != NULL) {
+ if ((conndata->ack_foreign_id_regex = gw_regex_comp(os, REG_EXTENDED)) == NULL)
+ panic(0, "Could not compile pattern '%s' defined for variable 'ack-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 'ack-foreign-id-regex'", octstr_get_cstr(conn->id), octstr_get_cstr(os));
+ }
octstr_destroy(os);
}
}