[chrony-users] libedit 20180525-3.1 escapes spaces in chronyc tab-completion

[Lonnie Abelbeck]

Hi,

Our project did a version bump of libedit to 20180525-3.1 and noticed spaces are now being escaped with a backslash with chronyc's tab-completion.

Example:
--
chronyc> sources<TAB>
sources sources -v sourcestats sourcestats -v

chronyc> sources<SPACE><TAB>
chronyc> sources\ -v <RETURN>
Unrecognized command
--

The change occurred with libedit revision 1.47 2017/10/15
http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libedit/filecomplete.c.diff?r1=1.46&r2=1.47

I took a stab at a fix by patching CPS_NormalizeLine() ...

--- chrony-3.3/cmdparse.c.orig 2018-06-08 17:07:48.644094438 -0500
+++ chrony-3.3/cmdparse.c 2018-06-08 17:28:30.578778580 -0500
@@ -218,7 +218,8 @@
if (first && strchr("!;#%", *p))
break;

- *q++ = *p;
+ if (!(*p == '\\' && isspace((unsigned char)*(p+1))))
+ *q++ = *p;
space = first = 0;
}

and it solves the issue with limited testing.

(BTW, applying De Morgan's law to the added logic might make it less readable, so I left it as shown.)

Lonnie

[Miroslav Lichvar]

Post by Lonnie Abelbeck
Hi,
Our project did a version bump of libedit to 20180525-3.1 and noticed spaces are now being escaped with a backslash with chronyc's tab-completion.

Interesting.

Post by Lonnie Abelbeck
The change occurred with libedit revision 1.47 2017/10/15
http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libedit/filecomplete.c.diff?r1=1.46&r2=1.47
I took a stab at a fix by patching CPS_NormalizeLine() ...

I think the fix should be closer to the readline/editline-specific
code. Maybe there is an option to disable the new behavior?

[Lonnie Abelbeck]

Post by Miroslav Lichvar

Post by Lonnie Abelbeck
Hi,
Our project did a version bump of libedit to 20180525-3.1 and noticed spaces are now being escaped with a backslash with chronyc's tab-completion.

Interesting.

Post by Lonnie Abelbeck
The change occurred with libedit revision 1.47 2017/10/15
http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libedit/filecomplete.c.diff?r1=1.46&r2=1.47
I took a stab at a fix by patching CPS_NormalizeLine() ...

I think the fix should be closer to the readline/editline-specific
code.

I agree, if possible.

Post by Miroslav Lichvar
Maybe there is an option to disable the new behavior?

I looked, and don't see an editline global option for this new feature.

In chronyc, the minimal code added for tab-completion is leveraged by using the filecomplete.c routines.

The question is, for chronyc should '\ ' input be handled as a ' ' or present an error. Possibly a '\ ' could occur in other external ways ?

Lonnie

[Miroslav Lichvar]

Post by Lonnie Abelbeck

Post by Miroslav Lichvar
Maybe there is an option to disable the new behavior?

I looked, and don't see an editline global option for this new feature.

I think it's strange that it does that with editline, but not
readline. Isn't editline supposed to be compatible with readline?

Maybe the multiword commands (e.g. the -v options) should be handled
in some other way? Or is that too much work?

Post by Lonnie Abelbeck
In chronyc, the minimal code added for tab-completion is leveraged by using the filecomplete.c routines.
The question is, for chronyc should '\ ' input be handled as a ' ' or present an error. Possibly a '\ ' could occur in other external ways ?

I'd prefer to not support any escaping, at least until we really need
it (I currently don't see why we would).

[Lonnie Abelbeck]

Post by Miroslav Lichvar

Post by Lonnie Abelbeck

Post by Miroslav Lichvar
Maybe there is an option to disable the new behavior?

I looked, and don't see an editline global option for this new feature.

I think it's strange that it does that with editline, but not
readline. Isn't editline supposed to be compatible with readline?

My understanding is editline is (was) a "compatible" subset of readline. Looking at the readline docs ...

http://www.delorie.com/gnu/docs/readline/rlman_47.html

There are optional rl_filename_quoting_function and rl_filename_quote_characters which defaults to the null string, no quoting. This latest libedit 20180525-3.1 seems to not be compatible with these quoting options.

Post by Miroslav Lichvar
Maybe the multiword commands (e.g. the -v options) should be handled
in some other way? Or is that too much work?

I'm not certain how that would be done, first we would add <SPACE> to the break characters:
--
- rl_basic_word_break_characters = "\t\n\r";
+ rl_basic_word_break_characters = " \t\n\r";
--
then possibly the command_name_generator() would use a 2 dimensional array to match ... not sure ...

Alternatively, I used this patch below to ignore any backslash quoting, almost perfect except the stdout still contains a '\ ', while internally the backslash is ignored.

Lonnie

--- chrony-3.3/client.c.orig 2018-06-12 13:29:58.335070738 -0500
+++ chrony-3.3/client.c 2018-06-12 13:48:26.650116681 -0500
@@ -109,6 +109,7 @@
if (on_terminal) {
#ifdef FEAT_READLINE
char *cmd;
+ char *p, *q;

rl_attempted_completion_function = command_name_completion;
rl_basic_word_break_characters = "\t\n\r";
@@ -119,6 +120,13 @@

/* user pressed return */
if( *cmd != '\0' ) {
+ /* strip any backslash quoting */
+ for (p = q = cmd; *p; p++) {
+ if (*p != '\\') {
+ *q++ = *p;
+ }
+ }
+ *q = '\0';
strncpy(line, cmd, sizeof(line) - 1);
line[sizeof(line) - 1] = '\0';
add_history(cmd);

[Lonnie Abelbeck]

Post by Miroslav Lichvar

Post by Lonnie Abelbeck

Post by Miroslav Lichvar
Maybe there is an option to disable the new behavior?

I looked, and don't see an editline global option for this new feature.

I think it's strange that it does that with editline, but not
readline. Isn't editline supposed to be compatible with readline?
Maybe the multiword commands (e.g. the -v options) should be handled
in some other way? Or is that too much work?

A complete fix is submitted to chrony-dev.

Lonnie