ungleich-public/cdist

Fork 10

Invalid remote code #317

New issue

Closed

opened 2021-11-20 15:24:15 +00:00 by ungleich-gitea · 6 comments

ungleich-gitea commented

2021-11-20 15:24:15 +00:00

Owner

Created by: pestaa

Hi,

Thanks for the great project!

Unfortunately I quickly run into this problem:

sed: -I or -i may not be used with stdin

The problem is caused by this command:

__key_value varnishd_enable --file /etc/rc.conf --delimiter '=' --value "YES"

I'm running FreeBSD 8.2-STABLE on the remote machine. The generated code-remote is what you'd expect:

sed -i "s|^varnishd_enable\(=\+\).*|varnishd_enable\1YES|" "/etc/rc.conf"

I believe the issue is due to improper argumenting. The -i flag gets the script, and the filename becomes the scripts, and no input file remains.

I tried to pass an empty string or reorder the arguments, but my shell-fu failed me. Please advice.

Thank you very much in advance.

*Created by: pestaa* Hi, Thanks for the great project! Unfortunately I quickly run into this problem: ``` sed: -I or -i may not be used with stdin ``` The problem is caused by this command: ``` __key_value varnishd_enable --file /etc/rc.conf --delimiter '=' --value "YES" ``` I'm running FreeBSD 8.2-STABLE on the remote machine. The generated `code-remote` is what you'd expect: ``` sed -i "s|^varnishd_enable$=\+$.*|varnishd_enable\1YES|" "/etc/rc.conf" ``` I believe the issue is due to improper argumenting. The `-i` flag gets the script, and the filename becomes the scripts, and no input file remains. I tried to pass an empty string or reorder the arguments, but my shell-fu failed me. Please advice. Thank you very much in advance.

ungleich-gitea closed this issue

2021-11-20 15:24:15 +00:00

ungleich-gitea referenced this issue

2021-11-20 15:48:17 +00:00

__key_value does not handle keys that contain a $ character #213

ungleich-gitea commented

2021-11-20 15:53:12 +00:00

Author

Owner

Created by: pestaa

Thanks Jake for the additional info!

I agree that the backup extension should be provided. However, I don't think backing up should slow the process down. The current behavior does not even create a backup file to begin with.

(Backup gets even more complicated when you consider multiple __key_value commands -- each time a new backup is created, so you need even more workaround.)

A really quick search gives 3 types that use sed -i: key_value, rvm and start_on_boot.

*Created by: pestaa* Thanks Jake for the additional info! I agree that the backup extension should be provided. However, I don't think backing up should slow the process down. The current behavior does not even create a backup file to begin with. (Backup gets even more complicated when you consider multiple __key_value commands -- each time a new backup is created, so you need even more workaround.) A really quick search gives 3 types that use `sed -i`: `key_value`, `rvm` and `start_on_boot`.

ungleich-gitea commented

2021-11-20 15:53:13 +00:00

Author

Owner

Created by: telmich

Hey Jake,

thanks for clearifying!

I guess we can change to create the backup file and rm -f it afterwards.

We could even add a parameter named "keepbackup" to the types
using "sed -i" to leave the files in place.

OR (perhaps even better):

finally implement boolean parameter (on the road^Wtodo list)
add --backup boolean parameter to types manipulating files
use .cdist-backup suffix for files we changed

Cheers,

Nico

Want commercial support for cdist?
Have a look at http://www.ungleich.ch/offers/cdist-support/

*Created by: telmich* Hey Jake, thanks for clearifying! I guess we can change to create the backup file and rm -f it afterwards. We could even add a parameter named "keepbackup" to the types using "sed -i" to leave the files in place. OR (perhaps even better): - finally implement boolean parameter (on the road^Wtodo list) - add --backup boolean parameter to types manipulating files - use .cdist-backup suffix for files we changed Cheers, Nico ## Want commercial support for cdist? Have a look at http://www.ungleich.ch/offers/cdist-support/

ungleich-gitea commented

2021-11-20 15:53:15 +00:00

Author

Owner

Created by: jdguffey

After looking at GNU sed's documentation (http://www.gnu.org/software/sed/manual/sed.html), I see a minor but very important difference in how the "-i" flag is handled.

FreeBSD:
If in-place editing is to be used, a backup file extension is required.
Usage looks like: sed -i .bak /regex/ file, so after execution of sed, file and file.bak exist.

GNU:
If in-place editing is to be used, a backup file extension is allowed.
Usage can be: sed -i /regex/ file or sed -i .bak /regex/ file.
In the first case, file is simply modified in-place. In the second case, GNU and FreeBSD sed have the same behavior.

*Created by: jdguffey* After looking at GNU sed's documentation (http://www.gnu.org/software/sed/manual/sed.html), I see a minor but very important difference in how the "-i" flag is handled. FreeBSD: If in-place editing is to be used, a backup file extension is _required_. Usage looks like: sed -i .bak /regex/ file, so after execution of sed, file and file.bak exist. GNU: If in-place editing is to be used, a backup file extension is _allowed_. Usage can be: sed -i /regex/ file _or_ sed -i .bak /regex/ file. In the first case, file is simply modified in-place. In the second case, GNU and FreeBSD sed have the same behavior.

ungleich-gitea commented

2021-11-20 15:53:16 +00:00

Author

Owner

Created by: telmich

Well, one could introduce a "--sed" parameter, but that would have to be repeated in other types as well. Not sure what's the cleanest solution here. Using a tmpfile is a workaround, though not very beautiful.

I'll close this issue for now, but feel free to reopen if, if you've new ideas or findings.

*Created by: telmich* Well, one could introduce a "--sed" parameter, but that would have to be repeated in other types as well. Not sure what's the cleanest solution here. Using a tmpfile is a workaround, though not very beautiful. I'll close this issue for now, but feel free to reopen if, if you've new ideas or findings.

ungleich-gitea commented

2021-11-20 15:53:17 +00:00

Author

Owner

Created by: pestaa

FreeBSD comes with a non-GNU version of sed, version number being approximately v1.41.2.1.2.1 (yeah, found in the sources).

Anyway, installing the GNU sed (named gsed in ports) solved the problem. Of course sed needs to be symlinked to gsed in this scenario. I think I'll have cdist do that! :)

(Note to FreeBSD admins: the default sed supports some kind of GNU-compatilibity switch, but it has no effect on the command line.)

Thanks @telmich for your prompt response, really appreciated.

I'm not sure how Steve could approach this problem. Maybe replace the sed command with something more cross-platform?

*Created by: pestaa* FreeBSD comes with a non-GNU version of sed, version number being approximately v1.41.2.1.2.1 (yeah, found in the sources). Anyway, installing the GNU sed (named `gsed` in ports) solved the problem. Of course `sed` needs to be symlinked to `gsed` in this scenario. I think I'll have cdist do that! :) (Note to FreeBSD admins: the default sed supports some kind of GNU-compatilibity switch, but it has no effect on the command line.) Thanks @telmich for your prompt response, really appreciated. I'm not sure how Steve could approach this problem. Maybe replace the sed command with something more cross-platform?

ungleich-gitea commented

2021-11-20 15:53:18 +00:00

Author

Owner

Created by: telmich

Hey Istvan,

Istvan Beregszaszi [Wed, Apr 04, 2012 at 04:55:45AM -0700]:

Thanks for the great project!

thanks for the good feedback! (*)

Unfortunately I quickly run into this problem:
sed: -I or -i may not be used with stdin
The problem is caused by this command:
__key_value varnishd_enable --file /etc/rc.conf --delimiter '=' --value "YES"
I'm running FreeBSD 8.2-STABLE on the remote machine. The generated code-remote is what you'd expect:

Yeah, nice fbsd! Long time ago I used to use it though...

sed -i "s|^varnishd_enable\(=\+\).*|varnishd_enable\1YES|" "/etc/rc.conf"

Can you check your actual sed version? It may be that GNU sed and
behaves differently.

Not sure why sed detects use of stdin. I'd suggest to look at the
source of the sed you are using and see how it detects that.

I believe the issue is due to improper argumenting. The -i flag gets the script, and the filename becomes the scripts, and no input file remains.

Alright, that may be possible.

If I have a look at my sed manpage, it looks like this:

sed [OPTION]... {script-only-if-no-other-script} [input-file]...

Using this sed:

[15:04] brief:% pacman -Qo $(which sed)
/bin/sed is owned by sed 4.2.1-4
[15:04] brief:% pacman -Qi sed | head -n 3
Name : sed
Version : 4.2.1-4
URL : http://www.gnu.org/software/sed

I've told Steven (the author of __key_value) about it, but we cannot
reproduce it.

Cheers,

Nico

(*) If you like cdist, it would be great if you could write a review, so
others can benefit from it as well.

Want commercial support for cdist? Have a look at http://www.ungleich.ch/offers/cdist-support/

*Created by: telmich* Hey Istvan, Istvan Beregszaszi [Wed, Apr 04, 2012 at 04:55:45AM -0700]: > Thanks for the great project! thanks for the good feedback! (*) > Unfortunately I quickly run into this problem: > > ``` > sed: -I or -i may not be used with stdin > ``` > > The problem is caused by this command: > > ``` > __key_value varnishd_enable --file /etc/rc.conf --delimiter '=' --value "YES" > ``` > > I'm running FreeBSD 8.2-STABLE on the remote machine. The generated `code-remote` is what you'd expect: Yeah, nice fbsd! Long time ago I used to use it though... > ``` > sed -i "s|^varnishd_enable$=\+$.*|varnishd_enable\1YES|" "/etc/rc.conf" > ``` Can you check your actual sed version? It may be that GNU sed and behaves differently. Not sure why sed detects use of stdin. I'd suggest to look at the source of the sed you are using and see how it detects that. > I believe the issue is due to improper argumenting. The `-i` flag gets the script, and the filename becomes the scripts, and no input file remains. Alright, that may be possible. If I have a look at my sed manpage, it looks like this: sed [OPTION]... {script-only-if-no-other-script} [input-file]... Using this sed: [15:04] brief:~% pacman -Qo $(which sed) /bin/sed is owned by sed 4.2.1-4 [15:04] brief:~% pacman -Qi sed | head -n 3 Name : sed Version : 4.2.1-4 URL : http://www.gnu.org/software/sed I've told Steven (the author of __key_value) about it, but we cannot reproduce it. Cheers, Nico (*) If you like cdist, it would be great if you could write a review, so others can benefit from it as well. ## Want commercial support for cdist? Have a look at http://www.ungleich.ch/offers/cdist-support/