Skip to content

__sed: Don't interpret --script - #148

Merged
4nd3r merged 2 commits into
skonfig:mainfrom
riiengineering:fix/type/__sed/dont-interpret-script
Aug 8, 2025
Merged

__sed: Don't interpret --script#148
4nd3r merged 2 commits into
skonfig:mainfrom
riiengineering:fix/type/__sed/dont-interpret-script

Conversation

@sideeffect42

@sideeffect42 sideeffect42 commented Aug 7, 2025

Copy link
Copy Markdown
Member

While implementing skonfig/extra#44 I discovered that the __sed type contains two bugs which cause it to interpret the script contents in unwanted ways:

  1. the unquoted here document interprets $ in --script as parameter expansions
  2. the use of echo can interpret escape sequences

As another optimisation step I think it should be possible to get rid of the here document altogether, albeit at the price of a less "explicit" code-remote.
Instead of reading the --script parameter contents and pasting them into a here document one could pass the parameter file to sed -f on the target directly (at least in the case where --script is not -).

If the script contains dollar signs, these can be interpreted as shell variables
if the here document is not quoted.

e.g. --script '1,/^DEVICESCAN/s#^DEVICESCAN.*$#DEVICESCAN ...#'

results in:

    code-remote:stderr
    ------------------
    sed: unmatched '#'
echo can interpret escape sequences which may interfere with the script.
It's better to just use or cat the script so that no escape sequences are
interpreted where they shouldn't.
@sideeffect42
sideeffect42 requested a review from 4nd3r August 7, 2025 17:32
@4nd3r
4nd3r merged commit 61102ae into skonfig:main Aug 8, 2025
1 check passed
@sideeffect42
sideeffect42 deleted the fix/type/__sed/dont-interpret-script branch August 10, 2025 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants