MadPogo has asked for the wisdom of the Perl Monks concerning the following question:

I have been running myself into a brick wall repeatedly and I am at a point were I must beg for help. I am trying to upload several images and am passing the image paths from one form to another for processing as hidden tags. I am then trying to read in the local image file and write it to the server in a name format that I choose. I am having little luck with the last portion. It runs through a foreach array that keys on the CGI param to determine if the field is of an image type. If so it will then attempt to upload the image. In each iteration it creates the file I want to populate with the users local file but it doesnt seem to want to write the data in. This is a relatively new area for me in Perl. In the past I have used FILEHANDLES and simply read and printed the file from one handle to the other. When that failed I attempted the use of syswrite thinking it was needed to specifically write non string data. That too failed here is the following routine in question, any help is appreciated:
sub publish_html { my $q = new CGI; &get_DTG; $header_name = "header.dat"; @header = ('BEGIN','XFER',$tar_file,$DESTINATIONS,'R','U','001','0 +40','NONE',$DTG,'NONE','MPU','END'); # Ignore bad extensions if good extensions is specified. undef @bad_extensions if @good_extensions; print header; print start_html(-title=>$title_tag,-bgcolor=>$bg_color,-text=>$te +xt_color); print font({-color=>'lime'},h2({-align=>CENTER},'MPU UPLOAD PROGRE +SS')),br,br; # Create DPS Header for routing if(open(OUTHEADER, "> $TEMP_DIR/$header_name") || die("Cannot Open + File $header_file for reading: $!")) { print font({-color=>'white',-size=>$content_font_size},'Creati +ng HEADER... '),br; foreach $line( @header ){ print OUTHEADER $line."\n"; print font({-color=>'white',-size=>$content_font_size},'Wri +ting... '.$line),br; } close (OUTHEADER); print font({-color=>'white',-size=>$content_font_size},'Header +file saved to... '.$TEMP_DIR."/".$header_name),br,br; } ############################################ #PERFORM FILE CHECKING AND UPLOAD IMAGES foreach $file ($q->param) { if ($file ne "" & $file =~ /image/){ $path = param($file); # PARSE AREA HTML NAME FOR IMAGE NAME PREPEND # areafile is a param that contains the area html page na +me # this is used to prepend the areaname to the image file +to # ensure all images are unique per area. $htmlfile = param('areafile'); if ($htmlfile =~ m!^(.*)\.(.*)$!) { $html_lead = $1; $html_ext = $2; } # PARSE OUT PATH FOR BUILDING PUBLISHED IMAGE NAMES if ($path =~ m!^(.*)\\(.*)$!) { $leadpath = $1; $filename = $2; if ($filename =~ m!^(.*)\.(.*)$!) { $name = $1; $file_ext = ".".$2; } } elsif ($filename =~ m!^(.*)/(.*)$!) { $name = $1; $file_ext = ".".$2; } else { $leadpath = "."; $filename = $path; } print font({-color=>'white',-size=>$content_font_size},'P +ROCESSING FILE... '.$path),br; $path =~ s/^.*(\\|\/)//; my $proceed_type = 0; if(@good_extensions) { foreach(@good_extensions) { my $ext = $_; $ext =~ s/\.//g; if($path =~ /\.$ext$/) { print font({-color=>'white',-size=>$content_fo +nt_size},'VERIFIED FILE TYPE OF... '.$path),br; $proceed_type = 1; last; } } unless($proceed_type) { push(@was_not_good_type, $path); } } elsif(@bad_extensions) { $proceed_type = 1; foreach(@bad_extensions) { my $ext = $_; $ext =~ s/\.//g; if($path =~ /\.$ext$/) { $proceed_type = 0; last; } } unless($proceed_type) { push(@was_a_bad_type, $path); } } else { $proceed_type = 1; } # WRITE IMAGE TO SERVER USING FORM FIELD NAME .EXTENSION T +O THE $TEMP_DIR # THIS PROCESS INSURES THAT NO 2 AREAS WILL HAVE THE SAME +IMAGE NAME if($proceed_type) { if(open(OUTFILE, "> $TEMP_DIR/$html_lead$file$file_e +xt") || die("Cannot Open File $TEMP_DIR/$html_lead$file$file_ext for +writing: $!")) { print font({-color=>'white',-size=>$content_font +_size},'UPLOADING... '.$path),br; if(open(READFILE, "< $path") || die("Cannot Op +en File $path for reading: $!")) { while (my $bytesread = read(READFILE, my + $buffer, 1024)) { syswrite(OUTFILE, my $buffer, 1024); } } close (INFILE); close (OUTFILE); } } if($max_size) { if((-s "$TEMP_DIR/$html_lead$file$file_ext") > ($max_s +ize * 1024)) { push(@was_too_big, $path); unlink("$TEMP_DIR/$html_lead$file$file_ext"); } } print font({-color=>'white',-size=>$content_font_size},'FI +LE SAVED TO... '.$TEMP_DIR.'/'.$html_lead.$file.$file_ext),br,br; push(@file_did_save, $path); &results; } else { push(@did_not_save, $path); } } print start_form(-action=>$SCRIPT_NAME); print center submit(-value=>'BACK TO FORM',-onclick=>'history.go(- +2)'); close_form(); print end_html; }

Replies are listed 'Best First'.
Re: Uploading an image
by Fastolfe (Vicar) on Dec 05, 2001 at 11:18 UTC

    In the future please try reducing your code to a simple test case. Few of us have the time to go through this amount of code to try and identify a problem. In any case, this might be where your problem is:

    while (my $bytesread = read(READFILE, my $buffer, 1024)) { syswrite(OUTFILE, my $buffer, 1024); }

    It looks like you're localizing $buffer within the while loop, which has the effect of setting it to 'undef' before you write anything out. Get rid of that second 'my' within your loop. You've already declared $buffer.

    I also noticed some strange stuff in your code that either is causing you problems or probably will:

    if ($file ne "" & $file =~ /image/){

    Did you mean && here? Also (and more seriously):

    if(open(OUTFILE, "> $TEMP_DIR/$html_lead$file$file_ext") ...

    Be sure this line runs cleanly with taint-checking (-T) enabled. It looks to me like you might be opening yourself up to a very serious security problem, but I haven't examined the code too closely to know whether or not you've adjusted for this. See this recent reply I made to someone making a very similar mistake: Re: Using Variables in Path Names

    I'm sure I don't have to say it, but make sure you're running with strict and warnings enabled, and in this case, taint-checking would be very helpful as well (see perlsec).

andye file upload
by andye (Curate) on Dec 05, 2001 at 16:08 UTC
    MadPogo,

    update: had the wrong end of the stick. Perhaps $path =~ s/^.*(\\|\/)//; is turning $path from a filehandle into a scalar? Suggest you use this syntax:

    $fh = $query->upload('uploaded_file'); while (<$fh>) { print; }

    update 2: OK, I've worked it out. You're passing the filenames from page to page in a hidden field, right? As soon as you pass them back to the client, they stop being filehandles, and are just scalars.

    andy.

Re: Uploading an image
by AidanLee (Chaplain) on Dec 05, 2001 at 11:10 UTC
    you might find my response to this node useful.