Willy,
On Wed, Apr 18, 2018 at 08:36:05AM +0200, Willy Tarreau wrote:
> > The problem is: the main parser (cli_parse_request()) does not have any
> > idea of what is an argument and how many there should be so it can't
> > tell if the command syntax is valid or not before passing it to the
> > commands parser.
>
> OK, I thought it was tokenized in the cli parser.
The tokenization is done in cli_split_args() which is called by parsers
WHEN they think they have all they need (after they called
cli_resync_args()).
>
> > What I did is allow the user to use << where he wants
> > and then the commands parsers will do their jobs and if they need more
> > data they will look for them in what is called 'payload'.
>
> OK
>
> > I guess your
> > point was to use << as a strict delimiter of arguments and payload.
>
> Not necessarily. By the way, shell supports <<EOF anywhere on the line.
> My worries come from your example above which splits the "set server"
> command and some of you comments which seem to imply that the arguments
> are looked up in the payload part.
Well, they are looked up in the payload part but only if the command
parser is missing its mandadory ones.
Let's use the "set server" keywords as an example. This command does not
usually takes a payload but it's what I wanted to add: "multi-lines for
everyone". But maybe the real point should be to add "payload for some" ?
Here would be valid ways of using "set server":
# set server <<
bk/fe state ready
# set server bk/fe state ready
# set server bk/fe <<
state
ready
# set server <<
bk/fe
state
ready
The parser will first call cli_resync_args().
What that will do is consume the payload in order to reach
its number of mandatory arguments. Internally the list of arguments will
become: 'set' 'server 'bk/fe' 'state' 'ready' and the payload will be
set to NULL. Then the parser will request tokenization (through
cli_split_args()) and do its job.
There is no "real" split between arguments and payload. It will be
command parsers that will decide what is the final payload according to
what they need.
It could feel convoluted but that's the way I found to:
- add multi-lines support to *everything* without knowing wether a command
supports multi-lines or how many arguments it needs (optional or
mandatory)
- being able to use << where the user wants
> > > Then we can indeed improve ocsp-response, map/acl to support taking
> > > entries
> > > either as arguments or as payload or both. In fact I think that there is
> > > no
> > > need for doing something very different from what we see in shell already
> > > :
> > > the command's args are argv[0..N] and the payload is in stdin. Then many
> > > commands decide where to pick their data from
> >
> > Yes, that is what I did but I also allowed the << pattern to be loosely
> > placed at the user conveniance.
>
> OK then that's fine. It's just not the first impression I got when reading
> it, and the commands you changed seemed to continue to use args even for
> the payload part. Please keep in mind the example I proposed regarding
> "add map" with HTTP status codes and reasons, to be sure that once your
> change is applied, it finally becomes possible to add "404 Not Found" as
> a payload where only the "add map" command decides how to parse that line
> (ie: decide that 404 is the key and the rest is the value, which implies that
> it's *not* tokenized). If this works easily, I'm quite confident we'll be on
> a good track.
>
Well, it already does that :) :
$ cat tst3 | socat /tmp/sock1 -
$ socat /tmp/sock1 -
show map #-1
0x7e6ad0 test.com bk
0x7f5260 200 OK
0x7f52a0 203 Non-Authoritative Information
0x82e3b0 204 No Content
0x82e4b0 301 Moved Permanently
0x82e5b0 302 Found
0x82e6b0 303 See Other
0x82e7b0 400 Bad Request
$ cat tst3
add map <<
#-1
100 Continue
200 OK
203 Non-Authoritative Information
204 No Content
301 Moved Permanently
302 Found
303 See Other
400 Bad Request
$
As you can see, I purposedly set a mandatory argument after the << to be
sure we're on the same page regarding what I did with the command parser
looking into the "payload" (I call payload everything that is after the
<<) and to wether or not this is the way to go :).
--
Aurélien.
