gnu@perl has asked for the wisdom of the Perl Monks concerning the following question:

K, here's the deal. I wrote this program (still writing it actually) and I would just like the input form people here on better ways to do things.

The program is working fine and it does most things that I want, but it's not 'pretty'. I am to the point where I can do most anything I want/need to in perl, but not always in the best/most effecient way.

So anyway, here's the code, fire at will: 

use strict;
use Archive::Zip qw( :ERROR_CODES :CONSTANTS );
use Net::FTP;
use File::Basename;
use Getopt::Long;

# Vars
my $progname    = $0;
   $progname    =~ s/^.*\///;
my $version     = "1.01";
my $DEBUG       = 0;

$|++;


my $file;
my $ftp_host       = 'ftp.uslec.net';
my $ftp_user       = 'cheese';
my $ftp_pass       = 'whiz';
my $log_method     = 'both'; # both, email or file
my $def_email      = 'someone@somewhere.com';
my $emails;
my $LOGFILE        = "/u20/home/gvc/log/$progname.log";
my $ALT_LOGF;
my @LOG_MSGS;           # helpful and generic info go to log only.

my $set_error      = 0; # only send an email if something goes wrong.
                        # this will get set in the event of an error
                        # and an email with the output of this program
+ is sent.
                        # to the unix group.


GetOptions("send=s"             =>      \$log_method,
           "email=s"            =>      \$emails,
           "logfile=s"          =>      \$ALT_LOGF,
           "ftpserver=s"        =>      \$ftp_host,
           "file=s"             =>      \$file,
           "user=s"             =>      \$ftp_user,
           "pass=s"             =>      \$ftp_pass,
           "version"            =>      \&display_version,
           "help"               =>      \&show_help,
           );

if ( ( ($log_method eq 'both') and (! defined $emails) )
or   ( ($log_method eq 'both') and (! defined $ALT_LOGF) ) )
{
         print "Both --email and --logfile must be set when 'both'\n",
               "is selected as the notification method with --send\n",
               "NOTE: 'both' is the default if --send is not set on\n"
+,
               "the command line\n";
         print "$progname --send <both|email|file>\n";
         exit;
}

if (! defined $file)
{
    print "You did not specify a file to compress and send\n",
          "Specify a file with --file <full path to file>\n";
          exit;
}


###############
# MAIN
    $LOG_MSGS[0]   = "-----------------------------------------\n";
    $LOG_MSGS[1]   = scalar localtime()."\n"; # date and time are firs
+t in message.
    my $dest_size  = 0;
    my $zip_size   = make_zip("$file");
    if ($zip_size > 0)
    {
        $dest_size = ftp_file("$file.ZIP") ;
        if ($dest_size != $zip_size)
        {
            push(@LOG_MSGS,
                      "FTP Error, dest file incorrect size\nRetrying F
+TP.\n"); 
            $set_error = 1;
            $dest_size = ftp_file("$file.ZIP");
            if ( $dest_size != $zip_size)
            {
                 push(@LOG_MSGS,(
                      "Second FTP attempt of $file.ZIP failed!\n",
                      "This is your only notice!  No further\n",
                      "ftp attempts will be tried with this file!\n\n"
+));
                 $set_error = 1;
            }
        }
        else    #file transferred correctly
        {
            unlink "$file";
            unlink "$file.ZIP";
        }
    }
    else
    {
        push(@LOG_MSGS,"Error on zip of $file!\n");
        $set_error = 1;
    }

push(@LOG_MSGS,"SUCCESS!!\n") if (!$set_error);

# Send the output to the log file
log_output($ALT_LOGF)
    if ( ($log_method eq 'file') or ($log_method eq 'both') );
# Send the output to the email address specified on the command line.
email_output($emails)
    if ( ($log_method eq 'email') or ($log_method eq 'both') );

#######################################################
#              ADMINISTRATIVE ALERTS                  #
#######################################################
# this will log to gvc's log directory no matter what.
# kind of a fallback for us to check on things.
log_output($LOGFILE);

# emails me if any errors, this should always be the
# last alert so we can even catch alert errors.
email_output($def_email)
    if ( $set_error );
#######################################################

###############
# Subs
sub make_zip
{
    my $big_file    = shift;
    my $zip         = Archive::Zip->new();
    if (-r $big_file and -f $big_file)
    {
        push(@LOG_MSGS,"Zipping: $big_file\n");
        $zip->addFile($big_file);
        my $zip_status = $zip->writeToFileNamed("$big_file.ZIP");
        if ( $zip_status != AZ_OK )
        {
            push(@LOG_MSGS,"ZIP Error on file: $big_file\n");
            $set_error = 1;
            return -1;
        }
        else
        {
            return -s "$big_file.ZIP"; # returns the size of the zip f
+ile
        };
    }
    else
    {
      push(@LOG_MSGS,(
                "Cannot read file: $big_file\n",
                "File $big_file will not be processed!\n")); 
      $set_error = 1;
      return -2;
    }
}

sub ftp_file
{
my $ftp_file = shift;    
push(@LOG_MSGS,"FTP-ing $ftp_file\n");
my $ftp = Net::FTP->new($ftp_host);
   $ftp->login($ftp_user,$ftp_pass) if defined $ftp;
   $ftp->type("I") if defined $ftp; # ZIP file is a binary file.
   $ftp->put($ftp_file) if defined $ftp;
   my $return = $ftp->size(basename($ftp_file)) if defined $ftp;
   $ftp->quit() if defined $ftp;
   $return = -1 if not defined $return; # if for some reason it
                                        # cannot find the remote
                                        # file.
   return $return;
}


sub email_output
{
    my $local_emails = shift;
    open(MAIL,"|/usr/bin/mailx -s \"SLINC FTP PROCESSES\" $local_email
+s");
    print MAIL @LOG_MSGS;
    close(MAIL);
}

sub log_output
{
    my $log_file = shift;
    my $open_stat = open(LOG,">>$log_file");
    if (!defined $open_stat)
    {
        push(@LOG_MSGS,"Problem opening log file $log_file: $!\n") if 
+($!);
        warn "Problem opening log file $log_file: $!\n";
        $set_error = 1;
        return;
    }
    print LOG @LOG_MSGS;
    close(LOG);
}


sub display_version
{
    print "$progname version $version\n";
    print "Written by Chad M. Johnson\n";
    print "12/02/2002\n";
    exit;
}

sub show_help
{
    print "$progname --send <both|email|file> --email <someone\@somewh
+ere.com> --logfile </path/to/file.log> --file </path/to/file/to/compr
+ess/and/send>\n";
    print "\n< and > are just for clarity in this example.\n",
          "Do not use them when entering options.\n";
    exit;
}

[download]

Replies are listed 'Best First'.
Re: Compress and FTP files
by pg (Canon) on Dec 04, 2002 at 16:34 UTC
    Very nice. Some two-penny suggestions:
    1. I would suggest to use constant for variables such like $version etc. If it is something should not be modified, specify 'use constant', so its value will not be accidentally changed.
    2. You used quite a lot of 'global' variables. For example, sub ftp_file uses $ftp_file, $ftp_user, $ftp_pass etc, but none of them are passed in as parameters. Try to use variables with a smaller scope.
    3. Things like $ftp_pass should not be stored with the source code, it is a little bit too dangerous. Promopt for password, even can be a GUI interface. (I know, to hard code the password, you could save yourself a little bit time, each time you use this script, but it is not worth compare to the security issue.)
    4. Package it as a class, so other people may reuse your code easily.
[reply]
      Thanks, I agree with you on the scoping issue. That is the next thing I am working on. I do like the idea of setting constants for certain thing like $version.

      I also strongly dislike coding in the login and password for an ftp account, but this program is designed to ftp to one account by default. The user can pass in an alternate ftp server and login / password with the command line options. The one problem I had (and the reason I coded in the login / pasword) is that users who are not allowed access to the ftp server can still use this to place files there. Do you have an idea on a workaround for that circumstance?

      I did consider making it a package, but didn't think it was important enough. I assumed there was already something like this on cpan, I didn't check to see if there was, I just assumed.

[reply]
        As an alternative to a username/password, you could create a keypair and use something like scp instead of FTP, and only grant access to the private key file to users who are supposed to put files using this program. No password, and you're moving your access control/security problem out to the file system, rather than creating a new security problem. And if you need to replace the key because it has been compromised, you can do it without changing the code.
        --
        Spring: Forces, Coiled Again!
[reply]
Configuration File
by Wally Hartshorn (Hermit) on Dec 05, 2002 at 20:30 UTC

    You might consider moving many of those variables (ftp host, ftp user, email address, log directory, etc) into a configuration file. That would allow you to change these values in the future, without touching the source code. The config file could even be located in a place where your operations staff (if you have one) can modify it, allowing them to do things like update an email address without involving you at all.

    I like to use Config::IniFiles (available on CPAN) for this purpose. I generally tie it to a hash, reducing the number of global variables that I have to keep track of.

    Here's a sample config.ini file. It has separate values for testing and production, making it easy to switch from one to the other:

    [prod]
ftp_host   = ftp.uslec.net
ftp_user   = cheese
ftp_pass   = whiz
log_method = both
def_email  = someone@somewhere.com
LOGFILE    = /u20/home/gvc/log/something.log
[test]
ftp_host   = ftp.uslec.net
ftp_user   = cheesetest
ftp_pass   = whiztest
log_method = both
def_email  = someonetest@somewhere.com
LOGFILE    = /u20/home/gvc/log/somethingtest.log

    [download]

    Here's a sample code snippet:

    use Config::IniFiles;
our (%cfg);
my %ini;
tie %ini, 'Config::IniFiles', (-file => "config.ini");
%cfg = %{$ini{'prod'}};
print "I will FTP files to" . $cfg{ftp_host};

    [download]

    Wally Hartshorn

[reply]
[d/l]
[select]
      I had considered moving some of the things to a config file, but I wanted this to be somewhat self contained. The idea was that the default server (ftp.uslec.net) with the default login would be used if nothing is specified on the command line.

      The user does have the ability to use other servers by naming them on the command line with their appropriate login/pass combo.

      Part of the purpose of this program was to allow users to ftp to a server where they normally don't have access rights. Keeping the default server/login/pass in the code fulfilled that need. The security of the information is maintained by having no read rights on the program for the users, only execute rights. This 'security' is of course no different than if the info was in a config file with similar access control.

      I do really like the idea of using config files for most things and dislike hard coding informatin, but for this purpose I could find no better way to do it. Thanks for the tip on Config::IniFiles, I had not used that before but I definately will use it for my next project.

[reply]

Back to Seekers of Perl Wisdom