Octavian Rasnita:

> Hi all,
>
> I am fighting with the following short test script, and I can make it work
> only if I use a subroutine.
> I want to make it work without a subroutine.
>
> The script works as it is, but if I remove the line that runs the
> subroutine, and the
> first and last lines of the subroutine (sub up { ... and }), the script
> doesn't want to run normally.
>
> It will copy the file, but in addition, it will create the same file but
> with a strange name (CGItem0995... or other random numbers) in my cgi-bin
> directory.
>
> Why is different if I use the subroutine?

   I should have caught the fact that you were trying to do a file upload in
your original post. Not having a use CGI; threw me off.

   The value returned by $cgiobject->param{"file") _is_ a filehandle, unlike
the hardcoded value that you were using in your test script. So you don't
need to call open, but can read directly from it. (And in fact calling open
here doesn't do what you want.)

   I'm not sure, but I think the reason that the behavior varies is this:
when you use CGI.pm's upload feature it creates a temporary file (at least
by default). Under Win32 this file cannot be deleted unless you close the
filehandle. When you create the filehandle in the subroutine using my it is
lexically scoped to the subroutine, and is closed when it goes out of scope.
So you need to do:

close $file;

after you are done reading from it.

There are some subtleties to uploading files. You should look at:

http://stein.cshl.org/WWW/software/CGI/cgi_docs.html#upload_caveats

I put a couple of notes in your code below. I'm not an expert on file upload
using CGI.pm, so I'm sure that aomeone on the list who is will have
something to say about it too. Please see my disclaimer at the end :).

>
> Thank you. Here is the script:
>
> #!c:\perl\bin\perl.exe

Again, you should use the -w switch. Since this is a cgi script you should
also use -T to turn on taint checking.

> use CGI;
> my $GET = new CGI; # create a new object

   It's a bit confusing when you give variables all-caps names, as that's
usually used for file handles and such.

> print "Content-type: text/html\n\n";
> print "Upload completed<br>";

   You might want to hold off on saying upload completed until you know that
it has been.
>
> &up; #I tried to comment this line
>
> sub up { #I tried to comment this line too.
> my $file = $GET->param("file");
> open (NEW, ">c:\\Program Files\\Apache Group\\Apache\\images\\MAIL.dbx");

Are you always going to be uploading to this file? Or is this just for
testing? If you want to upload to a file in some directory, with the
filename of the file the user submitted it will take a couple more steps
(hint: the name of the filehandle is the name of the remote file, possibly
including its full path).

Again, you should always check that a call to open has been successful.

> binmode NEW;

> open (INPUT, "$file");
> binmode INPUT;
You don't need this now, because you got a filehandle from the cgiobject.

> while (read(INPUT, $buffer, 1024)) {

This is now  while (read($file, $buffer, 1024)) {

But you might want to do something like this beforehand:

   if (!$file){
      #print an error and exit the script
   }

To check that the user did submit a file.

> print NEW $buffer;
> }
> close NEW;
> close INPUT;

This should now be:
    close $file;

if you aren't doing this in a subroutine.

> #End the subroutine
> }    # I also tried to comment this line
>

   There's quite a lot more to file uploading. At the very least this script
opens you up to a denial of service attack, if someone uploads very large
files to your machine. There are in fact a lot of potential security holes,
not the least of which is that your temp files are being written into your
cgi-bin directory. The link above gives instructions for avoiding this. I'm
not comfortable trying to give you an enumerated list of potential problems
and solutions- maybe someone with a lot of esperience doing file uploads
through cgi will be willing to. Be very careful about putting a file upload
script on a public webserver- make sure that someone who really knows what
they're doing has a good look at it before you do.

Tagore Smith


-- 
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to