4

Ubuntu 16.04

#!/bin/bash

site="hello"
wDir="/home/websites/${site}/httpdocs/"

for file in $(find "${wDir}" -name "*.css")
do
   echo "$file";
done
exit 0;

shellcheck warns me even if I define the start directory but the script works just fine.

root@me /scripts/ # shellcheck test.sh

In test.sh line 6:
for file in $(find "${wDir}" -name "*.css")
            ^-- SC2044: For loops over find output are fragile. Use find -exec or a while read loop.
Vituvo
  • 411
  • @IgnacioVazquez-AbramsIt doesn't but I did not consider shellcheck was warning about files with spaces. It did not click at the time mate. – Vituvo Mar 19 '18 at 12:06

2 Answers2

12

The problem is exactly what shellcheck is telling you: for loops iterating over the output of find or similar commands are fragile. For example:

$ ls
'a file with spaces' 

$ for file in $(find . ); do    echo "$file"; done
.
./a
file
with
spaces

The safe way would be either to use the -exec of find:

$ find . -exec echo  {} \;
.
./a file with spaces

Or to use a while loop:

$ find . -print0 | while IFS= read -r -d '' file; do echo "$file"; done
.
./a file with spaces
Jeff Schaller
  • 67,283
  • 35
  • 116
  • 255
terdon
  • 242,166
7

Using a for loop over find output is an anti-pattern at best. See BashFAQ/001 - How can I read a file (data stream, variable) line-by-line (and/or field-by-field)? for reason why. Use a while loop as below with a read command. The below command delimits the output of find with a NULL byte and read command reads by splitting on that byte, so that all files with special characters in their names are safely handled (including newlines)

#!/usr/bin/env bash

site="hello"
wDir="/home/websites/${site}/httpdocs/"

find "${wDir}" -name "*.css" -type f -print0 | while IFS= read -r -d '' file; do
    printf '%s\n' "$file"
done

Or altogether avoid using the pipe-lines and do process-substitution 

while IFS= read -r -d '' file; do
    printf '%s\n' "$file"
done< <(find "${wDir}" -name "*.css" -type f -print0)

The web ShellCheck does not report any issues for either of the two snippets above.

Inian
  • 12,807
  • 3
    imo, | while is anti pattern too.. find already has -exec which handles filenames properly... – Sundeep Mar 19 '18 at 14:27
  • 1
    @Sundeep, sometimes -exec isn't useful. Consider the case where you're modifying shell variables (though, granted, that would be while ... done < <(find ...) if you want the updated variables to persist and aren't using ksh or the lastpipe option), or calling builtins or shell functions (where -exec tells find to start a bunch of new shells, when you could otherwise use just one instance). -exec is a good tool to use when it's appropriate, but "when it's appropriate" is a constrained set. – Charles Duffy Mar 19 '18 at 15:19
  • good point.. and -exec sh/bash/zsh/etc -c is an option too when shell is needed.. I can only think of parallel processing as a reason for piping find's output.. – Sundeep Mar 19 '18 at 15:44
  • Note that echo "$file" is another anti-pattern – Stéphane Chazelas Mar 19 '18 at 15:48
  • @StéphaneChazelas: Done Stephane! Appreciate it – Inian Mar 19 '18 at 15:50
  • 1
    @Sundeep, variables set in -exec sh -c '...' don't persist after the find command exits, whereas if you put your find in a process substitution, you can populate data structures from a while read loop. Consider for example: declare -A exts=( ); while IFS= read -r -d '' filename; do [[ $filename = *.* ]] || continue; (( exts[${filename##*.}]+= 1 )); done < <(find . -maxdepth 1 -type f -print0); declare -p exts as something you couldn't do (with the exts variable surviving, and supporting more files than fit on one command line) with -exec. – Charles Duffy Mar 19 '18 at 15:56
  • @CharlesDuffy yup agree.. I meant that it is an additional option for cases that could be covered with that option.. – Sundeep Mar 19 '18 at 16:00