From 455e77d168c6934b7919e14dc6c38e2961f6bd02 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 26 Jan 2007 12:00:04 +0000 Subject: [PATCH] Fix fix for 8652 (failures with bad XML metadata for DjVu files) The mid-level functions in Image now fail gracefully on bogus data, as happens when the djvutoxml executable is missing. Removed the bogus exception check in ImagePage high-level code (which actually just causes a fatal error by calling a nonexistent method :) Also changed Image::initializeMultiPageXML to do clean lazy loading as Image::load does, and to return true/false as to success/failure for cleaner code using it. --- includes/Image.php | 51 ++++++++++++++++++---------- includes/ImagePage.php | 76 +++++++++++++++++------------------------- 2 files changed, 65 insertions(+), 62 deletions(-) diff --git a/includes/Image.php b/includes/Image.php index c2bf39fc3e..27745084d8 100644 --- a/includes/Image.php +++ b/includes/Image.php @@ -2251,25 +2251,34 @@ class Image * @param $page Integer: page number, starting with 1 */ function selectPage( $page ) { - wfDebug( __METHOD__." selecting page $page \n" ); - $this->page = $page; - if ( ! $this->dataLoaded ) { - $this->load(); - } - if ( ! isset( $this->multiPageXML ) ) { - $this->initializeMultiPageXML(); + if( $this->initializeMultiPageXML() ) { + wfDebug( __METHOD__." selecting page $page \n" ); + $this->page = $page; + $o = $this->multiPageXML->BODY[0]->OBJECT[$page-1]; + $this->height = intval( $o['height'] ); + $this->width = intval( $o['width'] ); + } else { + wfDebug( __METHOD__." selectPage($page) for bogus multipage xml on '$this->name'\n" ); + return; } - $o = $this->multiPageXML->BODY[0]->OBJECT[$page-1]; - $this->height = intval( $o['height'] ); - $this->width = intval( $o['width'] ); } + /** + * Lazy-initialize multipage XML metadata for DjVu files. + * @return bool true if $this->multiPageXML is set up and ready; + * false if corrupt or otherwise failing + */ function initializeMultiPageXML() { + $this->load(); + if ( isset( $this->multiPageXML ) ) { + return true; + } + # - # Check for files uploaded prior to DJVU support activation - # They have a '0' in their metadata field. + # Check for files uploaded prior to DJVU support activation, + # or damaged. # - if ( $this->metadata == '0' || $this->metadata == '' ) { + if( empty( $this->metadata ) || $this->metadata == serialize( array() ) ) { $deja = new DjVuImage( $this->imagePath ); $this->metadata = $deja->retrieveMetaData(); $this->purgeMetadataCache(); @@ -2283,8 +2292,14 @@ class Image ); } wfSuppressWarnings(); - $this->multiPageXML = new SimpleXMLElement( $this->metadata ); + try { + $this->multiPageXML = new SimpleXMLElement( $this->metadata ); + } catch( Exception $e ) { + wfDebug( "Bogus multipage XML metadata on '$this->name'\n" ); + $this->multiPageXML = null; + } wfRestoreWarnings(); + return isset( $this->multiPageXML ); } /** @@ -2305,10 +2320,12 @@ class Image if ( ! $this->isMultipage() ) { return null; } - if ( ! isset( $this->multiPageXML ) ) { - $this->initializeMultiPageXML(); + if( $this->initializeMultiPageXML() ) { + return count( $this->multiPageXML->xpath( '//OBJECT' ) ); + } else { + wfDebug( "Requested pageCount() for bogus multi-page metadata for '$this->name'\n" ); + return null; } - return count( $this->multiPageXML->xpath( '//OBJECT' ) ); } } //class diff --git a/includes/ImagePage.php b/includes/ImagePage.php index f57e9d754a..2e6e94e4d3 100644 --- a/includes/ImagePage.php +++ b/includes/ImagePage.php @@ -248,53 +248,39 @@ class ImagePage extends Article { htmlspecialchars( $this->img->getTitle()->getPrefixedText() ).'" />' . $anchorclose . '' ); if ( $this->img->isMultipage() ) { - try { - // Apparently SimpleXmlElement->__construct() can throw - // exceptions for malformed XML, although this isn't documented . . . - // http://us2.php.net/manual/en/function.simplexml-element-construct.php + $count = $this->img->pageCount(); - $count = $this->img->pageCount(); - - if ( $page > 1 ) { - $label = $wgOut->parse( wfMsg( 'imgmultipageprev' ), false ); - $link = $sk->makeLinkObj( $this->mTitle, $label, 'page='. ($page-1) ); - $this->img->selectPage( $page - 1 ); - $thumb1 = $sk->makeThumbLinkObj( $this->img, $link, $label, 'none' ); - } else { - $thumb1 = ''; - } - - if ( $page < $count ) { - $label = wfMsg( 'imgmultipagenext' ); - $this->img->selectPage( $page + 1 ); - $link = $sk->makeLinkObj( $this->mTitle, $label, 'page='. ($page+1) ); - $thumb2 = $sk->makeThumbLinkObj( $this->img, $link, $label, 'none' ); - } else { - $thumb2 = ''; - } - - $select = '
' ; - $select .= $wgOut->parse( wfMsg( 'imgmultigotopre' ), false ) . - ' ' . $wgOut->parse( wfMsg( 'imgmultigotopost' ), false ) . - '
'; - - $wgOut->addHTML( '
' . - "$select
$thumb1\n$thumb2
" ); - } catch( Exception $e ) { - if ( $e->getText() == "String could not be parsed as XML" ) { - // Hacky and fragile string test, yay - $errorMsg = wfMsg( 'imgmultiparseerror' ); - $wgOut->addHtml( "$errorMsg" ); - } else { - throw $e; - } + if ( $page > 1 ) { + $label = $wgOut->parse( wfMsg( 'imgmultipageprev' ), false ); + $link = $sk->makeLinkObj( $this->mTitle, $label, 'page='. ($page-1) ); + $this->img->selectPage( $page - 1 ); + $thumb1 = $sk->makeThumbLinkObj( $this->img, $link, $label, 'none' ); + } else { + $thumb1 = ''; } + + if ( $page < $count ) { + $label = wfMsg( 'imgmultipagenext' ); + $this->img->selectPage( $page + 1 ); + $link = $sk->makeLinkObj( $this->mTitle, $label, 'page='. ($page+1) ); + $thumb2 = $sk->makeThumbLinkObj( $this->img, $link, $label, 'none' ); + } else { + $thumb2 = ''; + } + + $select = '
' ; + $select .= $wgOut->parse( wfMsg( 'imgmultigotopre' ), false ) . + ' ' . $wgOut->parse( wfMsg( 'imgmultigotopost' ), false ) . + '
'; + + $wgOut->addHTML( '
' . + "$select
$thumb1\n$thumb2
" ); } } else { #if direct link is allowed but it's not a renderable image, show an icon. -- 2.20.1