Re: Patch: gw/urltrans.c

All right. Here it is.

Enjoy,
Nikos
----- Original Message ----- 
From: "Alexander Malysh" <[email protected]>
To: "Nikos Balkanas" <[email protected]>
Cc: <[email protected]>
Sent: Monday, August 30, 2010 5:21 PM
Subject: Re: Patch: gw/urltrans.c


see inlined...

Am 30.08.2010 um 16:06 schrieb Nikos Balkanas:

> Hi,
>
> Please see comments inlined.
>
> Nikos
> ----- Original Message ----- From: "Alexander Malysh" <[email protected]>
> To: "Nikos Balkanas" <[email protected]>
> Cc: <[email protected]>
> Sent: Monday, August 30, 2010 12:08 PM
> Subject: Re: Patch: gw/urltrans.c
>
>
>>> Hi,
>
>>> why do you change explicit configuration to something implicit?
>
> I don't understand. Please explain what is explicit and what is implicit.

use have to decide what to use keyword or keyword-regex and both settings 
just not allowed
and is error. Now it also indicated so. With you patch you make decision for 
user to use keyword or
keyword-regex...

>
>>> user have to decide whether to use keyword or keyword-regex and if both 
>>> given
>>> it's error and was indicated so.
>
> Just trying to align source with documentation, by popular demand.
>
>>> About the case sensitivity, I would like to give a user possibility to 
>>> decide.
>>> The policy should be:
>>> - if keyword given make it case insensitive (just backwards 
>>> compatibility)
>>> - if keyword-regex given it's case sensitive
>
> That's confusing for anyone. How do you know what users want? It could 
> blow the list off. I think everything should be treated equally. 
> keyword-regex is just an extention of keyword. Rules applying to the first 
> should apply to the second as well.

no, keyword-regex is replacement for keyword and keyword is there only for 
backwards compatibility. And keyword-regex was extra set
to be case sensitive because some users want it so and if you make it case 
insensitive user will not be able to do so.
Other way around works, e.g. you will match OK: [o][O][kK]
I see a convert_range in find_translation as a bug...

>
> Any votes from the list?
>
>>> So the patch should be:
>>> - remove octstr_convert_range from find_translation
>>> - add REG_ICASE for keyword
>>> - documentation
>
> No problem, but first please address my points.
>
>>> Thanks,
>>> Alexander Malysh
>
> Am 27.08.2010 um 02:24 schrieb Nikos Balkanas:
>
>> Added some needed initialization. Sorry about that.
>>
>> BR,
>> Nikos
>> ----- Original Message ----- From: "Nikos Balkanas" <[email protected]>
>> To: <[email protected]>
>> Cc: "Rene Kluwen" <[email protected]>; "'Alvaro Cornejo'" 
>> <[email protected]>
>> Sent: Friday, August 27, 2010 1:30 AM
>> Subject: Re: Patch: gw/urltrans.c
>>
>>
>>> Case insensitivity is observed in the code. It just lacks in 
>>> configuration.
>>> I changed patch to match documentation:
>>>
>>> 1) When given both keyword-regex and keyword, it will just prefer
>>> keyword-regex without complains (according to UG).
>>> 2) It will accept any case in configuration for both keyword and
>>> keyword-regex and do case incensitive matching.
>>> 3) Fixed a diagnostic that was pooping up evrywhere during matches.
>>>
>>> Enjoy,
>>> Nikos
>>> ----- Original Message ----- From: "Rene Kluwen" <[email protected]>
>>> To: "'Alvaro Cornejo'" <[email protected]>; "'Nikos Balkanas'"
>>> <[email protected]>
>>> Cc: <[email protected]>
>>> Sent: Thursday, August 26, 2010 5:10 PM
>>> Subject: RE: Patch: gw/urltrans.c
>>>
>>>
>>> A lot of hand sets, nowadays automatically convert the first letter to
>>> uppercase whilst typing an sms message.
>>>
>>> So I think case-insensivity is not a bad thing?
>>>
>>> == Rene
>>>
>>> -----Original Message-----
>>> From: [email protected] [mailto:[email protected]] On 
>>> Behalf
>>> Of Alvaro Cornejo
>>> Sent: Thursday, 26 August, 2010 15:44
>>> To: Nikos Balkanas
>>> Cc: [email protected]
>>> Subject: Re: Patch: gw/urltrans.c
>>>
>>> It should then be noted in the user guide. Otherwise we will continue
>>> receiving this questions over and over.
>>>
>>> |---------------------------------------------------------------------------
>>> --------------------------------------|
>>> EnvΞ½e y Reciba Datos y mensajes de Texto (SMS) hacia y desde cualquier
>>> celular y Nextel
>>> en el PerΟ, MΞΉxico y en mas de 180 paises. Use aplicaciones 2 vias via
>>> SMS y GPRS online
>>> Visitenos en www.perusms.NET www.smsglobal.com.mx y
>>> www.pravcom.com
>>>
>>>
>>>
>>> 2010/8/26 Nikos Balkanas <[email protected]>:
>>>> I don't know if this is really necessary. Just using lower case in the
>>>> keyword-regex pattern will work as well. Please disregard.
>>>>
>>>> BR,
>>>> Nikos
>>>> ----- Original Message ----- From: "Nikos Balkanas" 
>>>> <[email protected]>
>>>> To: <[email protected]>
>>>> Sent: Thursday, August 26, 2010 8:02 AM
>>>> Subject: Patch: gw/urltrans.c
>>>>
>>>>
>>>>> Hi,
>>>>>
>>>>> Currently keyword-regex is configured to only do exact case matches. 
>>>>> This
>>>>> is
>>>>> in contrast to keyword matching, which is case incensitive. 
>>>>> Additionaly,
>>>>> input string input string is converted to lower case for matching. 
>>>>> This
>>>>> will
>>>>> cause all keyword-regex patterns with capital letters to fail.
>>>>>
>>>>> This patch corrects that by inittializing keyword-regex to case
>>>>> incensitive
>>>>> matching.
>>>>> Reported by Mike Cariotoglou
>>>>>
>>>>> BR,
>>>>> Nikos
>>>>>
>>>>
>>>>
>>>>
>>>
>>>
>> <urltrans.diff>
>

Index: gw/urltrans.c
===================================================================
--- gw/urltrans.c	(revision 4837)
+++ gw/urltrans.c	(working copy)
@@ -868,7 +868,7 @@
     Octstr *url, *post_url, *post_xml, *text, *file, *exec;
     Octstr *accepted_smsc, *accepted_account, *forced_smsc, *default_smsc;
     Octstr *grpname;
-    int is_sms_service;
+    int is_sms_service, regex_flag = REG_EXTENDED;
     Octstr *accepted_smsc_regex;
     Octstr *accepted_account_regex;
     Octstr *allowed_prefix_regex;
@@ -970,7 +970,7 @@
 	    Octstr *aliases;
 	    
 	    /* convert to regex */
-            octstr_convert_range(tmp, 0, octstr_len(tmp), tolower);
+        regex_flag |= REG_ICASE;
 	    keyword_regex = octstr_format("^[ ]*(%S", tmp);
 	    octstr_destroy(tmp);
 
@@ -984,7 +984,6 @@
 	        
 	        for (i = 0; i < gwlist_len(l); ++i) {
 	            os = gwlist_get(l, i);
-	            octstr_convert_range(os, 0, octstr_len(os), tolower);
 	            octstr_format_append(keyword_regex, "|%S", os);
 	        }
 	        gwlist_destroy(l, octstr_destroy_item);
@@ -993,7 +992,7 @@
 	    octstr_append_cstr(keyword_regex, ")[ ]*");
 	}
 
-        if (keyword_regex != NULL && (ot->keyword_regex = gw_regex_comp(keyword_regex, REG_EXTENDED)) == NULL) {
+        if (keyword_regex != NULL && (ot->keyword_regex = gw_regex_comp(keyword_regex, regex_flag)) == NULL) {
             error(0, "Could not compile pattern '%s'", octstr_get_cstr(keyword_regex));
             octstr_destroy(keyword_regex);
             goto error;
@@ -1353,11 +1352,12 @@
         if (gw_regex_match_pre(t->keyword_regex, msg) == 1) {
             debug("", 0, "match found: %s", octstr_get_cstr(t->name));
             gwlist_append(list, t);
-        } else {
-            debug("", 0, "no match found: %s", octstr_get_cstr(t->name));
         }
     }
-    
+
+    if (!gwlist_len(list))
+       debug("", 0, "no match found: \"%s\"", octstr_get_cstr(msg));
+
     return list;
 }
 
@@ -1373,7 +1373,6 @@
 
     /* convert tolower and try to match */
     data = octstr_duplicate(msg->sms.msgdata);
-    octstr_convert_range(data, 0, octstr_len(data), tolower);
     i = 0;
     while((i = octstr_search_char(data, 0, i)) != -1 && i < octstr_len(data) - 1) {
         octstr_delete(data, i, 1);
Index: doc/userguide/userguide.xml
===================================================================
--- doc/userguide/userguide.xml	(revision 4837)
+++ doc/userguide/userguide.xml	(working copy)
@@ -6032,17 +6032,16 @@
          number of `%s' fields in the URL. This allows you to
         configure the gateway to use different URLs for the same
         keyword depending on the number of words the SMS message
-        contains. The keyword matches in non-case sensitive manner,
-		  which means you don't have to use aliases to handle different
-		  cased versions of your keyword.
+        contains. The keyword is case insensitive, which means you don't
+        have to use aliases to handle different cased versions of your keyword.
      </entry></row>
 
    <row><entry><literal>keyword-regex</literal></entry>
      <entry>POSIX regular expression</entry>
      <entry valign="bottom">
      This field may be used to enable service-selection based on a regular expression.
-     If this field is defined for a service, then the selection will rely on the regex only, never taking
-     the literal <literal>keyword</literal> into account.
+     You can define either keyword or keyword-regex in configuration, but not both in
+     the same sms-service. keyword-regex is case sensitive.
      See section on <xref linkend="regular-expressions"> for details.
    </entry></row>
 
@@ -6050,7 +6049,8 @@
      <entry>word-list</entry>
      <entry valign="bottom">
         If the service has aliases, they are listed as a list with 
-        each entry separated with a semicolon (';')
+        each entry separated with a semicolon (';'). Aliases are case
+        insensitive just like keyword.
 
      </entry></row>