r42184 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r42183‎ | r42184 | r42185 >
Date:23:53, 17 October 2008
Author:dale
Status:old (Comments)
Tags:
Comment:
fixed issues of r41969 revert. Removed XSS, ( less variable overloading ), fall-back ffmpeg command for failed multi-track ogg thumbnail generation
Modified paths:
  • /trunk/extensions/OggHandler/OggHandler.php (modified) (history)
  • /trunk/extensions/OggHandler/OggHandler_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/OggHandler/OggHandler.php
@@ -34,6 +34,15 @@
3535
3636 /******************* CONFIGURATION STARTS HERE **********************/
3737
 38+//if wgPlayerStats collection is enabled or not
 39+$wgPlayerStatsCollection=false;
 40+
 41+//the player stats js file (does not have to be the same server as OggHandler is installed on)
 42+$wgPlayerStatsCollectionJs = $wgScriptPath . '/extensions/PlayerStatsGrabber/playerStats.js';
 43+
 44+//url to the wiki index.php your submitting stats to (leave empty for the same server as current)
 45+$wgPlayerStatsCollectionScriptPath = '';
 46+
3847 // Location of the FFmpeg binary
3948 $wgFFmpegLocation = 'ffmpeg';
4049
Index: trunk/extensions/OggHandler/OggHandler_body.php
@@ -238,23 +238,36 @@
239239 # No audio, one frame
240240 ' -f mjpeg -an -vframes 1 ' .
241241 wfEscapeShellArg( $dstPath ) . ' 2>&1';
242 -
 242+
243243 $retval = 0;
244244 $returnText = wfShellExec( $cmd, $retval );
245245
246246 if ( $this->removeBadFile( $dstPath, $retval ) || $retval ) {
247 - // Filter nonsense
248 - $lines = explode( "\n", str_replace( "\r\n", "\n", $returnText ) );
249 - if ( substr( $lines[0], 0, 6 ) == 'FFmpeg' ) {
250 - for ( $i = 1; $i < count( $lines ); $i++ ) {
251 - if ( substr( $lines[$i], 0, 2 ) != ' ' ) {
252 - break;
 247+ #re-attempt encode command on frame time 1 and with mapping (special case for chopped oggs)
 248+ $cmd = wfEscapeShellArg( $wgFFmpegLocation ) .
 249+ ' -map 0:1 '.
 250+ ' -ss 1 ' .
 251+ ' -i ' . wfEscapeShellArg( $file->getPath() ) .
 252+ ' -f mjpeg -an -vframes 1 ' .
 253+ wfEscapeShellArg( $dstPath ) . ' 2>&1';
 254+
 255+ $retval = 0;
 256+ $returnText = wfShellExec( $cmd, $retval );
 257+ //if still bad return error:
 258+ if ( $this->removeBadFile( $dstPath, $retval ) || $retval ) {
 259+ // Filter nonsense
 260+ $lines = explode( "\n", str_replace( "\r\n", "\n", $returnText ) );
 261+ if ( substr( $lines[0], 0, 6 ) == 'FFmpeg' ) {
 262+ for ( $i = 1; $i < count( $lines ); $i++ ) {
 263+ if ( substr( $lines[$i], 0, 2 ) != ' ' ) {
 264+ break;
 265+ }
253266 }
 267+ $lines = array_slice( $lines, $i );
254268 }
255 - $lines = array_slice( $lines, $i );
 269+ // Return error box
 270+ return new MediaTransformError( 'thumbnail_error', $width, $height, implode( "\n", $lines ) );
256271 }
257 - // Return error box
258 - return new MediaTransformError( 'thumbnail_error', $width, $height, implode( "\n", $lines ) );
259272 }
260273 return new OggVideoDisplay( $file, $file->getURL(), $dstUrl, $width, $height, $length, $dstPath );
261274 }
@@ -390,7 +403,8 @@
391404 }
392405
393406 function setHeaders( $out ) {
394 - global $wgOggScriptVersion, $wgCortadoJarFile, $wgServer;
 407+ global $wgOggScriptVersion, $wgCortadoJarFile, $wgServer, $wgUser,
 408+ $wgPlayerStatsCollection, $wgPlayerStatsCollectionJs, $wgPlayerStatsCollectionScriptPath;
395409 if ( $out->hasHeadItem( 'OggHandler' ) ) {
396410 return;
397411 }
@@ -430,7 +444,27 @@
431445 }
432446 </style>
433447 EOT
434 - );
 448+);
 449+ //if collecting stats add relevant code:
 450+ if( $wgPlayerStatsCollection ){
 451+ $jsUserHash = sha1( $wgUser->getName() . $wgProxyKey );
 452+ $enUserHash = Xml::encodeJsVar( $jsUserHash );
 453+ //escape the javascript url:
 454+ $wgPlayerStatsCollectionJs = htmlspecialchars( $wgPlayerStatsCollectionJs );
 455+
 456+ if( trim($wgPlayerStatsCollectionScriptPath ) != '' )
 457+ $wgPlayerStatsCollectionScriptPath= htmlentities( $wgPlayerStatsCollectionScriptPath );
 458+
 459+ $wgPlayerStatsCollectionScriptPath = htmlspecialchars( $wgPlayerStatsCollectionScriptPath );
 460+ $out->addHeadItem('playerStatsCollection', <<<EOT
 461+<script type="text/javascript">
 462+wgOggPlayer.userHash = $enUserHash;
 463+wgServerOveride = "$wgPlayerStatsCollectionScriptPath";
 464+</script>
 465+<script type="text/javascript" src="$wgPlayerStatsCollectionJs"></script>
 466+EOT
 467+);
 468+ }
435469
436470 }
437471

Follow-up revisions

RevisionCommit summaryAuthorDate
r42237Reverting r42184 "fixed issues of r41969 revert. Removed XSS, ( less variable...brion00:11, 20 October 2008

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r41969Revert r41628:...tstarling14:14, 11 October 2008

Comments

#Comment by Brion VIBBER (talk | contribs)   00:12, 20 October 2008

There's some weird altering of a global variable here; could cause data corruption if called multiple times.

Reverting in r42237

Status & tagging log