[MediaWiki-commits] [Gerrit] mediawiki/core[master]: Do not lose message parameters in UploadFromChunks::verifyCh...
jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/327500 ) Change subject: Do not lose message parameters in UploadFromChunks::verifyChunk() .. Do not lose message parameters in UploadFromChunks::verifyChunk() This change is similar to If9ce05045ada1e3f55e031639e4c4ebc2a216de8 Having verifyChunk inside doStashFile was annoying. We'd have to catch the exception in UploadBase::tryStashFile in order to convert it to a proper Status object instead of the generic one that is currently built there. I felt like UploadBase::tryStashFile shouldn't have to be aware of this exception, so I moved that catch into a new UploadFromChunks::tryStashFile. It makes no sense to perform that check twice when running tryStashFile, so I got rid of it in doStashFile. But that also meant we had to add it to a few other (now deprecated) places calling doStashFile... But they should be cleaned up at some point anyway. This will make sure we get error output like this: "code":"filetype-bad-ie-mime", "key":"filetype-bad-ie-mime", "params":["text/html"] instead of: "code":"stashfailed", "key":"Cannot upload this file because Internet Explorer would detect it as \"text/html\", which is a disallowed and potentially dangerous file type.", "params":[] Bug: T32095 Change-Id: I2fa767656cb3a5b366210042b8b504dc10ddaf68 --- M includes/upload/UploadFromChunks.php 1 file changed, 46 insertions(+), 1 deletion(-) Approvals: Aaron Schulz: Looks good to me, approved jenkins-bot: Verified diff --git a/includes/upload/UploadFromChunks.php b/includes/upload/UploadFromChunks.php index 449fc05..3dabc0d 100644 --- a/includes/upload/UploadFromChunks.php +++ b/includes/upload/UploadFromChunks.php @@ -64,6 +64,52 @@ } /** +* {@inheritdoc} +*/ + public function tryStashFile( User $user, $isPartial = false ) { + try { + $this->verifyChunk(); + } catch ( UploadChunkVerificationException $e ) { + return Status::newFatal( $e->msg ); + } + + return parent::tryStashFile( $user, $isPartial ); + } + + /** +* {@inheritdoc} +* @throws UploadChunkVerificationException +* @deprecated since 1.28 Use tryStashFile() instead +*/ + public function stashFile( User $user = null ) { + wfDeprecated( __METHOD__, '1.28' ); + $this->verifyChunk(); + return parent::stashFile( $user ); + } + + /** +* {@inheritdoc} +* @throws UploadChunkVerificationException +* @deprecated since 1.28 +*/ + public function stashFileGetKey() { + wfDeprecated( __METHOD__, '1.28' ); + $this->verifyChunk(); + return parent::stashFileGetKey(); + } + + /** +* {@inheritdoc} +* @throws UploadChunkVerificationException +* @deprecated since 1.28 +*/ + public function stashSession() { + wfDeprecated( __METHOD__, '1.28' ); + $this->verifyChunk(); + return parent::stashSession(); + } + + /** * Calls the parent doStashFile and updates the uploadsession table to handle "chunks" * * @param User|null $user @@ -74,7 +120,6 @@ $this->mChunkIndex = 0; $this->mOffset = 0; - $this->verifyChunk(); // Create a local stash target $this->mStashFile = parent::doStashFile( $user ); // Update the initial file offset (based on file size) -- To view, visit https://gerrit.wikimedia.org/r/327500 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I2fa767656cb3a5b366210042b8b504dc10ddaf68 Gerrit-PatchSet: 2 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: Matthias MullieGerrit-Reviewer: Aaron Schulz Gerrit-Reviewer: Gergő Tisza Gerrit-Reviewer: jenkins-bot <> ___ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
[MediaWiki-commits] [Gerrit] mediawiki/core[master]: Do not lose message parameters in UploadFromChunks::verifyCh...
Matthias Mullie has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/327500 ) Change subject: Do not lose message parameters in UploadFromChunks::verifyChunk() .. Do not lose message parameters in UploadFromChunks::verifyChunk() This change is similar to If9ce05045ada1e3f55e031639e4c4ebc2a216de8 Having verifyChunk inside doStashFile was annoying. We'd have to catch the exception in UploadBase::tryStashFile in order to convert it to a proper Status object instead of the generic one that is currently built there. I felt like UploadBase::tryStashFile shouldn't have to be away of this exception, so I moved that catch into a new UploadFromChunks::tryStashFile. It makes no sense to perform that check twice when running tryStashFile, so I got rid of it in doStashFile. But that also meant we had to add it to a few other (now deprecated) places calling doStashFile... But they should be cleaned up at some point anyway. This will make sure we get error output like this: "code":"filetype-bad-ie-mime", "key":"filetype-bad-ie-mime", "params":["text/html"] instead of: "code":"stashfailed", "key":"Cannot upload this file because Internet Explorer would detect it as \"text/html\", which is a disallowed and potentially dangerous file type.", "params":[] Bug: T32095 Change-Id: I2fa767656cb3a5b366210042b8b504dc10ddaf68 --- M includes/upload/UploadFromChunks.php 1 file changed, 46 insertions(+), 1 deletion(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/00/327500/1 diff --git a/includes/upload/UploadFromChunks.php b/includes/upload/UploadFromChunks.php index 449fc05..3dabc0d 100644 --- a/includes/upload/UploadFromChunks.php +++ b/includes/upload/UploadFromChunks.php @@ -64,6 +64,52 @@ } /** +* {@inheritdoc} +*/ + public function tryStashFile( User $user, $isPartial = false ) { + try { + $this->verifyChunk(); + } catch ( UploadChunkVerificationException $e ) { + return Status::newFatal( $e->msg ); + } + + return parent::tryStashFile( $user, $isPartial ); + } + + /** +* {@inheritdoc} +* @throws UploadChunkVerificationException +* @deprecated since 1.28 Use tryStashFile() instead +*/ + public function stashFile( User $user = null ) { + wfDeprecated( __METHOD__, '1.28' ); + $this->verifyChunk(); + return parent::stashFile( $user ); + } + + /** +* {@inheritdoc} +* @throws UploadChunkVerificationException +* @deprecated since 1.28 +*/ + public function stashFileGetKey() { + wfDeprecated( __METHOD__, '1.28' ); + $this->verifyChunk(); + return parent::stashFileGetKey(); + } + + /** +* {@inheritdoc} +* @throws UploadChunkVerificationException +* @deprecated since 1.28 +*/ + public function stashSession() { + wfDeprecated( __METHOD__, '1.28' ); + $this->verifyChunk(); + return parent::stashSession(); + } + + /** * Calls the parent doStashFile and updates the uploadsession table to handle "chunks" * * @param User|null $user @@ -74,7 +120,6 @@ $this->mChunkIndex = 0; $this->mOffset = 0; - $this->verifyChunk(); // Create a local stash target $this->mStashFile = parent::doStashFile( $user ); // Update the initial file offset (based on file size) -- To view, visit https://gerrit.wikimedia.org/r/327500 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2fa767656cb3a5b366210042b8b504dc10ddaf68 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: Matthias Mullie___ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
[MediaWiki-commits] [Gerrit] mediawiki/core[master]: Do not lose message parameters in UploadFromChunks::verifyCh...
jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/326944 ) Change subject: Do not lose message parameters in UploadFromChunks::verifyChunk() .. Do not lose message parameters in UploadFromChunks::verifyChunk() This code is gross, and my changes do not really make it better, but it works more correctly more often. Bug: T147720 Change-Id: If9ce05045ada1e3f55e031639e4c4ebc2a216de8 --- M includes/upload/UploadFromChunks.php 1 file changed, 8 insertions(+), 2 deletions(-) Approvals: jenkins-bot: Verified Anomie: Looks good to me, approved diff --git a/includes/upload/UploadFromChunks.php b/includes/upload/UploadFromChunks.php index 08cf434..449fc05 100644 --- a/includes/upload/UploadFromChunks.php +++ b/includes/upload/UploadFromChunks.php @@ -222,7 +222,7 @@ $this->verifyChunk(); $this->mTempPath = $oldTemp; } catch ( UploadChunkVerificationException $e ) { - return Status::newFatal( $e->getMessage() ); + return Status::newFatal( $e->msg ); } $status = $this->outputChunk( $chunkPath ); if ( $status->isGood() ) { @@ -364,7 +364,7 @@ $this->mDesiredDestName = $oldDesiredDestName; $this->mTitle = false; if ( is_array( $res ) ) { - throw new UploadChunkVerificationException( $res[0] ); + throw new UploadChunkVerificationException( $res ); } } } @@ -376,4 +376,10 @@ } class UploadChunkVerificationException extends MWException { + public $msg; + public function __construct( $res ) { + $this->msg = call_user_func_array( 'wfMessage', $res ); + parent::__construct( call_user_func_array( 'wfMessage', $res ) + ->inLanguage( 'en' )->useDatabase( false )->text() ); + } } -- To view, visit https://gerrit.wikimedia.org/r/326944 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: If9ce05045ada1e3f55e031639e4c4ebc2a216de8 Gerrit-PatchSet: 2 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: Bartosz DziewońskiGerrit-Reviewer: Anomie Gerrit-Reviewer: Bartosz Dziewoński Gerrit-Reviewer: jenkins-bot <> ___ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
[MediaWiki-commits] [Gerrit] mediawiki/core[master]: Do not lose message parameters in UploadFromChunks::verifyCh...
Bartosz Dziewoński has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/326944 ) Change subject: Do not lose message parameters in UploadFromChunks::verifyChunk() .. Do not lose message parameters in UploadFromChunks::verifyChunk() This code is gross, and my changes do not really make it better, but it works more correctly more often. Bug: T147720 Change-Id: If9ce05045ada1e3f55e031639e4c4ebc2a216de8 --- M includes/upload/UploadFromChunks.php 1 file changed, 7 insertions(+), 2 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/44/326944/1 diff --git a/includes/upload/UploadFromChunks.php b/includes/upload/UploadFromChunks.php index 08cf434..e65dc56 100644 --- a/includes/upload/UploadFromChunks.php +++ b/includes/upload/UploadFromChunks.php @@ -222,7 +222,7 @@ $this->verifyChunk(); $this->mTempPath = $oldTemp; } catch ( UploadChunkVerificationException $e ) { - return Status::newFatal( $e->getMessage() ); + return Status::newFatal( $e->msg ); } $status = $this->outputChunk( $chunkPath ); if ( $status->isGood() ) { @@ -364,7 +364,7 @@ $this->mDesiredDestName = $oldDesiredDestName; $this->mTitle = false; if ( is_array( $res ) ) { - throw new UploadChunkVerificationException( $res[0] ); + throw new UploadChunkVerificationException( $res ); } } } @@ -376,4 +376,9 @@ } class UploadChunkVerificationException extends MWException { + public $msg; + public function __construct( $res ) { + $this->msg = call_user_func_array( 'wfMessage', $res ); + parent::__construct( $this->msg->inLanguage( 'en' )->useDatabase( false )->text() ); + } } -- To view, visit https://gerrit.wikimedia.org/r/326944 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If9ce05045ada1e3f55e031639e4c4ebc2a216de8 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: Bartosz Dziewoński___ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits