[MediaWiki-commits] [Gerrit] mediawiki/core[master]: Do not lose message parameters in UploadFromChunks::verifyCh...

2016-12-20 Thread jenkins-bot (Code Review)
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 Mullie 
Gerrit-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...

2016-12-15 Thread Matthias Mullie (Code Review)
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...

2016-12-13 Thread jenkins-bot (Code Review)
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ński 
Gerrit-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...

2016-12-13 Thread Code Review
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