koalaman / shellcheck

ShellCheck, a static analysis tool for shell scripts
https://www.shellcheck.net
GNU General Public License v3.0
36.36k stars 1.77k forks source link

Quote this to prevent word splitting #2662

Open thisguyshouldworkforus opened 1 year ago

thisguyshouldworkforus commented 1 year ago

For bugs

#!/usr/bin/env bash

read -r -a EXISTING_PATHS <<< $(grep -Eri '^/' /etc/logrotate.conf /etc/logrotate.d/* | sed -r 's/(\s+)?(\{$)?|(\*[\.\_]log\ \{)//g' | sed 's/\/$//g' | awk -F ':' '{print $2}' | grep -Eiv '\.log$' | sort -u)

Here's what shellcheck currently says:

#!/usr/bin/env bash

# Error on SC2046
read -r -a EXISTING_PATHS_UNQUOTED <<< $(grep -Eri '^/' /etc/logrotate.conf /etc/logrotate.d/* | sed -r 's/(\s+)?(\{$)?|(\*[\.\_]log\ \{)//g' | sed 's/\/$//g' | awk -F ':' '{print $2}' | grep -Eiv '\.log$' | sort -u)
echo "${#EXISTING_PATHS_UNQUOTED[@]}"
13

# No error on SC2046
read -r -a EXISTING_PATHS_QUOTED <<< "$(grep -Eri '^/' /etc/logrotate.conf /etc/logrotate.d/* | sed -r 's/(\s+)?(\{$)?|(\*[\.\_]log\ \{)//g' | sed 's/\/$//g' | awk -F ':' '{print $2}' | grep -Eiv '\.log$' | sort -u)"
echo "${#EXISTING_PATHS_QUOTED[@]}"
1

Here's what I wanted or expected to see:

brother commented 1 year ago

logrotate states that paths with spaces must be enclosed in quotes, this means that logrotate config files where a path with spaces in it is the first on the line will not be found with your example. If the quoted path is second or later in the line you could happen upon it.

brother ~$ head -n 1 /tmp/early /tmp/late 
==> /tmp/early <==
"/tmp/example/file path" /tmp/some/path {

==> /tmp/late <==
/tmp/some/path "/tmp/example/file path" {
brother ~$ grep -Eri '^/' /tmp/early /tmp/late 
/tmp/late:/tmp/some/path "/tmp/example/file path" {

Looking at the sed statements I think you are anyhow removing the spaces in the first one there. Given that I think you are safe to just add an ignore statement before the execution of this. Example

brother ~$ grep -Eri '^/' /tmp/early /tmp/late | sed -r 's/(\s+)?(\{$)?|(\*[\.\_]log\ \{)//g' 
/tmp/late:/tmp/some/path"/tmp/example/filepath"
TriMoon commented 1 year ago

Why not just pipe the output of that line, using Process Substitution, to the read command, you are already using multiple pipes anyhow :thinking: It will make the code more readable for a human also... (Plus shellcheck does not complain...) eg:

#!/usr/bin/env bash
read -r -a EXISTING_PATHS_UNQUOTED < <(
        grep -Eri '^/' \
            /etc/logrotate.conf \
            /etc/logrotate.d/* \
    | sed -r 's/(\s+)?(\{$)?|(\*[\.\_]log\ \{)//g' \
    | sed 's/\/$//g' \
    | awk -F ':' '{print $2}' \
    | grep -Eiv '\.log$' \
    | sort -u
)
echo "${#EXISTING_PATHS_UNQUOTED[@]}"

So in your example i must agree with shellcheck on this example...


Disclaimer, i wrote the above out of head, the -u might need to be changed with a <, been too long since i used this kind... Fixed code i hope :wink: