Re: [cgiapp] Download a file

[Josh Jore <suppressed>]

On 5 Dec 2002, Stelian Iancu wrote:

> On Thu, 2002-12-05 at 12:00, Stelian Iancu wrote:
>
> Here is the sub I am using:
>
> sub download_method {
>         my $self = shift;
>         my $q = $self->query;
>         my $file = $q->param('path');
>         my ($dev,$ino,$mode,$nlink,$uid,$gid,$rdev,$size,
>     $atime,$mtime,$ctime,$blksize,$blocks)
>             = stat $file;
>
>         $self->header_props(-type=>'octet/stream', -attachment=>$file,
> -length=>$size);
> }

Just a stylistic nit: you're only using one value from stat(), only
extract that part. It's better practice to not clutter your code with
spurious variables that aren't actually being used.

my $size = (stat $file)[7];

You can also use File::stat and then do:

my $st = stat($file) or die "Couldn't stat $file: $!";
$self->header_props( -type       => 'octet/stream',
                     -attachment => $file,
                     -length     => $size );

There's also the issue that you're doing a terribly unsecure thing: taking
arbitrary input from the web browser and using it directly without any
sanity checks at all. David Wheeler has a HOWTO on the subject which is
a reasonable place to start http://www.dwheeler.com/secure-programs/. The
really short version is that your current use will allow "evil hackers" to
download most anything on your server. Also read the perlsec manual page.

Lastly... stat() is a system call and you should really be checking the
return value and $! for errors. The most probably problem is that your
$file doesn't actually exist where you think it does. If you don't yet
understand relative and absolute file paths then you should find out
immediately.

Josh



---------------------------------------------------------------------
Web Archive:  http://www.mail-archive.com/suppressed/
To unsubscribe, e-mail: suppressed
For additional commands, e-mail: suppressed