Addshore has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/364470 )
Change subject: Use TitleParser in ImportPlanValidator & check
......................................................................
Use TitleParser in ImportPlanValidator & check
ImportPlanValidator now checks a simple call of
getTitle as the first check. Failures here expose
a nice error from a MalformedTitleException to the
use in order for them to fix their title.
Bug: T163500
Change-Id: I69a1329852d187b782f85ef74cc613c1dfd727fc
---
M src/Data/ImportPlan.php
M src/Services/ImportPlanValidator.php
M tests/phpunit/Services/ImportPlanValidatorTest.php
3 files changed, 65 insertions(+), 16 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/FileImporter
refs/changes/70/364470/1
diff --git a/src/Data/ImportPlan.php b/src/Data/ImportPlan.php
index 30b5de4..a0690d7 100644
--- a/src/Data/ImportPlan.php
+++ b/src/Data/ImportPlan.php
@@ -2,6 +2,8 @@
namespace FileImporter\Data;
+use MalformedTitleException;
+use MediaWiki\MediaWikiServices;
use RuntimeException;
use Title;
@@ -53,20 +55,28 @@
return $this->details;
}
+ public function getTitleText() {
+ if ( $this->title === null ) {
+ $intendedFileName = $this->request->getIntendedName();
+ if ( $intendedFileName ) {
+ return $intendedFileName . '.' .
$this->details->getSourceFileExtension();
+ } else {
+ return
$this->details->getSourceLinkTarget()->getText();
+ }
+ }
+ return $this->title->getText();
+ }
+
/**
+ * @throws MalformedTitleException if title parsing failed
+ * @throws RuntimeException if Title::newFromLinkTarget returned null
when given a TitleValue
* @return Title
*/
public function getTitle() {
if ( $this->title === null ) {
- $intendedFileName = $this->request->getIntendedName();
-
- if ( $intendedFileName ) {
- $fileExtension =
$this->details->getSourceFileExtension();
- $intendedTitleText = $intendedFileName . '.' .
$fileExtension;
- $this->title = Title::newFromText(
$intendedTitleText, NS_FILE );
- } else {
- $this->title = Title::newFromLinkTarget(
$this->details->getSourceLinkTarget() );
- }
+ $titleParser =
MediaWikiServices::getInstance()->getTitleParser();
+ $titleValue = $titleParser->parseTitle(
$this->getTitleText(), NS_FILE );
+ $this->title = Title::newFromLinkTarget( $titleValue );
if ( $this->title === null ) {
throw new RuntimeException( __METHOD__ . '
failed to get a Title object.' );
@@ -80,14 +90,14 @@
* @return string
*/
public function getFileName() {
- return pathinfo( $this->getTitle()->getText() )['filename'];
+ return pathinfo( $this->getTitleText() )['filename'];
}
/**
* @return string
*/
public function getFileExtension() {
- return pathinfo( $this->getTitle()->getText() )['extension'];
+ return pathinfo( $this->getTitleText() )['extension'];
}
}
diff --git a/src/Services/ImportPlanValidator.php
b/src/Services/ImportPlanValidator.php
index 4581713..1ece81c 100644
--- a/src/Services/ImportPlanValidator.php
+++ b/src/Services/ImportPlanValidator.php
@@ -8,6 +8,7 @@
use FileImporter\Exceptions\TitleException;
use FileImporter\Interfaces\ImportTitleChecker;
use FileImporter\Services\UploadBase\UploadBaseFactory;
+use MalformedTitleException;
use UploadBase;
class ImportPlanValidator {
@@ -51,6 +52,7 @@
* @throws RecoverableTitleException When there is a problem with the
title that can be fixed.
*/
public function validate( ImportPlan $importPlan ) {
+ $this->runBasicTitleCheck( $importPlan );
// Checks the extension doesn't provide easy ways to fix
$this->runFileExtensionCheck( $importPlan );
$this->runDuplicateFilesCheck( $importPlan );
@@ -60,6 +62,17 @@
$this->runRemoteTitleConflictCheck( $importPlan );
}
+ private function runBasicTitleCheck( ImportPlan $importPlan ) {
+ try {
+ $importPlan->getTitle();
+ } catch ( MalformedTitleException $e ) {
+ throw new RecoverableTitleException(
+ $e->getMessageObject(),
+ $importPlan
+ );
+ }
+ }
+
private function runFileTitleCheck( ImportPlan $importPlan ) {
$plannedTitleText = $importPlan->getTitle()->getText();
if ( $plannedTitleText != wfStripIllegalFilenameChars(
$plannedTitleText ) ) {
diff --git a/tests/phpunit/Services/ImportPlanValidatorTest.php
b/tests/phpunit/Services/ImportPlanValidatorTest.php
index c5f5a6e..c370f41 100644
--- a/tests/phpunit/Services/ImportPlanValidatorTest.php
+++ b/tests/phpunit/Services/ImportPlanValidatorTest.php
@@ -17,6 +17,7 @@
use FileImporter\Services\ImportPlanValidator;
use FileImporter\Services\UploadBase\UploadBaseFactory;
use FileImporter\Services\UploadBase\ValidatingUploadBase;
+use MalformedTitleException;
use PHPUnit_Framework_MockObject_MockObject;
use PHPUnit_Framework_TestCase;
use Title;
@@ -97,11 +98,22 @@
return $mock;
}
- private function getMockImportPlan( Title $planTitle = null, Title
$sourceTitle = null ) {
+ private function getMockImportPlan(
+ Title $planTitle = null,
+ Title $sourceTitle = null,
+ $getTitleFails = false
+ ) {
$mock = $this->getMock( ImportPlan::class, [], [], '', false );
- $mock->expects( $this->any() )
- ->method( 'getTitle' )
- ->willReturn( $planTitle );
+ if ( !$getTitleFails ) {
+ $mock->expects( $this->any() )
+ ->method( 'getTitle' )
+ ->willReturn( $planTitle );
+ } else {
+ $mock->expects( $this->any() )
+ ->method( 'getTitle' )
+ ->willThrowException( new
MalformedTitleException( 'mockexception' ) );
+ }
+
$mock->expects( $this->any() )
->method( 'getDetails' )
->willReturn(
@@ -228,7 +240,7 @@
$this->getMockDuplicateFileRevisionChecker( 1,
0 ),
$this->getMockImportTitleChecker( 0, true )
],
- 'Invalid, Bad title (too long)' => [
+ 'Invalid, Bad filename (too long)' => [
new RecoverableTitleException(
'fileimporter-filenameerror-toolong',
$emptyPlan
@@ -241,6 +253,20 @@
$this->getMockImportTitleChecker( 0, true ),
$this->getMockValidatingUploadBase(
UploadBase::FILENAME_TOO_LONG, true )
],
+ 'Invalid, Bad characters "<", getTitle throws
MalformedTitleException' => [
+ new RecoverableTitleException(
+ 'mockexception',
+ $emptyPlan
+ ),
+ $this->getMockImportPlan(
+ $this->getMockTitle( 'Name<Name.JPG',
false ),
+ $this->getMockTitle( 'SourceName.JPG',
false ),
+ true
+ ),
+ $this->getMockDuplicateFileRevisionChecker( 0,
0 ),
+ $this->getMockImportTitleChecker( 0, true ),
+ $this->getMockValidatingUploadBase( true, true )
+ ],
];
return $validTests + $invalidTests;
--
To view, visit https://gerrit.wikimedia.org/r/364470
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I69a1329852d187b782f85ef74cc613c1dfd727fc
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/FileImporter
Gerrit-Branch: master
Gerrit-Owner: Addshore <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
