Skip Menu |

This queue is for tickets about the CGI-Uploader CPAN distribution.

Report information
The Basics
Id: 14006
Status: resolved
Priority: 0/
Queue: CGI-Uploader

People
Owner: MARKSTOS [...] cpan.org
Requestors: steven [...] knowmad.com
Cc:
AdminCc:

Bug Information
Severity: Unimportant
Broken in: 1.1_1
Fixed in: (no value)



Subject: gen_thumb cannot handle a filehandle
In your Synopsis example code you use &gen_thumb; however this code does not work, as &gen_thumb (or more namely Image::Magick) cannot handle the filehandle that is passed to it by CGI::Uploader. I have worked around the issue, and included a diff that will show you what I did.
--- ImageMagick.pm 2005-04-02 13:32:16.000000000 -0500 +++ /home/steven/work/tp/CalendarKidz/perl5/CGI/Uploader/Transform/ImageMagick.pm 2005-08-03 16:26:04.000000000 -0400 @@ -4,6 +4,7 @@ use File::Temp qw/tempfile/; use Params::Validate (qw/:all/); use Carp::Assert; +use Data::Dumper; use vars (qw/@EXPORT $VERSION/); $VERSION = 1.1_1; @@ -49,6 +50,10 @@ h => { type => SCALAR | UNDEF, regex => qr/^\d*$/, optional => 1 }, }); die "must supply 'w' or 'h'" unless (defined $p{w} or defined $p{h}); + die "File not found - $orig_filename" unless -e $orig_filename; + warn "Going to xform '$orig_filename'"; + warn "Ref = " . ref($orig_filename); + warn "Dump = " . Dumper($orig_filename); # Having both Graphics::Magick and Image::Magick loaded at the same time # can cause very strange problems, so we take care to avoid that @@ -60,6 +65,8 @@ } elsif (exists $INC{'Image/Magick.pm'}) { $magick_module = 'Image::Magick'; + # Workaround for Image::Magick filehandle limitations. + $orig_filename = $self->{'updir_path'} . '/' . scalar($orig_filename) if ref($orig_filename); } # If neither are already loaded, try loading either one.
Date: Wed, 3 Aug 2005 19:16:30 -0500
From: Mark Stosberg <mark [...] summersault.com>
To: Guest via RT <bug-CGI-Uploader [...] rt.cpan.org>
Subject: Re: [cpan #14006] gen_thumb cannot handle a filehandle
RT-Send-Cc:
On Wed, Aug 03, 2005 at 05:03:43PM -0400, Guest via RT wrote: Show quoted text
> > In your Synopsis example code you use &gen_thumb; however this code > does not work, as &gen_thumb (or more namely Image::Magick) cannot > handle the filehandle that is passed to it by CGI::Uploader. I have > worked around the issue, and included a diff that will show you what I > did.
Hello Steven, Thanks for the feedback. I have some questions about your patch. + die "File not found - $orig_filename" unless -e $orig_filename; This is already checked, with: die "Error while reading $orig_filename: $err" if $err; + warn "Going to xform '$orig_filename'"; + warn "Ref = " . ref($orig_filename); + warn "Dump = " . Dumper($orig_filename); Were you suggesting I include these lines in the module? + # Workaround for Image::Magick filehandle limitations. + $orig_filename = $self->{'updir_path'} . '/' . scalar($orig_filename) if ref($orig_filename); I don't understand this line. The documentation has $orig_filename being a scalar when it comes it. So it shouldn't be necessary to call ref() on it, and if it /were/ a reference, calling scalar() on a reference doesn't do anything but it in scalar context. You review the automated tests for gen_thumb shipped with the distribution to see that it does actually work with Image::Magick. If you believe you have found a bug that is not covered by those existing tests, please add an additional test which illustrates your bug. I did find as i reviewed the Transform/ImageMagick module that there was a documentation bug. I made this patch for it: - filename => $orig_filename, - w => $width, - h => $height, + $orig_filename, + [ w => $width, h => $height ] However, I think the examples in the main CGI::Uploader documentation were correct. Mark -- http://mark.stosberg.com/
From: william [...] knowwmad.com
Show quoted text
> Were you suggesting I include these lines in the module? > > + # Workaround for Image::Magick filehandle limitations. > + $orig_filename = $self->{'updir_path'} . '/' . > scalar($orig_filename) if ref($orig_filename); > > I don't understand this line. The documentation has $orig_filename > being a scalar when it comes it.
Hi Mark, The problem we've been facing is that the $orig_filename is coming in as a filehandle, not a reference. That line was suppose to turn it back into a scalar of the absolute path to the file so that Image::Magick could use it. In further debugging this evening, I found that Uploader.pm is calling gen_thumb in two different ways--once in store_upload with $file_name and a second time in create_store_gen_file with $tmp_filename. Substituting the $tmp_filename in the first call seems to eliminate the need for the workaround. I'm still having a problem with thumbnails but believe it is a different issue. William
From: william [...] knowwmad.com
Mark, I've successfully worked around this issue by passing $tmp_filename from store_upload() in CGI::Uploader. I'm attaching a patch to ImageMagick.pm with some addt'l debugging checks for the _cal_target_size which was producing errors when I had the issues mentioned earlier. Perhaps they will be useful. Thanks, William
--- CGI-Uploader-1.1_1-orig/lib/CGI/Uploader/Transform/ImageMagick.pm 2005-04-02 13:32:16.000000000 -0500 +++ /home/william/work/tp/CalendarKidz/perl5/CGI/Uploader/Transform/ImageMagick.pm 2005-08-04 02:12:53.000000000 -0400 @@ -103,7 +103,7 @@ } -# Calculate the target with height +# Calculate the target width and height # # my ($target_w,$target_h) = _calc_target_size($img,$p{w},$p{h}) # @@ -116,9 +116,14 @@ sub _calc_target_size { my ($img,$w,$h) = @_; + if (defined $w) { die "Invalid width: $w" unless $w > 0; } + if (defined $h) { die "Invalid height: $h" unless $h > 0; } + my $target_h = $h; my $target_w = $w; my ($orig_w,$orig_h) = $img->Get('width','height'); + die "Could not determine original width of image: $orig_w" unless defined $orig_w && $orig_w > 0; + die "Could not determine original height of image: $orig_h" unless defined $orig_h && $orig_h > 0; $target_h = sprintf("%.1d", ($orig_h * $target_w) / $orig_w) unless $target_h; $target_w = sprintf("%.1d", ($orig_w * $target_h) / $orig_h) unless $target_w;
Date: Thu, 4 Aug 2005 21:23:42 -0500
From: Mark Stosberg <mark [...] summersault.com>
To: Guest via RT <bug-CGI-Uploader [...] rt.cpan.org>
Subject: Re: [cpan #14006] gen_thumb cannot handle a filehandle
RT-Send-Cc:
On Thu, Aug 04, 2005 at 02:16:24AM -0400, Guest via RT wrote: Show quoted text
> > I've successfully worked around this issue by passing $tmp_filename from > store_upload() in CGI::Uploader. I'm attaching a patch to ImageMagick.pm > with some addt'l debugging checks for the _cal_target_size which was > producing errors when I had the issues mentioned earlier. Perhaps they > will be useful.
William, Any chance of getting an automated test to confirm the bug as part of your submission? I'm also curious why the existing test suite didn't catch this. Mark -- http://mark.stosberg.com/