I haven't gone through your code too carefully, but one glaring thing stood out. In your print_results() sub, you have $CGI::DISABLE_UPLOADS = 0;. If CGI.pm has uploads disabled by default, you need to reenable it before you instantiate your CGI object (or before your first use of a :cgi function if using the function-oriented interface). It's when you instantiate the object that CGI.pm grabs all of its data.

    # Good
    $CGI::DISABLE_UPLOADS = 0;
    my $q = CGI->new;

    # Bad
    my $q = CGI->new;
    $CGI::DISABLE_UPLOADS = 0;
[download]

Oh yeah, and use strict. :)

michellem wrote:

I virtually always use strict as a matter of course. This was an exception to my usual rule, because I knew beforehand that strict would complain about using a string as a filehandle.

So you'll disable strict for the entire program because of one line of code? Just turn off strict for that scope:

{
    no strict 'refs';
    while (<$file>) {
        $length += length($_);
    }
}
[download]

Cheers,
Ovid

Update: Okay, found the problem and fixed it. The upload method takes a filename, not a filehandle. Your code passed the filehandle/filename thingy that Stein created, so I don't know if this caused the problem or not, but I changed it to a constant to avoid any confusion. Also, you used sysopen for a file without specifying O_WRONLY. How do you write to a file that you didn't specify that you were going to write to? :)This could use some work, but here is some working code.

#!D:/perl/bin/perl.exe -wT
use CGI qw/ :standard /;
use Fcntl;
use strict;
$|++;

$CGI::DISABLE_UPLOADS = 0;
use constant BUFFER_SIZE   => 16384;
use constant UPLOAD_FILE   => 'file_upload';
use constant FILESHARE_DIR => 'xi_fileshare.dir';

print header,
    start_html('file upload'),
    h1('file upload');
    
my $dir;

if ( param('file_upload') ) {
    $dir = print_results();
    print a( { href => "xi_fileshare.cgi?directory=$dir"}, "Back to fi
+le listing" );
    exit;
} else {
    print_form();
}
print end_html;
exit;

sub print_form {
    open FILE, ">".FILESHARE_DIR or graceful_exit("Can't write to dire
+ctory file: $!");
    my $directory = param( 'directory' );
    print FILE $directory;
    close FILE;

    print br,
          start_multipart_form(),
          filefield(-name=>UPLOAD_FILE,-size=>60),
          br,
          submit(-label=>'Upload File'),
          end_form;
}

sub print_results {
    my $file = param( UPLOAD_FILE );
    if ( ! $file ) {
        graceful_exit("No File!");
    }
    my $content_type = uploadInfo($file)->{'Content-Type'};
    my $file_handle = upload( UPLOAD_FILE );
    my $testfilename="whatever.xls";

    open FILE, "<".FILESHARE_DIR or graceful_exit("Can't file director
+y file: $!");
    chomp( my $directory = <FILE> );
    sysopen (OUTFILE, "$directory/$testfilename", O_WRONLY | O_CREAT) 
+or graceful_exit("Can't create file: $!!");

    my $length = 0;
    while ( read( $file_handle, my $buffer, BUFFER_SIZE ) ) {
        print pre( $buffer );
        print OUTFILE $buffer or graceful_exit( "Can't print to $direc
+tory/$testfilename: $!" ); 
        $length += length $buffer;
    }

    print p( "$directory/$testfilename" ),
          br,
          p( "Dir: $directory" ),
          h2('File name'),
          p( $file ),
          h2('File Mime Type'),
          p( $content_type ),
          h2('File Length'),
          p( $length );
    close (OUTFILE); 
}

sub graceful_exit {
    my $err = shift;
    print h3("Sorry, but an error in your input has occured! If you ca
+n figure it out, this is it: $err"),
          p( "Use your browser's <b>BACK</b> button and try again with
+ changed input" ),
          br,
          end_html;
    exit;
}
[download]

The code is strict compliant, I've taken out some duplicate code and changed repeated variables to constants. I've also cleaned up your error messages so they're a bit more useful. This could use a bit more work, but it should do for now.

Vote for paco!

Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.

I changed the position of the $CGI::Disable uploads - and that had no effect. Then I checked - uploads aren't disabled by default, so I removed that entirely. I still have the same problem - empty files in the upload directory. I think I must be doing something dumb, but I just can't see it.

And yes, you are right about disabling strict - so I've changed my practice. :-)

Ovid,
Thanks first for wrangling this code - your solutions worked really well. I have a couple of questions about your code, if you don't mind enlightening me, I'd appreciate it. (Always an apprentice!)
The most important question is why did you switch from the object oriented form of CGI to the function-oriented form?
Second question is using sysopen rather than open? I know that sysopen in general provides for more control - locking, etc. Is it generally better to use it than regular open?
Thanks!

To answer your questions: I switched from the object-oriented interface to the function oriented interface because you were using both and I decided to stick with one or the other:

    use CGI ':all';
    use Fcntl;
    use strict;

    my $q = new CGI;
    print header,
          start_html('file upload'),
          h1('file upload');
[download]

From the snippet above, you can see that you're instantiating a new CGI object but you're also using the start_html and h1 functions that you've imported into your namespace with ":all". There are other examples in your program, but I decided to just use the function-oriented interface as it makes the code look a bit cleaner (though there are times that the object-oriented interface can give you some fine-grained control that the function oriented interface cannot). If I were to have switched to the OO interface, the above snippet would have looked like this:

    use CGI;
    use Fcntl;
    use strict;

    my $q = CGI->new;
    print $q->header,
          $q->start_html('file upload'),
          $q->h1('file upload');
[download]

To be frank, the reason I used sysopen is because that's what you were using :) I wasn't really focusing on making sure that everything was exactly what you needed. I was just trying to get the darned thing to work.

sysopen has the advantage of offering finer control over opening files than open. For instance, you can open files with sysopen and specify that they not exist when you try to open that. With open, you have to add extra code to test for existence of the file.

Cheers,
Ovid

Vote for paco!

Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.

use strict; isn't something to tack on after a script is finished, it should be one of the first things you type. It makes debugging everything much easier, especially in the case of misused/mistyped variables.

That being said, perhaps this node would be a help to you.

Are you sure that your permissions on the directory you're trying to write to are correct? This is a major source of confusion for folks oftentimes.

-Any sufficiently advanced technology is
indistinguishable from doubletalk.

I virtually always use strict as a matter of course. This was an exception to my usual rule, because I knew beforehand that strict would complain about using a string as a filehandle.

Also, this is only a small part of the whole application - yes, all of the permissions are set correctly.

I've gotten that error before when my HTML form wasn't set up correctly. View the source of your form after it prints and make sure the form tag has the type="multipart/form-data" in it. I know you are using CGI and its supposed to that but make sure because the errors you are describing point that direction.