nnrpd's Argify() and passwords with spaces

[Russ Allbery]

Jeffrey M Vinocur <jeff at marduk.litech.org> writes:

> The problem stems from the fact that once we've called Argify on the
> input, the whitespace is lost.  The only clean solution we could come up
> with is to only Argify the first n arguments (where n is whatever is
> needed) and thus the remainder would be available for individual
> command-processing functions to use as they see fit.

One thing that you may want to look at if you're rewriting code in this
area is the new vector interface I added to libinn in CURRENT.  My
intention was to eventually replace all the Argify stuff in the source
base with that; the API is a bit more thorough and complete and there's a
test suite for it.

I don't think there are any current split functions for vectors that match
this (split out the first n), but that's obviously generally useful and it
should be reasonably straightforward to add.

> Easily fixed, but for one problem:  if there was leading whitespace,
> argv[0] won't point to the beginning of the malloc'd chunk, and
> attempting to free it would be bad.  It's fixable, of course, but
> requires some thought.  So unless someone has evidence that command
> parsing is a bottleneck...

I doubt that it is, particularly for nnrpd which has to go to disk to do
almost anything anyway.

> and add the following two functions (after which Argify can be defined
> as a wrapper around nArgify with n=-1 for backwards compatibility):

Yeah, if you're going to this sort of work, definitely take a look at
vector (the interface is in include/inn/vector.h).  I think the new code
should be simpler and easier to work with.

> So, two implementation questions:

> - It's pretty clear that leading whitespace should be removed,
>   but what about trailing whitespace?  That is, should a command
>   like "AUTHINFO PASS    foo  bar " be read as a password of 
>   "foo  bar" or "foo  bar "?

>   (In the absence of any strong opinions, the latter is easier to
>   implement.)

I think we may as well support trailing spaces if it's easier.

> - There are two possible behaviors for nArgify/reArgify when
>   there are more than n arguments (and we leave the remainder of
>   the input as argv[n]):

>   * Return n+1.  This has the benefit of making checking for too
>     many arguments very easy, however there is that odd ac-- that
>     has to be done before calls to reArgify (and if there are
>     fewer than n arguments in the first place, later calls to
>     reArgify will walk over the last argument again for no
>     reason, because of the ac--).

>   * Return n.  This conceptually makes more sense, I think, and
>     removes the ac--, but it means that code like (ac > max) has
>     to be rewritten as (ac > max || (ac == max && av[ac] != NULL)).

>   Any thoughts on which is less ugly?  If you were writing code
>   that used these functions, which would you rather see?  Would
>   the latter be improved by having a function (or macro) to
>   encapsulate the check?

I think the vector interface may force a somewhat different decision here,
since you really want to return a new vector.  That argues for doing the
latter, since the number of items you split things into is already stored
in the vector as the vector length.

-- 
Russ Allbery (rra at stanford.edu)             <http://www.eyrie.org/~eagle/>