summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--README.md5
-rwxr-xr-xbin/shuriken36
-rw-r--r--src/lib/config.sync.source.sh36
-rw-r--r--src/shuriken.default.conf3
-rwxr-xr-xtests/cli.sh84
5 files changed, 152 insertions, 12 deletions
diff --git a/README.md b/README.md
index 9e9dc81..b853260 100644
--- a/README.md
+++ b/README.md
@@ -204,6 +204,11 @@ SYNC_DESTINATIONS=(
)
```
+`SYNC_DESTINATIONS` must be a Bash array, even for a single destination (for
+example `SYNC_DESTINATIONS=( '/path/with spaces/' )`). A scalar string is
+rejected with an error, since word-splitting would break destinations that
+contain spaces.
+
`--sync` runs `rsync -av --delete "$DIST_DIR/" "$destination"` for each
destination by default. The trailing slash on `DIST_DIR/` means the generated
contents are copied into the target directory. Set `SYNC_DELETE=no` or pass
diff --git a/bin/shuriken b/bin/shuriken
index 216460c..fd922e9 100755
--- a/bin/shuriken
+++ b/bin/shuriken
@@ -4567,13 +4567,37 @@ resolve_sync_destinations() {
# destinations_ref is a nameref output filled by resolve_config_array.
# shellcheck disable=SC2034
local -n destinations_ref="$1"; shift
+ local sync_decl
+
+ # A list of rsync destinations is inherently a list, and the only
+ # unambiguous spelling is a Bash array: SYNC_DESTINATIONS=( '/path/with
+ # spaces/' ) preserves embedded spaces, whereas a scalar string would be
+ # word-split (e.g. "/path/with spaces/" -> two broken destinations passed
+ # to rsync). So we reject scalar declarations outright instead of silently
+ # word-splitting them. (TAR_OPTS deliberately keeps scalar word-splitting:
+ # turning "-c -v" into separate options is the desired behaviour there.)
+ #
+ # declare -p fails only when SYNC_DESTINATIONS was never set; that is a
+ # legitimate "unset" case handled by the || true below, so we skip the
+ # array-vs-scalar check entirely when the variable does not exist.
+ if sync_decl=$(declare -p SYNC_DESTINATIONS 2>/dev/null); then
+ case "$sync_decl" in
+ declare\ -a*) ;;
+ *)
+ config_error \
+ 'SYNC_DESTINATIONS must be an array, e.g. SYNC_DESTINATIONS=( '\''dest1'\'' '\''dest2'\'' )'
+ return 1
+ ;;
+ esac
+ fi
- # Parse SYNC_DESTINATIONS (array or scalar) via the shared config-array
- # helper. Unlike TAR_OPTS there is no default: an unset or empty value
- # simply yields an empty destinations array for callers to validate.
- # The helper returns non-zero when SYNC_DESTINATIONS was never declared;
- # we ignore that (|| true) since the already-empty array is exactly the
- # desired result, and the bare non-zero return would otherwise trip set -e.
+ # Parse SYNC_DESTINATIONS (always an array at this point) via the shared
+ # config-array helper. Unlike TAR_OPTS there is no default: an unset or
+ # empty value simply yields an empty destinations array for callers to
+ # validate. The helper returns non-zero when SYNC_DESTINATIONS was never
+ # declared; we ignore that (|| true) since the already-empty array is
+ # exactly the desired result, and the bare non-zero return would otherwise
+ # trip set -e.
resolve_config_array SYNC_DESTINATIONS destinations_ref || true
}
diff --git a/src/lib/config.sync.source.sh b/src/lib/config.sync.source.sh
index 3d516dc..f4df169 100644
--- a/src/lib/config.sync.source.sh
+++ b/src/lib/config.sync.source.sh
@@ -2,13 +2,37 @@ resolve_sync_destinations() {
# destinations_ref is a nameref output filled by resolve_config_array.
# shellcheck disable=SC2034
local -n destinations_ref="$1"; shift
+ local sync_decl
- # Parse SYNC_DESTINATIONS (array or scalar) via the shared config-array
- # helper. Unlike TAR_OPTS there is no default: an unset or empty value
- # simply yields an empty destinations array for callers to validate.
- # The helper returns non-zero when SYNC_DESTINATIONS was never declared;
- # we ignore that (|| true) since the already-empty array is exactly the
- # desired result, and the bare non-zero return would otherwise trip set -e.
+ # A list of rsync destinations is inherently a list, and the only
+ # unambiguous spelling is a Bash array: SYNC_DESTINATIONS=( '/path/with
+ # spaces/' ) preserves embedded spaces, whereas a scalar string would be
+ # word-split (e.g. "/path/with spaces/" -> two broken destinations passed
+ # to rsync). So we reject scalar declarations outright instead of silently
+ # word-splitting them. (TAR_OPTS deliberately keeps scalar word-splitting:
+ # turning "-c -v" into separate options is the desired behaviour there.)
+ #
+ # declare -p fails only when SYNC_DESTINATIONS was never set; that is a
+ # legitimate "unset" case handled by the || true below, so we skip the
+ # array-vs-scalar check entirely when the variable does not exist.
+ if sync_decl=$(declare -p SYNC_DESTINATIONS 2>/dev/null); then
+ case "$sync_decl" in
+ declare\ -a*) ;;
+ *)
+ config_error \
+ 'SYNC_DESTINATIONS must be an array, e.g. SYNC_DESTINATIONS=( '\''dest1'\'' '\''dest2'\'' )'
+ return 1
+ ;;
+ esac
+ fi
+
+ # Parse SYNC_DESTINATIONS (always an array at this point) via the shared
+ # config-array helper. Unlike TAR_OPTS there is no default: an unset or
+ # empty value simply yields an empty destinations array for callers to
+ # validate. The helper returns non-zero when SYNC_DESTINATIONS was never
+ # declared; we ignore that (|| true) since the already-empty array is
+ # exactly the desired result, and the bare non-zero return would otherwise
+ # trip set -e.
resolve_config_array SYNC_DESTINATIONS destinations_ref || true
}
diff --git a/src/shuriken.default.conf b/src/shuriken.default.conf
index 9d0b1a8..d71ef8e 100644
--- a/src/shuriken.default.conf
+++ b/src/shuriken.default.conf
@@ -42,6 +42,9 @@ TAR_OPTS=(-c)
# Destinations for `shuriken --sync`. Each destination is passed to rsync
# with DIST_DIR/ as the source, so the generated contents are copied into the
# target directory. Sync deletes removed files from destinations by default.
+# SYNC_DESTINATIONS must be a Bash array, even for a single destination, e.g.
+# SYNC_DESTINATIONS=( '/path/with spaces/' ). A scalar string is rejected with
+# an error because word-splitting would break destinations containing spaces.
SYNC_DELETE=yes
SYNC_DESTINATIONS=(
# admin@fishfinger.buetow.org:/var/www/htdocs/example.org/
diff --git a/tests/cli.sh b/tests/cli.sh
index a16cbea..a28c1f1 100755
--- a/tests/cli.sh
+++ b/tests/cli.sh
@@ -5494,6 +5494,84 @@ test_sync_rejects_missing_dist() {
test::teardown
}
+test_sync_rejects_scalar_destinations() {
+ # Regression for an0: a scalar SYNC_DESTINATIONS with spaces used to be
+ # word-split into multiple broken rsync destinations. We now reject scalar
+ # declarations outright so the only spelling is an array, where embedded
+ # spaces are preserved naturally.
+ local config_file
+ local dist_dir
+ local fake_bin
+ local output
+ local rsync_log
+
+ test::setup
+ fake_bin="$TEST_TMPDIR/bin"
+ config_file="$TEST_TMPDIR/shuriken.conf"
+ dist_dir="$TEST_TMPDIR/dist"
+ rsync_log="$TEST_TMPDIR/rsync.log"
+
+ test::install_rsync_spy "$fake_bin"
+ mkdir -p "$dist_dir"
+ printf 'generated\n' > "$dist_dir/index.html"
+ {
+ printf 'DIST_DIR=%q\n' "$dist_dir"
+ printf 'SYNC_DESTINATIONS=%q\n' '/path/with spaces/'
+ } > "$config_file"
+
+ output=$(
+ cd "$TEST_TMPDIR"
+ PATH="$fake_bin:$PATH" TEST_RSYNC_LOG="$rsync_log" \
+ test::capture_failure_output "$TEST_SHURIKEN" --sync
+ )
+
+ test::assert_contains \
+ 'ERROR: SYNC_DESTINATIONS must be an array' \
+ "$output"
+ # rsync must never run: the scalar was rejected, not word-split.
+ test::assert_path_absent "$rsync_log"
+ test::teardown
+}
+
+test_sync_array_destination_preserves_spaces() {
+ # A single array destination containing spaces must reach rsync intact as
+ # one argument, proving the array path is unaffected by the an0 fix.
+ local config_file
+ local dist_dir
+ local fake_bin
+ local rsync_log
+ local rsync_output
+
+ test::setup
+ fake_bin="$TEST_TMPDIR/bin"
+ config_file="$TEST_TMPDIR/shuriken.conf"
+ dist_dir="$TEST_TMPDIR/dist"
+ rsync_log="$TEST_TMPDIR/rsync.log"
+
+ test::install_rsync_spy "$fake_bin"
+ mkdir -p "$dist_dir"
+ printf 'generated\n' > "$dist_dir/index.html"
+ {
+ printf 'DIST_DIR=%q\n' "$dist_dir"
+ printf 'SYNC_DESTINATIONS=(%q)\n' '/path/with spaces/'
+ } > "$config_file"
+
+ (
+ cd "$TEST_TMPDIR"
+ PATH="$fake_bin:$PATH" TEST_RSYNC_LOG="$rsync_log" \
+ "$TEST_SHURIKEN" --sync
+ )
+
+ rsync_output=$(<"$rsync_log")
+ # argc=4 (-av, --delete, source, single destination): the destination is
+ # NOT split into two arguments despite the embedded space.
+ # The rsync spy logs each argument with %q quoting, so a single argument
+ # with a space appears as one escaped arg3 (not two split args).
+ test::assert_contains 'argc=4' "$rsync_output"
+ test::assert_contains 'arg3=/path/with\ spaces/' "$rsync_output"
+ test::teardown
+}
+
test_positional_commands_fail_without_deprecation() {
local output
local old_command
@@ -6125,6 +6203,12 @@ main() {
'--sync rejects missing dist' \
test_sync_rejects_missing_dist
test::run_case \
+ '--sync rejects scalar destinations' \
+ test_sync_rejects_scalar_destinations
+ test::run_case \
+ '--sync array destination preserves spaces' \
+ test_sync_array_destination_preserves_spaces
+ test::run_case \
'positional commands fail without deprecation output' \
test_positional_commands_fail_without_deprecation
test::run_case \