6
\$\begingroup\$

We have a Debian Squeeze server where we upload files through SFTP, and I wrote this script to automatically compress uploaded files.

The script is scheduled through a cron job in an unprivileged user's crontab.

As of now, the script works and does its job.

But I'm pretty novice to Bash scripting, and I'm wondering if it has any major flaws. (e.g. If the file name contains " what happens? Does it have security flaws?)

Also, am I doing it right, or there are better ways to do it?

#!/bin/bash

# Use the newline character
# as the field separator.
IFS=$'\n'

# The folder to watch.
dir="/path"

# Get the folder's content, recurively.
for item in $(find "${dir}")
do

        # If the item is a regular file.
        if [ -f "${item}" ]
        then

                # Check if the file is in use 
                # by someone else. E.g. The SFTP
                # daemon because the user is still
                # uploading it.
                lsof -n "${item}" > /dev/null

                # If no one is using the file.
                if [ "$?" -ne 0 ]
                then

                        # Get the file extension.
                        ext=$(echo "${item}" | awk -F "." '{ print $NF }')

                        # If the file is not a zip archive
                        # and does not exists a zip archive for it
                        if [ "${ext}" != "zip" -a ! -f "${item}.zip" ]
                        then
                                # Create the zip archive.
                                zip "${item}.zip" "${item}" > /dev/null
                        fi

                fi

        fi

done
\$\endgroup\$
  • 1
    \$\begingroup\$ Grabbing the extension can be implemented without awk using ext=${filename##*.}. \$\endgroup\$ – qubyte Feb 24 '12 at 12:10
5
\$\begingroup\$

Slightly different than @kojiro's answer, using find for everything:

find /path \
     -type f \
     ! -iname "*.zip" \
     ! -exec /usr/sbin/lsof -n "{}" \; \
     ! -exec test -f "{}.zip" \; \
     -exec zip -q "{}.zip" "{}" \; \
     -exec rm "{}" \;

(last step only if the removal of the original is required).

\$\endgroup\$
  • \$\begingroup\$ $dir will break on wordsplitting, et al. \$\endgroup\$ – Chris Down Apr 4 '12 at 13:38
  • \$\begingroup\$ Thanks, $dir was only meant as an example for the "folder to watch", hopefully this is clearer. \$\endgroup\$ – beny23 Apr 4 '12 at 14:09
  • \$\begingroup\$ For some reason I have trouble remembering that test is available as an external command. This is an excellent answer. \$\endgroup\$ – kojiro Apr 4 '12 at 14:58
  • \$\begingroup\$ @kojiro test and [ are external, you're probably thinking of [[. \$\endgroup\$ – Chris Down Apr 4 '12 at 16:13
  • \$\begingroup\$ @ChrisDown Well, no, I was actually thinking of test, which is both an external and a builtin. What I was trying to say was that the existence of builtin test blinded me from thinking of using /bin/test \$\endgroup\$ – kojiro Apr 4 '12 at 17:09
3
\$\begingroup\$

Almost all the work you've done can be done by find directly. It's efficient at eliminating based on your parameters, and can pass a tailored list of arguments to bash for final zipping.

#!/bin/bash

# The folder to watch.
dir="/path"

# You shouldn't ever loop over for x in $(find …) this way, because it can break easily;
# however, find offers several options itself to give you another approach:

find "$dir" \
    -type f \ # If the item is a regular file
    ! -iname '*.zip' \ # If the filename does not end with zip (case-insensitive)
    ! -exec lsof -n {} \; \ # If the file is not in use by someone else
    -exec bash -c '
        # back to bash for this part
        # loop over the parameters passed to bash, which should only be interesting files
        for item
        do
            # Create the zip archive only when one does not exist
            [[ -f ${item}.zip ]] || zip "${item}.zip" "${item}" > /dev/null
        done
    ' _ {} + # pass all the found files to bash to process.

Edited to reflect that IFS isn't being used anymore.

\$\endgroup\$
  • \$\begingroup\$ It's generally considered the convention to not capitalise non-environment variables, and $OLDIFS isn't one. I don't even see where you are using it... \$\endgroup\$ – Chris Down Apr 4 '12 at 12:27
  • \$\begingroup\$ Good point; IFS isn't being used anymore. So edited. \$\endgroup\$ – kojiro Apr 4 '12 at 13:34
  • \$\begingroup\$ @l0b0 what makes you say it can't handle filenames with newlines? Here's a simplified demonstration \$\endgroup\$ – kojiro Apr 4 '12 at 13:48
  • \$\begingroup\$ I just saw IFS=$'\n', but that's been edited away. This works OK. \$\endgroup\$ – l0b0 Apr 4 '12 at 14:19
  • \$\begingroup\$ @l0b0 fwiw, even setting IFS=$'\n' wouldn't have caused a problem in this case, since it wouldn't have been exported to the nested bash. \$\endgroup\$ – kojiro Apr 4 '12 at 17:07
2
\$\begingroup\$

I don't see why it needs to be so complicated. You also risk all sorts of problems if the files happen to contain newlines (and this isn't just a theoretical, I've seen it happen, especially when copying text to file managers that don't chomp filenames when renaming).

A note: globstar requires bash4.

#!/bin/bash
shopt -s nullglob globstar extglob
for file in /path/**/!(*.zip); do
    [[ -f $file && ! -e $file.zip ]] && lsof -n "$file" >/dev/null || continue
    zip -q "$file"{.zip,}
done
\$\endgroup\$
  • \$\begingroup\$ Agreed. -- I feel like either the zip'd files or the originals should be ultimately moved elsewhere, or the originals deleted for that matter, but that wasn't in the question. zip foo foo.zip keeps foo on the system, its not like 'gzip` \$\endgroup\$ – sente Apr 4 '12 at 12:43
  • 1
    \$\begingroup\$ The /path/**/* glob won't work if the system isn't running Bash 4, that should be mentioned as well. \$\endgroup\$ – sente Apr 4 '12 at 12:49
  • \$\begingroup\$ @StuartPowers -- I did mention that in the second paragraph. \$\endgroup\$ – Chris Down Apr 4 '12 at 13:00
  • \$\begingroup\$ Why not use extglob as well, and then for file in /path/**/!(*.zip)? Proof \$\endgroup\$ – kojiro Apr 4 '12 at 14:00
  • \$\begingroup\$ @kojiro - Good catch! \$\endgroup\$ – Chris Down Apr 4 '12 at 14:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.