Bug #37810 for CGI-Application-PhotoGallery: Error handling errors

Sun Jul 20 08:25:29 2008 tlhackque [...] yahoo.com - Ticket created

Subject:

Error handling errors

handle_error returns a 500 status (internal server error) when the error isn't file not found. This generally causes a content-free error page due to the usual microsoft 'feature' of providing a 'helpful' error page. It also causes errors in the system log. handle_error should return status 200 even in the error case; the error template will then display as expected. This was discovered because thumbnail doesn't trap errors doing the resize. This makes them hard to track down. Even when you see the error, it's a mysterious internal error in the graphics library. In this case, it turned out to be a .JPG file that was corrupted. We can eval the resize & return a more meaningful error. In fact, in this particular case, we can return an error image so that the whole gallery doesn't crash because of a single corrupt file. Patch attached.

Subject:

pg_error_patch2.diff

--- /usr/lib/perl5/site_perl/5.8.8/CGI/Application/PhotoGallery.pm~ 2008-03-11 11:23:32.000000000 -0400 +++ /usr/lib/perl5/site_perl/5.8.8/CGI/Application/PhotoGallery.pm 2008-07-20 08:17:23.000000000 -0400 @@ -473,7 +473,24 @@ unless ( $data ) { my $gfx = $self->gfx_lib; - $data = $gfx->resize( $path, $size ); + eval { + $data = $gfx->resize( $path, $size ); + }; if ($@) { + my $errstr = $@; + my $errimg = __FILE__; + $errimg =~ s|.pm$|/nothumb.jpg|; + if( -r $errimg ) { + eval { + $data = $gfx->resize( $errimg, $size ); + }; if ($@) { + return handle_error( $self, "Unable to resize $path; file may be corrupt; error image also corrupt\nError string:$errstr" ); + } + } else { + return handle_error( $self, "Unable to resize $path; file may be corrupt; error image also missing\nError string:$errstr\n" ); + } + binmode STDOUT; + return $data; + } $cache->set( $key => $data ); } @@ -675,7 +692,7 @@ $error = 'ERROR: File not found.'; } else { - $self->header_props( { -status => '500 Error' } ); + $self->header_props( { -status => '200 Error' } ); } my $html = $self->load_tmpl(

Sun Jul 20 08:27:24 2008 tlhackque [...] yahoo.com - Correspondence added

Subject:	Image for patch
From:	tlhackque [...] yahoo.com

Here is the error image file.

Download nothumb.jpg
image/pjpeg 3.3k

Sun Jul 20 11:31:17 2008 tlhackque [...] yahoo.com - Correspondence added

Subject:	Error handling errors redux; height scaling
From:	tlhackque [...] yahoo.com

Sorry, I uploaded an incomplete patch. Guess that's the price of having version control! Here's (hopefully) the cleaned-up version that I meant to upload. Note that the error image file goes in site_perl/5.x.x/auto/CGI/Application/PhotoGallery/ While I'm at it: the width-scaling patch provided earlier needs the equivalent for height scaling. That is also included in this patch. Enjoy.

--- /usr/lib/perl5/site_perl/5.8.8/CGI/Application/PhotoGallery.pm~ 2008-03-11 11:23:32.000000000 -0400 +++ /usr/lib/perl5/site_perl/5.8.8/CGI/Application/PhotoGallery.pm 2008-07-20 11:28:09.000000000 -0400 @@ -138,6 +138,13 @@ and height attributes on the image tag, thus saving the image will retain the full resolution. +=head2 max_height + +Setting this value will force the browser to scale images down to this +particular height and proportioned width. This is done by setting the width +and height attributes on the image tag, thus saving the image will retain the +full resolution. + =head2 cache_root Specifies where the file cache data will be stored. Defaults to FileCache @@ -473,7 +480,26 @@ unless ( $data ) { my $gfx = $self->gfx_lib; - $data = $gfx->resize( $path, $size ); + eval { + $data = $gfx->resize( $path, $size ); + }; if ($@) { + my $errstr = $@; + my $errimg = $self->_dist_file( 'nothumb.jpg' ); + + eval { + $data = $gfx->resize( $errimg, $size ); + }; if ($@) { + return $self->handle_error( "Unable to resize $path; file may be corrupt; error image also corrupt\nError string:$errstr" ); + } + + $self->header_props( + { -type => $self->mime_types->mimeTypeOf( $errimg ), + -last_modified => HTTP::Date::time2str( ( stat( $errimg ) )[ 9 ] ) + } + ); + binmode STDOUT; + return $data; + } $cache->set( $key => $data ); } @@ -585,7 +611,12 @@ my $gfx = $self->gfx_lib; - my ( $width, $height ) = $gfx->size( $path ); + my ( $width, $height ); + eval { + ($width, $height) = $gfx->size( $path ); + }; if ($@) { + return $self->handle_error("Unable to determine size of $path; file may be corrupt.\nError string:$@"); + } # get data for prev/next/parent links my ( undef, $search_dir ) = fileparse( $path ); @@ -622,6 +653,14 @@ } } + if ( defined( my $max_height = $self->param( 'max_height' ) ) ) { + if ( $height > $max_height ) { + my $scale = $max_height / $height; + $width = int( $width * $scale ); + $height = int( $height * $scale ); + } + } + $html->param( photo => $photo, width => $width, @@ -675,7 +714,7 @@ $error = 'ERROR: File not found.'; } else { - $self->header_props( { -status => '500 Error' } ); + $self->header_props( { -status => '200 Error' } ); } my $html = $self->load_tmpl(

Fri Aug 08 14:07:14 2008 bricas [...] cpan.org - Taken

Fri Aug 08 14:07:30 2008 bricas [...] cpan.org - Status changed from 'new' to 'open'