[Home]

Summary:ASTERISK-10283: [patch] Asterisk case sensitive problem.
Reporter:Eliel Sardanons (eliel)Labels:
Date Opened:2007-09-11 19:20:55Date Closed:2007-11-07 12:59:08.000-0600
Priority:TrivialRegression?No
Status:Closed/CompleteComponents:Applications/app_queue
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 10700.patch
( 1) app_queue.c.patch
Description:I think all asterisk should be case insensitive cause I have seen this problem in too many modules:
The config (in this case queues.conf) is loaded case sensitive, but the CLI is case insensitive so we have a problem like this:
We have 2 queues named: Prueba and prueba they are loaded in memory case sensitive:

eliel-laptop*CLI> queue show
Prueba       has 0 calls (max unlimited) in 'ringall' strategy (0s holdtime), W:0, C:0, A:0, SL:0.0% within 0s
  Members:
     sip/eliel (Unavailable) has taken no calls yet
  No Callers
eliel-laptop*CLI>
prueba       has 0 calls (max unlimited) in 'ringall' strategy (0s holdtime), W:0, C:0, A:0, SL:0.0% within 0s
  Members:
     Sip/eliel (Unavailable) has taken no calls yet
  No Callers

But when I try to get info from an especific queue like this:

eliel-laptop*CLI> queue show prueba
Prueba       has 0 calls (max unlimited) in 'ringall' strategy (0s holdtime), W:0, C:0, A:0, SL:0.0% within 0s
  Members:
     sip/eliel (Unavailable) has taken no calls yet
  No Callers

eliel-laptop*CLI> queue show Prueba
Prueba       has 0 calls (max unlimited) in 'ringall' strategy (0s holdtime), W:0, C:0, A:0, SL:0.0% within 0s
  Members:
     sip/eliel (Unavailable) has taken no calls yet
  No Callers

I always get one of the two queues info... I did a patch to make the autocomplete of queue_show case insensitive but we are loosing some of the great 'autocomplete' features like doing a:
'queue show PrUeBa' :-).

This problem is found again in chan_sip.c and other modules, my patch I think shouldn't be applied, I think asterisk should load the config in a case insensitive manner and don't allow to load sip peers like ELIEL and eliel or queues like Prueba and prueba (discarding the lastone).

****** ADDITIONAL INFORMATION ******

My patch solve the queue problem like this:

------------  BEFORE:
eliel-laptop*CLI> queue show prueba
Prueba       has 0 calls (max unlimited) in 'ringall' strategy (0s holdtime), W:0, C:0, A:0, SL:0.0% within 0s
  Members:
     sip/eliel (Unavailable) has taken no calls yet
  No Callers

eliel-laptop*CLI> queue show Prueba
Prueba       has 0 calls (max unlimited) in 'ringall' strategy (0s holdtime), W:0, C:0, A:0, SL:0.0% within 0s
  Members:
     sip/eliel (Unavailable) has taken no calls yet
  No Callers

--------------- AFTER:
eliel-laptop*CLI> queue show Prueba
Prueba       has 0 calls (max unlimited) in 'ringall' strategy (0s holdtime), W:0, C:0, A:0, SL:0.0% within 0s
  Members:
     sip/eliel (Unavailable) has taken no calls yet
  No Callers

eliel-laptop*CLI> queue show prueba
prueba       has 0 calls (max unlimited) in 'ringall' strategy (0s holdtime), W:0, C:0, A:0, SL:0.0% within 0s
  Members:
     Sip/eliel (Unavailable) has taken no calls yet
  No Callers


Comments:By: Russell Bryant (russell) 2007-09-12 15:15:35

Your comments say that you want to make it case _insensitive_ everywhere, which I would agree with.  However, your patch appears to make a change for the opposite.
By: jmls (jmls) 2007-09-12 15:46:54

please make it case insensitive.

While we're at it, can we also make the dialplan case insensitive ?

;)
By: Mark Michelson (mmichelson) 2007-09-12 16:16:51

russell:

"This problem is found again in chan_sip.c and other modules, my patch I think shouldn't be applied, I think asterisk should load the config in a case insensitive manner and don't allow to load sip peers like ELIEL and eliel or queues like Prueba and prueba (discarding the lastone)."

He's saying his patch fixes his immediate problem but that the better fix would be to make the queues themselves case insensitive since the CLI is case insensitive. I'll post a patch soon which does this, at least for queues anyway.
By: Eliel Sardanons (eliel) 2007-09-12 16:49:32

putnopvut understand my poor english, my patch does not solve the "big" problem here that needs a major rework of many load_configs on modules. If you agree to make asterisk case insensitive I will start working with the other modules.
By: Mark Michelson (mmichelson) 2007-09-12 17:15:56

I've uploaded a patch (10700.patch) for app_queue which will make queues case insensitive in comparisons.

The way it works is if there are two queues, "qUEUe" and "QueUe" then it will only load into memory the first it comes across in the config file and give a warning when it sees the second saying that it will be skipped.

I also did some brief testing using realtime queues as well. Let me know if there are any problems.
By: Russell Bryant (russell) 2007-09-12 17:57:08

I'm not really sure which way is a better way to go.  As you have pointed out, most places in Asterisk like this are case sensitive.  The reason is simply performance of string comparisons.

Perhaps we should talk about this on the -dev list.  The question is whether we should move toward not being case sensitive across most areas of Asterisk, or do we stay with how it is being case sensitive.

Either way, it's important to be consistent ...
By: Mark Michelson (mmichelson) 2007-09-13 11:13:31

I realized after writing 10700.patch that part of it should be implemented in 1.4 and trunk no matter if we move to case-insensitive queues or not. That is, Asterisk should check if multiple queues have the same name and warn the user that subsequent queues with that name will be skipped. As such, I have committed part of 10700.patch to 1.4 and trunk already. I just left out the strcasecmp changes.

The change to 1.4 is in revision 82326 and the change to trunk is in revision 82327.
By: Eliel Sardanons (eliel) 2007-09-17 12:32:40

putnopvut: Why you say that 'it should be implemented in 1.4 and trunk no matter if we move to case-insensitive queues or not', queues case sensitive are loaded in a correct manner in asterisk, and the dialplan application is using the correct queue while calling it with one ('test') or the other ('Test').
The only problem I see is on the CLI commands.
By: Mark Michelson (mmichelson) 2007-09-17 13:50:35

Sorry. I wasn't clear.

What I meant is if someone defines two queues with the exact same name, "test" and "test" then there is no warning given to the user that the two queues have the same name. Furthermore, the way Asterisk handled this condition was to replace previously defined queues with newer ones as it parsed the config file. This could lead to some confusion if a user accidentally defined two queues with duplicate names.

All I've done is made Asterisk print a warning that it has come across a queue with a duplicate name and warn the user that it is going to skip this queue. This comparison is case-sensitive.
By: Tilghman Lesher (tilghman) 2007-10-23 11:22:36

putnopvut:  in your last note, you say that the comparison is case-sensitive, but the patch you posted is case-INsensitive.

While I agree with the warning, I think we need to keep the queue name case-sensitive.  The problem the reporter is experiencing is due to the fact that MySQL is by default case-insensitive on string comparisons.
By: Eliel Sardanons (eliel) 2007-10-23 11:42:58

Corydon76, not only MySQL is case insensitive, also the CLI is case insensitive in string comparisons (and autocomplete), that is the big problem here, because if we continue loading queues, peer, etc in a sensitive manner then "test" and "Test" will be different and a "queue show test" or "queue show Test" will reply with the same queue (the first loaded on the list). That is what putnopvut is trying to avoid.
By: Eliel Sardanons (eliel) 2007-10-23 11:48:22

I agree that asterisk should continue comparing in a case sensitive manner, but we could filter duplicate entries in a case insensitive way to prevent this situations.
By: Eliel Sardanons (eliel) 2007-10-23 11:50:18

sorry I miss this: filter duplicates while loading config files.
By: Tilghman Lesher (tilghman) 2007-11-07 12:59:08.000-0600

I think this issue is pretty much dead, now.  We are not going to make the queue names case-insensitive.  The warning from the patch has already been added to trunk.  Closing.