From 47f4d0ec2485e0fe8b6a178d93a2f8d29a9d7f71 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Fri, 4 Nov 2011 23:49:03 +0000 Subject: [PATCH] * Added File::normalizeTitle() function and used it to consolidate file title validity checks and sanitation. (bug 32195) * Broke a few long lines. --- includes/filerepo/ArchivedFile.php | 6 ++-- includes/filerepo/File.php | 38 ++++++++++++++++++--- includes/filerepo/FileRepo.php | 2 +- includes/filerepo/LocalRepo.php | 9 ++--- includes/filerepo/RepoGroup.php | 17 +++------ includes/filerepo/UnregisteredLocalFile.php | 14 ++++---- 6 files changed, 53 insertions(+), 33 deletions(-) diff --git a/includes/filerepo/ArchivedFile.php b/includes/filerepo/ArchivedFile.php index 6043d262d3..34044ba174 100644 --- a/includes/filerepo/ArchivedFile.php +++ b/includes/filerepo/ArchivedFile.php @@ -73,8 +73,8 @@ class ArchivedFile { $this->dataLoaded = false; $this->exists = false; - if( is_object( $title ) ) { - $this->title = $title; + if( $title instanceof Title ) { + $this->title = File::normalizeTitle( $title, 'exception' ); $this->name = $title->getDBkey(); } @@ -86,7 +86,7 @@ class ArchivedFile { $this->key = $key; } - if ( !$id && !$key && !is_object( $title ) ) { + if ( !$id && !$key && !( $title instanceof Title ) ) { throw new MWException( "No specifications provided to ArchivedFile constructor." ); } } diff --git a/includes/filerepo/File.php b/includes/filerepo/File.php index e78eca4104..ca340c818d 100644 --- a/includes/filerepo/File.php +++ b/includes/filerepo/File.php @@ -61,12 +61,12 @@ abstract class File { */ /** - * @var LocalRepo + * @var FileRepo */ var $repo; /** - * @var Title + * @var Title|false */ var $title; @@ -87,14 +87,44 @@ abstract class File { /** * Call this constructor from child classes * - * @param $title - * @param $repo + * @param $title Title|false + * @param $repo FileRepo|false */ function __construct( $title, $repo ) { + if ( $title !== false ) { // account for UnregisteredLocalFile et all + $title = self::normalizeTitle( $title, 'exception' ); + } $this->title = $title; $this->repo = $repo; } + /** + * Given a string or Title object return either a + * valid Title object with namespace NS_FILE or null + * @param $title Title|string + * @param $exception string|false Use 'exception' to throw an error on bad titles + * @return Title|null + */ + static function normalizeTitle( $title, $exception = false ) { + $ret = $title; + if ( $ret instanceof Title ) { + # Normalize NS_MEDIA -> NS_FILE + if ( $ret->getNamespace() == NS_MEDIA ) { + $ret = Title::makeTitleSafe( NS_FILE, $ret->getDBkey() ); + # Sanity check the title namespace + } elseif ( $ret->getNamespace() !== NS_FILE ) { + $ret = null; + } + } else { + # Convert strings to Title objects + $ret = Title::makeTitleSafe( NS_FILE, (string)$ret ); + } + if ( !$ret && $exception !== false ) { + throw new MWException( "`$title` is not a valid file title." ); + } + return $ret; + } + function __get( $name ) { $function = array( $this, 'get' . ucfirst( $name ) ); if ( !is_callable( $function ) ) { diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php index 4efd42e9be..20a13438f3 100644 --- a/includes/filerepo/FileRepo.php +++ b/includes/filerepo/FileRepo.php @@ -644,7 +644,7 @@ abstract class FileRepo { * @param $title Title of image * @return Bool */ - function checkRedirect( $title ) { + function checkRedirect( Title $title ) { return false; } diff --git a/includes/filerepo/LocalRepo.php b/includes/filerepo/LocalRepo.php index 373fda1b5d..f941045d3f 100644 --- a/includes/filerepo/LocalRepo.php +++ b/includes/filerepo/LocalRepo.php @@ -137,15 +137,10 @@ class LocalRepo extends FSRepo { * * @param $title Title of file */ - function checkRedirect( $title ) { + function checkRedirect( Title $title ) { global $wgMemc; - if( is_string( $title ) ) { - $title = Title::newFromText( $title ); - } - if( $title instanceof Title && $title->getNamespace() == NS_MEDIA ) { - $title = Title::makeTitle( NS_FILE, $title->getText() ); - } + $title = File::normalizeTitle( $title, 'exception' ); $memcKey = $this->getSharedCacheKey( 'image_redirect', md5( $title->getDBkey() ) ); if ( $memcKey === false ) { diff --git a/includes/filerepo/RepoGroup.php b/includes/filerepo/RepoGroup.php index 4fe7df3adb..1e6538035e 100644 --- a/includes/filerepo/RepoGroup.php +++ b/includes/filerepo/RepoGroup.php @@ -108,22 +108,15 @@ class RepoGroup { if ( !$this->reposInitialised ) { $this->initialiseRepos(); } - if ( !($title instanceof Title) ) { - $title = Title::makeTitleSafe( NS_FILE, $title ); - if ( !is_object( $title ) ) { - return false; - } - } - - if ( $title->getNamespace() != NS_MEDIA && $title->getNamespace() != NS_FILE ) { - throw new MWException( __METHOD__ . ' received an Title object with incorrect namespace' ); + $title = File::normalizeTitle( $title ); + if ( !$title ) { + return false; } # Check the cache if ( empty( $options['ignoreRedirect'] ) && empty( $options['private'] ) - && empty( $options['bypassCache'] ) - && $title->getNamespace() == NS_FILE ) + && empty( $options['bypassCache'] ) ) { $useCache = true; $time = isset( $options['time'] ) ? $options['time'] : ''; @@ -201,7 +194,7 @@ class RepoGroup { /** * Interface for FileRepo::checkRedirect() */ - function checkRedirect( $title ) { + function checkRedirect( Title $title ) { if ( !$this->reposInitialised ) { $this->initialiseRepos(); } diff --git a/includes/filerepo/UnregisteredLocalFile.php b/includes/filerepo/UnregisteredLocalFile.php index 3922a0d753..f0eeec0816 100644 --- a/includes/filerepo/UnregisteredLocalFile.php +++ b/includes/filerepo/UnregisteredLocalFile.php @@ -46,7 +46,7 @@ class UnregisteredLocalFile extends File { /** * @throws MWException - * @param $title string + * @param $title Title|false * @param $repo FSRepo * @param $path string * @param $mime string @@ -55,18 +55,19 @@ class UnregisteredLocalFile extends File { if ( !( $title && $repo ) && !$path ) { throw new MWException( __METHOD__.': not enough parameters, must specify title and repo, or a full path' ); } - if ( $title ) { - $this->title = $title; + if ( $title instanceof Title ) { + $this->title = File::normalizeTitle( $title, 'exception' ); $this->name = $repo->getNameFromTitle( $title ); } else { $this->name = basename( $path ); - $this->title = Title::makeTitleSafe( NS_FILE, $this->name ); + $this->title = File::normalizeTitle( $this->name, 'exception' ); } $this->repo = $repo; if ( $path ) { $this->path = $path; } else { - $this->path = $repo->getRootDirectory() . '/' . $repo->getHashPath( $this->name ) . $this->name; + $this->path = $repo->getRootDirectory() . '/' . + $repo->getHashPath( $this->name ) . $this->name; } if ( $mime ) { $this->mime = $mime; @@ -122,7 +123,8 @@ class UnregisteredLocalFile extends File { function getURL() { if ( $this->repo ) { - return $this->repo->getZoneUrl( 'public' ) . '/' . $this->repo->getHashPath( $this->name ) . rawurlencode( $this->name ); + return $this->repo->getZoneUrl( 'public' ) . '/' . + $this->repo->getHashPath( $this->name ) . rawurlencode( $this->name ); } else { return false; } -- 2.20.1