[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: off topic! What is the error in this script



On Mon 26 Jun 2017 at 09:24:40 (-0400), Greg Wooledge wrote:
> On Sun, Jun 25, 2017 at 06:30:18PM +0300, Teemu Likonen wrote:
> > Dominic Knight [2017-06-25 15:35:46+01] wrote:
> > 
> > > To convert a series of .flac files to .mp3 files I attempted to use the
> > > following line;
> > >
> > >> $ find -name "*.flac" -exec bash -c 'ffmpeg -i "{}" -y -acodec
> > > libmp3lame -ab 320k "${0/.flac}.mp3"' {} \;
> 
> Teemu has highlighted one of the bugs here:
> 
> > The arguments for "bash -c" go like this:
> > 
> >     bash -c '...' name one two three
> 
> The other bug is that you are have {} inside of a larger argument to
> find -exec.  This is "allowed" by GNU find (a horrible misfeature of
> the GNU implementation), but is not allowed by other implementations
> of find, because it leads directly to a code injection vulnerability.
> 
> (You are also using the implicit "." starting path for find, which is
> another GNU extension.  This one is not a security problem, just a
> portability problem.)
> 
> > And in '...' the arguments are in variables $0 (=name), $1 (=one), $2
> > (=two), $3 (=three) etc. So:
> > 
> >     find -name "*.flac" -exec \
> >         bash -c 'ffmpeg -i file:"$1" -c:a libmp3lame -ab 320k -y file:"${1%.flac}.mp3"' \
> >         foo {} \;
> > 
> > Note the "foo": it is saved to $0 ("shell's name") and and then the
> > actual filename is in usual first positional parameter $1. We also want
> > to have explicitl "file:" protocol with ffmpeg so that any "something:"
> > prerix in filename is not interpreted as a protocol name. (See "man
> > ffmpeg-protocols".)
> > 
> > But here's another way without double quoting:
> > 
> >     while read -r -d $'\0' input; do
> >         ffmpeg -i file:"$input" -c:a libmp3lame -ab 320k \
> >             -y file:"${input%.flac}.mp3"
> >     done < <(find . -name '*.flac' -print0)
> 
> $'\0' is just the same as ''.  Also, you need to set IFS to the empty
> string for this read, to suppress trimming of leading/trailing IFS
> whitespace characters in filenames.
> 
> while IFS= read -r -d '' input; do ...
> 
> I'd probably add -type f to the find command.
> 
> Finally, ffmpeg is known to have an issue inside loops that read
> from stdin.  Specifically, ffmpeg is one of those programs that reads
> all available stdin, which means the loop will only run once, with the
> other filenames being slurped up by ffmpeg with unpredictable results.
> ssh is another such program.
> 
> There are two workarounds for that:
> 
> 1) Add </dev/null to the ffmpeg command.
> 
> 2) Use a file descriptor other than stdin for the outer while loop.
>    For example,
> 
>      while IFS= read -r -d '' input <&3; do
>          ffmpeg -i file:"$input" -c:a libmp3lame -ab 320k \
>              -y file:"${input%.flac}.mp3"
>      done 3< <(find . -type f -name '*.flac' -print0)

While it's laudable to write command lines such as these, I can't help
being reminded of perl obfuscation contests.

I've been recording radio/TV programmes/vinyl discs/whatever for years,
and generating mp3 files from them (and from MIDI/youtube-type files).
I have bash functions for handling the different sources, targets
("faithful", or compressed for noisy backgrounds, or wav for further
processing), and processes (gapless concatenation or individual
tracks). They're named <source>[s]2mp3[x] where s is for concatenating
the inputs, and x with compression.

This means you haven't got to remember how to apply your preferred
bitrates, compression parameters and so on, and the functions can look
after output directory creation, preservation of timestamps, and any
other housekeeping tasks.

Just a suggestion…

Cheers,
David.


Reply to: