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

"ups" fixed :D

http://www.blogalex.com/wp-content/uploads/2009/06/kannel-http-mo-params2.patch

Regards,
--
Alejandro Guerrieri
[email protected]



On 09/06/2009, at 15:40, Alejandro Guerrieri wrote:

> Oh, yes, sorry :D
>
> I'd rather duplicate the msg...
> --
> Alejandro Guerrieri
> [email protected]
>
>
>
> On 09/06/2009, at 15:38, Alejandro Guerrieri wrote:
>
>> Yes, I have to move that afterwards, otherwise the  
>> urltrans_fill_escape_codes call would panic:
>>
>> 2009-06-09 12:46:13 [15580] [6] PANIC: gwlib/octstr.c:2488:  
>> seems_valid_real: Assertion `ostr->len >= 0' failed. (Called from  
>> gwlib/octstr.c:1552:octstr_split_words.)
>> ...
>> 2009-06-09 12:46:13 [15580] [6] PANIC: 0    
>> bearerbox                           0x0009136d gw_panic + 253
>> 2009-06-09 12:46:13 [15580] [6] PANIC: 1    
>> bearerbox                           0x000925de log_thread_to + 750
>> 2009-06-09 12:46:13 [15580] [6] PANIC: 2    
>> bearerbox                           0x0009b360 octstr_split_words +  
>> 48
>> 2009-06-09 12:46:13 [15580] [6] PANIC: 3    
>> bearerbox                           0x000191e2  
>> urltrans_fill_escape_codes + 114
>> 2009-06-09 12:46:13 [15580] [6] PANIC: 4    
>> bearerbox                           0x0005d91b smsc_fake_create +  
>> 21803
>>
>> Probably the bb_smscconn_receive call destroys the msg structure?
>>
>> Regards,
>> --
>> Alejandro Guerrieri
>> [email protected]
>>
>>
>>
>> On 09/06/2009, at 15:16, Alexander Malysh wrote:
>>
>>> only one last thing:
>>>
>>> +        msg->sms.account = octstr_duplicate(account);
>>> +        msg->sms.binfo = octstr_duplicate(binfo);
>>> +        if (ret == -1) {
>>> +            retmsg = octstr_create("Not accepted");
>>> +            retstatus = fm->status_error;
>>> +        } else {
>>> +            retmsg = urltrans_fill_escape_codes(fm->message_sent,  
>>> msg);
>>> +            retstatus = fm->status_sent;
>>> +        }
>>> +        ret = bb_smscconn_receive(conn, msg);
>>> +    }
>>>
>>> are you sure you want check ret before bb_smscconn_receive ? ;)
>>>
>>> Thanks,
>>> Alex
>>>
>>> Am 09.06.2009 um 13:23 schrieb Alejandro Guerrieri:
>>>
>>>> Ok, done!
>>>>
>>>> http://www.blogalex.com/wp-content/uploads/2009/06/kannel-http-mo-params1.patch
>>>>
>>>> I've fixed the dlr-url bug _only_ on the generic part. If no  
>>>> objections, I'm doing a second patch to fix it on the kannel smsc  
>>>> (since it's in fact a "separate" issue).
>>>>
>>>> I've also fixed a small glitch on the dlrmsg response code (I was  
>>>> using the "error" status code for successful submits as well).
>>>>
>>>> Last but not least, I've added url translation to the response  
>>>> message, so now you can include escape codes on the response,  
>>>> which may come handy on many cases (for example, to return  
>>>> kannel's message id on the requests). Userguide part updated  
>>>> accordingly.
>>>>
>>>> Regards,
>>>> --
>>>> Alejandro Guerrieri
>>>> [email protected]
>>>>
>>>>
>>>>
>>>> On 09/06/2009, at 11:27, Alexander Malysh wrote:
>>>>
>>>>>
>>>>> Am 09.06.2009 um 11:23 schrieb Alejandro Guerrieri:
>>>>>
>>>>>> Alex,
>>>>>>
>>>>>> Regarding the charsets, I agree it's better to tackle it now  
>>>>>> that leave it for "later" (aka, "never" ;) ).
>>>>>>
>>>>>> I'm fixing the charset issue and the dlr bugs and resending  
>>>>>> later.
>>>>>>
>>>>>
>>>>> thanks alex!
>>>>>
>>>>>> Regards,
>>>>>> --
>>>>>> Alejandro Guerrieri
>>>>>> [email protected]
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 09/06/2009, at 11:17, Alexander Malysh wrote:
>>>>>>
>>>>>>>
>>>>>>> Am 09.06.2009 um 11:05 schrieb Alejandro Guerrieri:
>>>>>>>
>>>>>>>> Alex,
>>>>>>>>
>>>>>>>> Commenting inline below.
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> --
>>>>>>>> Alejandro Guerrieri
>>>>>>>> [email protected]
>>>>>>>>
>>>>>>>>
>>>>>>>> On 09/06/2009, at 10:44, Alexander Malysh wrote:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Seems you don't handle charset at all. Do you assume to  
>>>>>>>>> receive UTF-8 in any case? I don't think it's
>>>>>>>>> applicable to generic interface. At least alt-charset should  
>>>>>>>>> be taken in account...
>>>>>>>>>
>>>>>>>>
>>>>>>>> I'm not adding anything new here, the MO handling is built on  
>>>>>>>> the original code for kannel_receive_sms (which was the  
>>>>>>>> default interface for MO on the generic interface anyway).  
>>>>>>>> There's no charset handling in there either. I agree it makes  
>>>>>>>> sense to have charset handling, since an http interface  
>>>>>>>> should be as flexible as possible, but is it a showstopper at  
>>>>>>>> this point?
>>>>>>>>
>>>>>>>
>>>>>>> It is not really show stopper but if it will not be added now  
>>>>>>> it will remain so for ages. It is not a problem for  
>>>>>>> kannel_receive_sms because it's
>>>>>>> well defined interface to smsbox which uses UTF-8.
>>>>>>> I would prefer to see it implemented (there only few line of  
>>>>>>> code ;) )
>>>>>>>
>>>>>>>>
>>>>>>>>> some comments to patch below...
>>>>>>>>>
>>>>>>>>> why is dlrurl required for DLR? We have dlrurl stored in DLR  
>>>>>>>>> storage...
>>>>>>>>>
>>>>>>>>> +    }
>>>>>>>>> +    else if (dlrurl != NULL && dlrmask != 0 && dlrmid !=  
>>>>>>>>> NULL) {
>>>>>>>>> +        /* we got a DLR, and we don't require additional  
>>>>>>>>> values */
>>>>>>>>> +        Msg *dlrmsg;
>>>>>>>>>
>>>>>>>>
>>>>>>>> Idem... why is it needed for the kannel interface then?
>>>>>>>
>>>>>>> hmm, seems both buggy then. We don't need dlr-url for DLR.  
>>>>>>> Could you please fix both interfaces?
>>>>>>>
>>>>>>>>
>>>>>>>>> hmm this check is not complete. it should be udh_len +  
>>>>>>>>> msgdata_len but msgdata_len depends in coding...
>>>>>>>>>
>>>>>>>>> +    else if (udh != NULL && octstr_len(udh) >  
>>>>>>>>> MAX_SMS_OCTETS) {
>>>>>>>>> +        error(0, "HTTP[%s]: UDH field is too long, rejected",
>>>>>>>>> +              octstr_get_cstr(conn->id));
>>>>>>>>> +        retmsg = octstr_create("UDH field is too long,  
>>>>>>>>> rejected");
>>>>>>>>> +        retstatus = fm->status_error;
>>>>>>>>> +    }
>>>>>>>>>
>>>>>>>>
>>>>>>>> Idem... this is from kannel_receive_sms as well. If it's  
>>>>>>>> wrong here, then it's wrong on the "kannel" smsc as well.
>>>>>>>
>>>>>>> Hmm seems, I'm wrong here. This check checks for the right UDH  
>>>>>>> but allows long MO messages.
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> why do you always load generic config options ?
>>>>>>>>>
>>>>>>>>> static void generic_send_sms(SMSCConn *conn, Msg *sms)
>>>>>>>>> {
>>>>>>>>> @@ -1635,6 +1934,7 @@
>>>>>>>>> cfg_get_bool(&conndata->no_sep, cfg, octstr_imm("no-sep"));
>>>>>>>>> conndata->proxy = cfg_get(cfg, octstr_imm("system-id"));
>>>>>>>>> conndata->alt_charset = cfg_get(cfg, octstr_imm("alt- 
>>>>>>>>> charset"));
>>>>>>>>> +    conndata->fieldmap = generic_get_field_map(cfg);
>>>>>>>>>
>>>>>>>>> ---> conndata->fieldmap = NULL:
>>>>>>>>>
>>>>>>>>> if (conndata->send_url == NULL)
>>>>>>>>> panic(0, "HTTP[%s]: Sending not allowed. No 'send-url'  
>>>>>>>>> specified.",
>>>>>>>>> @@ -1693,7 +1993,7 @@
>>>>>>>>>           octstr_get_cstr(conn->id));
>>>>>>>>>     goto error;
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> --->    conndata->fieldmap = generic_get_field_map(cfg);
>>>>>>>>>
>>>>>>>>> -        conndata->receive_sms = kannel_receive_sms; /*  
>>>>>>>>> emulate sendsms interface */
>>>>>>>>> +        conndata->receive_sms = generic_receive_sms; /*  
>>>>>>>>> emulate sendsms interface */
>>>>>>>>> conndata->send_sms = generic_send_sms;
>>>>>>>>> conndata->parse_reply = generic_parse_reply;
>>>>>>>>
>>>>>>>> True, it's a waste of CPU cycles if you're using other smsc- 
>>>>>>>> types instead. Fixing it.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Alex
>>>>>>>>>
>>>>>>>>> Am 09.06.2009 um 10:14 schrieb Alejandro Guerrieri:
>>>>>>>>>
>>>>>>>>>> Ok, here's a new version of my patch to add support for  
>>>>>>>>>> custom MO parameters on the generic http-smsc.
>>>>>>>>>>
>>>>>>>>>> New features:
>>>>>>>>>>
>>>>>>>>>> 1. Support for setting the numeric response code for  
>>>>>>>>>> successful and failed requests (as it is now, it always  
>>>>>>>>>> returns 202 HTTP_ACCEPTED).
>>>>>>>>>> 2. Support for setting the text response for successful  
>>>>>>>>>> requests (right now it returns "Sent.").
>>>>>>>>>> 3. Some code cleanups (extra lines, parameter expansion  
>>>>>>>>>> after authorization).
>>>>>>>>>> 4. Documentation.
>>>>>>>>>>
>>>>>>>>>> http://www.blogalex.com/archives/192
>>>>>>>>>>
>>>>>>>>>> I can commit if no objections.
>>>>>>>>>>
>>>>>>>>>> Regards,
>>>>>>>>>> --
>>>>>>>>>> Alejandro Guerrieri
>>>>>>>>>> [email protected]
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 01/06/2009, at 22:50, Alexander Malysh wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> here some comments...
>>>>>>>>>>>
>>>>>>>>>>> in generic_receive_sms you first receive all values than  
>>>>>>>>>>> converts it and only
>>>>>>>>>>> after this done check authorization. Make no sense...  
>>>>>>>>>>> first check auth then convert anything.
>>>>>>>>>>>
>>>>>>>>>>> +    else if (from == NULL || to == NULL || text == NULL) {
>>>>>>>>>>> +
>>>>>>>>>>> +        error(0, "HTTP[%s]: Insufficient args",
>>>>>>>>>>>
>>>>>>>>>>> why extra line?
>>>>>>>>>>>
>>>>>>>>>>> I think retmsg should also be configurable. Because not  
>>>>>>>>>>> all gateways will accept 'Sent'  as response.
>>>>>>>>>>> I think even HTTP error code for not accepted MOs/DLRs  
>>>>>>>>>>> should be configurable.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Alex
>>>>>>>>>>>
>>>>>>>>>>> P.S. And userguide part would be nice ;)
>>>>>>>>>>>
>>>>>>>>>>> Am 28.05.2009 um 23:00 schrieb Alejandro Guerrieri:
>>>>>>>>>>>
>>>>>>>>>>>> HI,
>>>>>>>>>>>>
>>>>>>>>>>>> I've finished my patch to configure the parameter names  
>>>>>>>>>>>> for MO on the generic http-smsc.
>>>>>>>>>>>>
>>>>>>>>>>>> To use it you just add a few entries on the smsc  
>>>>>>>>>>>> definition, only with the parameters you want to rename.
>>>>>>>>>>>>
>>>>>>>>>>>> e.g:
>>>>>>>>>>>>
>>>>>>>>>>>> generic-param-from = "phoneNumber"
>>>>>>>>>>>> generic-param-to = "shortCode"
>>>>>>>>>>>> generic-param-text = "message"
>>>>>>>>>>>>
>>>>>>>>>>>> More details and the patch here:
>>>>>>>>>>>>
>>>>>>>>>>>> http://www.blogalex.com/archives/171
>>>>>>>>>>>>
>>>>>>>>>>>> Of course I'm writing the userguide part if this goes  
>>>>>>>>>>>> forward :)
>>>>>>>>>>>>
>>>>>>>>>>>> Regards,
>>>>>>>>>>>> --
>>>>>>>>>>>> Alejandro Guerrieri
>>>>>>>>>>>> [email protected]
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>>
>