anordal / shellharden

The corrective bash syntax highlighter
Mozilla Public License 2.0
4.62k stars 129 forks source link

Command substituion does not work with double quotes #47

Closed GordianDziwis closed 2 years ago

GordianDziwis commented 2 years ago

This only works if I remove the qoutes arround $(make_extension_volume_string).

make_extension_volume_string() {
    extensions_volume_string=""
    for extension_source_dir in "${EXTENSION_SOURCE_DIRS[@]}"; do
        host_path=$(realpath "$extension_source_dir")
        base=$(basename "$extension_source_dir")
        extensions_volume_string+=" --volume {$host_path}:/usr/lib/ckanext/{$base}"
    done
    printf '%s' "$extensions_volume_string"
}

podman run -dt --pod "$pod_name" \
    --name ckan-"$IMAGE_STAGE" \
    --env-file "$env" \
    --env "CKAN_REDIS_URL=redis://localhost:6379/1" \
    --env "CKAN_SQLALCHEMY_URL=postgresql://ckan:$POSTGRES_PASSWORD@localhost:5432/ckan" \
    --env "CKAN_SOLR_URL=http://localhost:8983/solr/ckan" \
    --volume "$(realpath "$MERGE_INI")":/etc/ckan/merge.ini \
    "$(make_extension_volume_string)" \
    --volume ckan_home:/usr/lib/ckan \
    --volume ckan_storage:/var/lib/ckan \
    ckan-"$IMAGE_STAGE"

Otherwise this throws this error:

+ podman run -dt --pod ckan-develop --name ckan-development --env-file develop --env CKAN_REDIS_URL=redis://localhost:6379/1 --env CKAN_SQLALCHEMY_URL=postgresql://ckan:ckan@localhost:5432/ckan --env CKAN_SOLR_URL=http://localhost:8983/solr/ckan --volume /home/beavis/repositories/ckan-podman/deployment/ckan.ini:/etc/ckan/merge.ini ' --volume {/home/beavis/repositories/ckanext-test}:/usr/lib/ckanext/{ckanext-test}' --volume ckan_home:/usr/lib/ckan --volume ckan_storage:/var/lib/ckan ckan-development
Error: invalid reference format

What am I doing wrong?

anordal commented 2 years ago

Yes, finding a script that works by word splitting is typical and expected. Be prepared to rewrite cases like this if you want to have all expansions quoted. Note that quoting disables both word splitting and glob expansion of the expression. As is typical, the glob expansion behaviour in contrast looks more like a bug than intentional, which is why leaving it unquoted is seldom a correct solution. It won't be accepted by Shellharden or ShellCheck either.

There are two main ways out of this:

  1. Use an array. You already do with EXTENSION_SOURCE_DIRS – bonus points for that!
  2. There are safe ways to split a string if you must.

An array seems a good fit for your case. Unfortunately, there is no way to return an array from a function. Other than using a global variable. Maybe this is acceptible, since you already do for its input. Or you might inline the function. Any way, it would do this:

extension_volumes=()
for extension_source_dir in "${EXTENSION_SOURCE_DIRS[@]}"; do
    …
    extension_volumes+=("--volume" "{$host_path}:/usr/lib/ckanext/{$base}")
done
podman run … \
    "${extension_volumes[@]}"

Hope this is helpful.

GordianDziwis commented 2 years ago

Thanks a lot, this is very helpful!