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.

diff --git a/includes/Image.php b/includes/Image.php
index c2bf39f..2774508 100644 (file)
--- 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 f57e9d7..2e6e94e 100644 (file)
--- a/includes/ImagePage.php
+++ b/includes/ImagePage.php
@@ -248,53 +248,39 @@ class ImagePage extends Article {
                                     htmlspecialchars( $this->img->getTitle()->getPrefixedText() ).'" />' . $anchorclose . '</div>' );
 
                                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 = '<form name="pageselector" action="' . $this->img->getEscapeLocalUrl( '' ) . '" method="GET" onchange="document.pageselector.submit();">' ;
-                                               $select .= $wgOut->parse( wfMsg( 'imgmultigotopre' ), false ) .
-                                                       ' <select id="pageselector" name="page">';
-                                               for ( $i=1; $i <= $count; $i++ ) {
-                                                       $select .= Xml::option( $wgLang->formatNum( $i ), $i,
-                                                               $i == $page );
-                                               }
-                                               $select .= '</select>' . $wgOut->parse( wfMsg( 'imgmultigotopost' ), false ) .
-                                                       '<input type="submit" value="' .
-                                                       htmlspecialchars( wfMsg( 'imgmultigo' ) ) . '"></form>';
-       
-                                               $wgOut->addHTML( '</td><td><div class="multipageimagenavbox">' .
-                                                  "$select<hr />$thumb1\n$thumb2<br clear=\"all\" /></div></td></tr></table>" );
-                                       } catch( Exception $e ) {
-                                               if ( $e->getText() == "String could not be parsed as XML" ) {
-                                                       // Hacky and fragile string test, yay
-                                                       $errorMsg = wfMsg( 'imgmultiparseerror' );
-                                                       $wgOut->addHtml( "</td><td>$errorMsg</td></tr></table>" );
-                                               } 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 = '<form name="pageselector" action="' . $this->img->getEscapeLocalUrl( '' ) . '" method="GET" onchange="document.pageselector.submit();">' ;
+                                       $select .= $wgOut->parse( wfMsg( 'imgmultigotopre' ), false ) .
+                                               ' <select id="pageselector" name="page">';
+                                       for ( $i=1; $i <= $count; $i++ ) {
+                                               $select .= Xml::option( $wgLang->formatNum( $i ), $i,
+                                                       $i == $page );
+                                       }
+                                       $select .= '</select>' . $wgOut->parse( wfMsg( 'imgmultigotopost' ), false ) .
+                                               '<input type="submit" value="' .
+                                               htmlspecialchars( wfMsg( 'imgmultigo' ) ) . '"></form>';
+
+                                       $wgOut->addHTML( '</td><td><div class="multipageimagenavbox">' .
+                                          "$select<hr />$thumb1\n$thumb2<br clear=\"all\" /></div></td></tr></table>" );
                                }
                        } else {
                                #if direct link is allowed but it's not a renderable image, show an icon.