cyrus.conf and non-alphanumeric service names

[Florian G. Pflug]

Hi

I've stumbled across the following strange behaviour / bug in cyrus.conf 
parsing today. If you define a service containing non-isalnum (but also 
non-isspace) characters, the service name is silently cut off at the 
first non-isalum characters, due to this code fragment in 
masterconf.c:process_section()

for (q = p; isalnum((int) *q); q++) ;
if (q) { *q = '\0'; q++; }

This causes the two lines:
imap-domain.org  cmd=.... listen="ip1:imap"
imap-otherdomain.org cmd=... listen="ip2:imap"
to defined *two* services named "imap"

This would be only mildly annoying if the duplicate-servicename check in 
master/master.c detected the situation and ingored the second service -
but it doesn't :-(

------------------------------------------------------------
/* see if we have an existing entry that can be reused */
for (i = 0; i < nservices; i++) {
     /* skip non-primary instances */
     if (Services[i].associate > 0)
         continue;
     /* must have empty/same service name, listen and proto */
     if ((!Services[i].name || !strcmp(Services[i].name, name)) &&
         (!Services[i].listen || !strcmp(Services[i].listen, listen)) &&
         (!Services[i].proto || !strcmp(Services[i].proto, proto)))
         break;
}

/* we have duplicate service names in the config file */
if ((i < nservices) && Services[i].exec) {
     ....
------------------------------------------------------------
AFAICT that code actually checks for duplicate service *definitions* 
(where the name, the listen and the proto are the same), not for 
duplicate service *names* - contrary to what the comments say.

Even that might be tolerable, where it not that the lockfiles in socket/ 
contain only the service name, and the id in their filename. This 
finally breaks things badly, since now I have two services running (On 
two IPs) with use the *same* lockfile in socket/. Meaning that accept() 
is only run on *one* of these socket at a given time...

I'm not exactly sure what the correct fix for this is - I don't know if 
keeping service names unique really *is* important. Maybe it's not, and 
the "listen" should just be added to the lockfile name.

regards, Florian Pflug