Beefy Boxes and Bandwidth Generously Provided by pair Networks
We don't bite newbies here... much

comment on

( #3333=superdoc: print w/replies, xml ) Need Help??
First of all, I just want to say thank you for the excellent comments you offered. I've coded Perl for about a year now, but within Text::Forge I'm shielded from most of the issues involved in traditional CGI programming, hence my immature code. I ignored some of your suggestions because I'm lazy.

This line does nothing useful, it does not untaint $mode
tr/// can't be used to untaint? I didn't know that.

This code is a very obtuse way to do something quite simple, namely go to one of two subs depending on the user input.
Quite true. This is just my "cookie cutter" dispatch hash... it makes more sense in a CGI that has many functions; although a CGI with many functions probably doesn't make much sense...

The main thing I've changed below is file locking. Could you take a look at this and tell me what you think? I'm attempting to use semaphores as advised by KM in a few of his/her posts.

I like the following and will definitely use them in my next CGI:
1. waiting for lock sleeper 2. your DieNice, WarnAdmin, and Unindent routines
#!/usr/local/bin/perl -Tw # Pre-live checklist: # [X] add some file locking for Godsakes!!! # [X] link to this thing and do auth ONLY through SSL!! # [X] protect /admin dir with .htaccess # [X] use $ENV{ REMOTE_USER } once .htaccess is in place # [X] make sure to untaint $ENV{ REMOTE_USER } # [X] write a file-storing routine ... just did it inline instead # [X] bonus points: use XML :) # [X] app tempfiles? # [X] move to "live" data dir automatically? # [X] add CSV validation to _validate_upload # [X] test CSV validation to _validate_upload # [X] count fields in a cool way for $num_fields use lib '/dw02/d37/dawgpolo/myperl'; use lib '/dw02/d37/dawgpolo/myperl/lib'; use CGI; use CGI::Carp qw( fatalsToBrowser ); # I think this slows us down use Data::Dumper; use Date::Format; use Dawg qw( %CONFIG %CSV ); use Dawg::DB; use Dawg::Draw; use Fcntl qw( :flock ); use strict; $ENV{ HTTPS } or die "Insecure! Change URL to https://$ENV{ HTTP_HOST +}". "$ENV{ SCRIPT_NAME }\n"; $ENV{ HTTPS } =~ tr/A-Za-z//cd; $ENV{ SERVER_ADMIN } = $CONFIG{ EMAIL }; $ENV{ PATH } = '/usr/local/bin:/usr/bin:/bin'; my $cgi = CGI->new; my $draw = Dawg::Draw->new; my $title = 'CSV Update Tool'; my $self = $ENV{ SCRIPT_NAME }; my $user = $ENV{ REMOTE_USER }; $user =~ tr/A-Za-z//cd; my $userIP = $ENV{ REMOTE_ADDR }; sub main { my %dispatch = ( upload => \&upload, debug => \&debug, DEFAULT => \&upload, ); $draw->headers(); $draw->html_start( { title => $title } ); my $action = $cgi->param('action') || 'DEFAULT'; my $do_sub = $dispatch{ $action }; if ( $do_sub ) { &$do_sub(); } else { warn "[warn] Invalid action: [$action]"; die "[die] You can't make me do that!\n"; } $draw->html_end(); } sub upload { my @errors = (); # array of errors found during execution my @lines = (); # input lines of uploaded file my $page = ''; # page that uploaded file corresponds to my $ucpage = ''; # uppercased version of the same thang my $mode = $cgi->param( 'mode' ); $mode =~ tr#a-z_##cd; # untaint that pa +ram! my $in = $cgi->upload( 'datafile_in' ); # returns a fileh +andle my $logfile = "$CONFIG{ ADMIN }/transactions.log"; # log entire tran +saction my $qwiklog = "$CONFIG{ ADMIN }/quicklog.csv"; # abbreviated log open( LOG, ">> $logfile" ) or die "Couldn't open $logfile"; open( QWK, ">> $qwiklog" ) or die "Couldn't open $qwiklog"; if ( $mode ) { # They're trying to upload a file push @errors, "Invalid filename" unless ( ref $in eq 'Fh' ); # retrieve the uploaded file and check for data integrity unless ( @errors ) { my $rv = Dawg::DB->slurp_csv_safe( $in ); if ( $rv ) { @lines = @$rv; } else { die "No lines returned while slurping CSV"; } if ( $mode =~ /^([a-z_]+)_go$/ ) { $page = $1; $ucpage = uc($page); die "unknown page [$ucpage]" unless ( exists $CSV{$ucpage} ); my $result = Dawg::DB->validate_csv( $ucpage, \@lines ); @errors = @$result unless $result == 1; } } if ( @errors ) { print qq[ <FONT COLOR="RED">ERRORS OCCURRED DURING FILE UPLOAD:\ +n<BR> ]; print qq[ <UL>\n ]; foreach my $error ( @errors ) { print "<LI>$error\n"; } print qq[ </UL></FONT>\n ]; } else { # upload was good. my $time = time2str( "%C", time() ); my $outfile = "$CONFIG{DATA}/$page.csv" or die "cannot draft datafile filename for [$page]"; open( CSV, "> $outfile" ) or die "Couldn't open $outfile"; print qq[ <FONT COLOR="GREEN">$ucpage UPLOAD SUCCESSFUL</FONT><B +R>\n ]; # - 1 - open and lock semaphores my $csv_sem = "$CONFIG{ADMIN}/.csv.lock"; my $log_sem = "$CONFIG{ADMIN}/.transactions.lock"; my $qwk_sem = "$CONFIG{ADMIN}/.quicklog.lock"; open( CSV_SEM, "> $csv_sem" ) or die "$csv_sem: $!"; open( LOG_SEM, "> $log_sem" ) or die "$log_sem: $!"; open( QWK_SEM, "> $qwk_sem" ) or die "$qwk_sem: $!"; flock( CSV_SEM, LOCK_EX ) or die "Can't lock CSV"; flock( LOG_SEM, LOCK_EX ) or die "Can't lock LOG"; flock( QWK_SEM, LOCK_EX ) or die "Can't lock QWK"; # - 2 - write the datafile itself for (@lines) { print CSV "$_\n" } # write csv to datafile # - 3 - write the entire transaction print LOG "[ $time ] $ucpage UPDATED by $user [$userIP]\n"; for (@lines) { print LOG "$_\n" } # log the transaction # - 4 - write a "qwiklog" entry print QWK "[ $time ] $ucpage UPDATED by $user [$userIP]\n"; # - 5 - close all filehandles, drop locks close( CSV ) or die "Couldn't close file $outfile"; # file lock +s close( LOG ) or die "Couldn't close file $logfile"; # are remov +ed close( QWK ) or die "Couldn't close file $qwiklog"; # automatic +ally close( CSV_SEM ) or warn "close error on $csv_sem: $!"; close( LOG_SEM ) or warn "close error on $log_sem: $!"; close( QWK_SEM ) or warn "close error on $qwk_sem: $!"; } } else { # They're not trying to upload a file YET, so give them tips print <<End_HTML; Hints:<BR> <UL> <LI>Make sure the uploaded file is TRUE CSV, eg. blah,1,2,"blow, joe +" <LI>Click the respective Upload button once, then <EM>wait</EM> <LI>You can only upload one file at a time </UL> End_HTML } foreach my $table ( sort keys %CSV ) { my $name = lc($table); print <<UploadForms; <BR> Download <A HREF="$CONFIG{ WEBDATA }/$name.csv">$name</A> datafile <FORM ACTION="$self" METHOD="POST" ENCTYPE="multipart/form-data"> <INPUT TYPE="FILE" NAME="datafile_in" SIZE="50" MAXLENGTH="80"> <BR> <INPUT TYPE="SUBMIT" VALUE="Upload $name"> <INPUT TYPE="HIDDEN" NAME="mode" VALUE="${name}_go"> </FORM> <BR> UploadForms } } sub debug { print "<PRE>\n"; print Data::Dumper->Dump( [\%ENV], ['ENV'] ); print "</PRE>\n"; } main();

In reply to Re: Re: Code review, good 'ol CGI. by meonkeys
in thread Code review, good 'ol CGI. by meonkeys

Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post; it's "PerlMonks-approved HTML":

  • Are you posting in the right place? Check out Where do I post X? to know for sure.
  • Posts may use any of the Perl Monks Approved HTML tags. Currently these include the following:
    <code> <a> <b> <big> <blockquote> <br /> <dd> <dl> <dt> <em> <font> <h1> <h2> <h3> <h4> <h5> <h6> <hr /> <i> <li> <nbsp> <ol> <p> <small> <strike> <strong> <sub> <sup> <table> <td> <th> <tr> <tt> <u> <ul>
  • Snippets of code should be wrapped in <code> tags not <pre> tags. In fact, <pre> tags should generally be avoided. If they must be used, extreme care should be taken to ensure that their contents do not have long lines (<70 chars), in order to prevent horizontal scrolling (and possible janitor intervention).
  • Want more info? How to link or How to display code and escape characters are good places to start.
Log In?

What's my password?
Create A New User
Domain Nodelet?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others rifling through the Monastery: (1)
As of 2023-06-01 06:24 GMT
Find Nodes?
    Voting Booth?

    No recent polls found