[scponly] getopt problem with long options

[Chris Fensch]

Hi,

  I just wanted to let you guys know that after I upgraded from 4.1 to 4.3 
(I missed the 4.2 release), I experienced some problems with rsync:

Jan 16 08:01:05 XXX scponly[22052]: option e is not permitted for use 
with /usr/bin/rsync (arg was rver)(username: XXX(XXX), IP/port: 
::ffff:XXX.XXX.XXX.XXX 25942 22))
Jan 16 08:01:05 XXX scponly[22052]: requested command (/usr/bin/rsync
--server -logDtprz --delete . /backup/subversion/) tried to use disallowed
argument (username: XXX(XXX), IP/port: ::ffff:XXX.XXX.XXX.XXX 25942 22))

As you can see the command was not dangerous at all. However, the 
long argument '--server' gets parsed by getopt as:
'-' -> invalid/ignore
's' -> invalid/ignore
'e' -> valid 'e', argument 'rver'

For some reason the version of getopt on my system Linux / glibc 2.3.3, 
cannot probably deal with long arguments (starting with a double dash 
'--').

I found a solution, by using getopt_long instead of getopt. I don't know 
if this is available on all system. In order to use, you simply have to 
replace the "while ((ch = getopt ..." call in helper.c with:

struct option long_opt[] = {
        {0, 0, 0, 0}
};

while ((ch = getopt_long(ac, av, cmdarg->opts, long_opt, NULL)) != -1)

The empty long_opt structure is necessary, otherwise you will get the same 
broken behaviour regarding long options again. Also, in order for this to 
compile, you have to include getopt.h, which isn't because, it is guarded 
by #ifdef HAVE_GETOPT_H. HAVE_GETOPT_H will be defined in config.h, which 
is included later. So you have to change the order of includes here.

Oh, and some notes on the "optreset" issue. It seems to me that with 
glibc, setting "optind" to 1 seems enough to reset getopt or getopt_long. 
According to the documentation in the getopt.c glibc source:

/* Index in ARGV of the next element to be scanned.
   This is used for communication to and from the caller
   and for communication between successive calls to `getopt'.

   On entry to `getopt', zero means this is the first call; initialize.
   
   When `getopt' returns -1, this is the index of the first of the
   non-option elements that the caller should itself scan.
   
   Otherwise, `optind' communicates from one call to the next
   how much of ARGV has been scanned so far.  */

/* 1003.2 says this must be 1 before any call.  */
int optind = 1;

As far as I understand the source code, it will also initialise (even if 
optind == 1), if it is not initialised sofar. But, if you set optind = 0, 
then it will run a reinitialisation.

Best Regards and I hope someone might find this useful

  Chris