On Wed, Apr 18, 2018 at 08:09:57AM +0200, Aurélien Nephtali wrote: > Hello Willy, > > On Wed, Apr 18, 2018 at 07:38:24AM +0200, Willy Tarreau wrote: > > Hi Aurélien, > > > > On Tue, Apr 17, 2018 at 09:06:35PM +0200, Aurélien Nephtali wrote: > > > Example: > > > If this command is entered: > > > set server bk/fe << > > > state ready > > > > > > Three arguments and a payload will be tagged. This command requires (in > > > some cases) seven arguments (including the keywords) so a call to > > > cli_resync_args(ci, 7) will extract what can be found from the payload. > > > Once a parser has done everything it can to ensure it has access to its > > > arguments, it can call cli_split_args() to do the proper tokenization > > > and modify the input string to insert null bytes. > > > > I'm confused, I thought we agreed *not* to mix arguments and payload, like > > in the map example I provided, but now you seem to imply that extra args > > are looked up in the payload part, while here you seem to have stuck to > > this design, and your "add map" example seems to do exactly the case which > > makes it useless (ie no way to enter spaces in the value because of the > > undesired tokenizing). > > > > 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. > 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. > > 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. Thanks, Willy
