Skip to content

Commit

Permalink
Remove escaping of backslashes to support literal ${var}
Browse files Browse the repository at this point in the history
THIS IS A BACKWARDS INCOMPATIBLE CHANGE

Automatic escaping of backslashes made it impossible to write `${var}`
to a U-Boot variable. When doing this, you have to remember that `fwup`
evaluates variable substitution twice - once when making the .fw file
and once when applying it. You obviously have to escape the `$` when
creating the .fw file. That made sense. Then to survive the apply step,
you'd think that you could double escape the `$`. You'd be wrong,
though, since `fwup` was escaping the backslashes that you were adding.
Therefore, the variable substitution was guaranteed to happen since you
couldn't double escape a `$`.

Amazingly, this behavior was never tested in the regression tests. When
you run into it, it's weird enough to be pretty confusing, imho.
Hopefully that and how rare should have been in real uses cases makes
this something that no one actually did.

This changes string processing to not automatically escape backslashes
so that it is possible to escape a `$` through to the end. This allows
you to write a U-Boot environment variable with a `${var}` in it.
This locks down string processing behaviors by adding unit tests.
  • Loading branch information
fhunleth committed Jan 29, 2024
1 parent 704e4ca commit 3409a40
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 3 deletions.
7 changes: 5 additions & 2 deletions src/cfgprint.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,15 @@ static void fwup_cfg_opt_nprint_var(cfg_opt_t *opt, unsigned int index, struct s
case CFGT_STR:
{
const char *str = cfg_opt_getnstr(opt, index);
// Fwup's feature of re-evaluating strings when applying firmware
// updates is a feature and used to modify behavior when applying
// firmware updates (aka runtime). Fwup has always escaped double
// quotes, though, since it really messes up error messages when
// double quotes aren't escaped.
ssprintf(s, "\"");
while (str && *str) {
if (*str == '"')
ssprintf(s, "\\\"");
else if (*str == '\\')
ssprintf(s, "\\\\");
else
ssprintf(s, "%c", *str);
str++;
Expand Down
8 changes: 8 additions & 0 deletions tests/004_env_vars.test
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ cat >"$CONFIG" <<EOF
# Test substitution in a field
meta-product = \${TEST_ENV_VAR}
# Test substitution in double quotes
meta-author = "\${TEST_ENV_VAR}"
# Test no substitution in single quotes
meta-misc = '\${TEST_ENV_VAR}'
# Test substitution in a resource name
file-resource "\${TEST_ENV_VAR}" {
# Test substitution in the middle of a string
Expand All @@ -30,6 +36,8 @@ EOF

cat >"$EXPECTED_META_CONF" <<EOF
meta-product="${TEST_ENV_VAR}"
meta-author="${TEST_ENV_VAR}"
meta-misc="\${TEST_ENV_VAR}"
file-resource "${TEST_ENV_VAR}" {
length=1024
blake2b-256="b25c2dfe31707f5572d9a3670d0dcfe5d59ccb010e6aba3b81aad133eb5e378b"
Expand Down
45 changes: 45 additions & 0 deletions tests/201_string_handling.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#!/bin/sh

#
# Test environment variables embedded in strings in the config files
#
# If this fails, you're using an old version of libconfuse. This is sadly
# very common. See the readme, but the basic fix is to go to the libconfuse
# GitHub and build it yourself.
#

. "$(cd "$(dirname "$0")" && pwd)/common.sh"

export TEST_ENV_VAR=1K.bin

cat >$CONFIG <<EOF
# Escape sequence in double quotes
meta-product = "Octal \044 and hex \x3c"
# Backslashes are escaped
meta-description = "\\\\$"
# Escape sequences and variables dont get processed in single quotes
meta-version = '\044 \x3c \${}'
# Substitution happens in double quotes
meta-author = "Substitution \${}"
# Escapes quotes in double quotes (fwup weird behavior 1)
meta-platform = "\"\""
# Escapes quotes in single quotes (fwup weird behavior 2)
meta-architecture = '""'
EOF

cat >$EXPECTED_META_CONF <<EOF
meta-product="Octal $ and hex <"
meta-description="\\$"
meta-version="\044 \x3c \${}"
meta-author="Substitution"
meta-platform="\"\""
meta-architecture="\"\""
EOF

$FWUP_CREATE -c -f $CONFIG -o $FWFILE

# Check that the zip file was created as expected
check_meta_conf

# Check that the verify logic works on this file
$FWUP_VERIFY -V -i $FWFILE
3 changes: 2 additions & 1 deletion tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ TESTS = 001_simple_fw.test \
197_fractional_end_segment.test \
198_gpt_partial.test \
199_partial_read.test \
200_uboot_redundant_255.test
200_uboot_redundant_255.test \
201_string_handling.test

EXTRA_DIST = $(TESTS) common.sh 1K.bin 1K-corrupt.bin 150K.bin

0 comments on commit 3409a40

Please sign in to comment.