r71943 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r71942‎ \| r71943 \| r71944 >
Date:	12:18, 30 August 2010
Author:	daniel
Status:	ok (Comments)
Tags:
Comment:	use new UploadVerifyFile hook, introduced in r71942
Modified paths:	/trunk/extensions/PagedTiffHandler/PagedTiffHandler.i18n.php (modified) (history) /trunk/extensions/PagedTiffHandler/PagedTiffHandler.php (modified) (history) /trunk/extensions/PagedTiffHandler/PagedTiffHandler_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/PagedTiffHandler/PagedTiffHandler_body.php
—	—	@@ -59,13 +59,13 @@
60	60	* - identify-warnings
61	61	* - check for running-identify-service
62	62	*/
63		~~- static function check( $saveName, $tempName, &$error ) {~~
	63	+ static function verifyFile( $upload, $mime, &$error ) {
64	64	global $wgTiffMaxEmbedFiles, $wgTiffMaxMetaSize, $wgMaxUploadSize,
65	65	$wgTiffRejectOnError, $wgTiffRejectOnWarning, $wgTiffUseTiffReader,
66	66	$wgTiffReaderPath, $wgTiffReaderCheckEofForJS;
67	67
68		~~- # XXX: it would be much nicer if the hook would get the mime type as a parameter~~
69		~~- $mime = MimeMagic::singleton()->guessMimeType( $tempName, false );~~
	68	+ $tempName = $upload->getTempPath();
	69	+ $saveName = $upload->getLocalFile()->getName();
70	70
71	71	if ( $mime != "image/tiff" ) {
72	72	# not a tiff file, do not check
—	—	@@ -78,58 +78,70 @@
79	79	$tr = new TiffReader( $tempName );
80	80	$tr->check();
81	81	if ( !$tr->isValidTiff() ) {
82		~~- $error = 'tiff_bad_file';~~
83		~~- wfDebug( __METHOD__ . ": $error ($saveName)\n" );~~
	82	+ $error = array( 'tiff_bad_file' );
	83	+ wfDebug( __METHOD__ . ": {$error[0]} ($saveName)\n" );
84	84	return false;
85	85	}
86	86	if ( $tr->checkScriptAtEnd( $wgTiffReaderCheckEofForJS ) ) {
87		~~- $error = 'tiff_script_detected';~~
88		~~- wfDebug( __METHOD__ . ": $error ($saveName)\n" );~~
	87	+ $error = array( 'tiff_script_detected' );
	88	+ wfDebug( __METHOD__ . ": {$error[0]} ($saveName)\n" );
89	89	return false;
90	90	}
91	91	if ( !$tr->checkSize() ) {
92		~~- $error = 'tiff_size_error';~~
93		~~- wfDebug( __METHOD__ . ": $error ($saveName)\n" );~~
	92	+ $error = array( 'tiff_size_error' );
	93	+ wfDebug( __METHOD__ . ": {$error[0]} ($saveName)\n" );
94	94	return false;
95	95	}
96	96	}
97	97	$meta = self::getTiffImage( false, $tempName )->retrieveMetaData();
98	98	if ( !$meta && $meta != - 1 ) {
99		~~- $error = 'tiff_out_of_service';~~
100		~~- wfDebug( __METHOD__ . ": $error ($saveName)\n" );~~
	99	+ $error = array( 'tiff_out_of_service' );
	100	+ wfDebug( __METHOD__ . ": {$error[0]} ($saveName)\n" );
101	101	return false;
102	102	}
103	103	if ( $meta == - 1 ) {
104		~~- $error = 'tiff_error_cached';~~
105		~~- wfDebug( __METHOD__ . ": $error ($saveName)\n" );~~
	104	+ $error = array( 'tiff_error_cached' );
	105	+ wfDebug( __METHOD__ . ": {$error[0]} ($saveName)\n" );
106	106	}
107		~~- return self::extCheck( $meta, $error, $saveName );~~
	107	+
	108	+ $ok = self::verifyMetaData( $meta, $error, $saveName );
	109	+
	110	+ if ( !$ok ) {
	111	+ wfDebug( __METHOD__ . ": file is ok ($saveName)\n" );
	112	+ }
	113	+
	114	+ return $ok;
108	115	}
109	116
110		~~- static function extCheck( $meta, &$error, $saveName = '' ) {~~
	117	+ static function verifyMetaData( $meta, &$error, $saveName = '' ) {
111	118	global $wgTiffMaxEmbedFiles, $wgTiffMaxMetaSize;
112	119
113	120	$errors = PagedTiffHandler::getMetadataErrors( $meta );
114	121	if ( $errors ) {
115		~~- $error = 'tiff_bad_file';~~
	122	+ $error = array( 'tiff_bad_file', PagedTiffHandler::joinMessages( $errors ) );
116	123
117		~~- // NOTE: in future, it will become possible to pass parameters~~
118		~~- // $error = array( 'tiff_bad_file' , PagedTiffHandler::joinMessages( $errors ) );~~
119		~~- // does that work now? ^DK~~
120		-
121		~~- wfDebug( __METHOD__ . ": $error ($saveName) " . PagedTiffHandler::joinMessages( $errors, false ) . "\n" );~~
	124	+ wfDebug( __METHOD__ . ": {$error[0]} ($saveName) " . PagedTiffHandler::joinMessages( $errors, false ) . "\n" );
122	125	return false;
123	126	}
124		~~- if ( ( strlen( serialize( $meta ) ) + 1 ) > $wgTiffMaxMetaSize ) {~~
125		~~- $error = 'tiff_too_much_meta';~~
126		~~- wfDebug( __METHOD__ . ": $error ($saveName)\n" );~~
	127	+
	128	+ if ( $meta['page_amount'] <= 0 \|\| empty( $meta['page_data'] ) ) {
	129	+ $error = array( 'tiff_page_error', $meta['page_amount'] );
	130	+ wfDebug( __METHOD__ . ": {$error[0]} ($saveName)\n" );
127	131	return false;
128	132	}
129	133	if ( $wgTiffMaxEmbedFiles && $meta['page_amount'] > $wgTiffMaxEmbedFiles ) {
130		~~- $error = 'tiff_too_much_embed_files';~~
131		~~- wfDebug( __METHOD__ . ": $error ($saveName)\n" );~~
	134	+ $error = array( 'tiff_too_much_embed_files', $meta['page_amount'], $wgTiffMaxEmbedFiles );
	135	+ wfDebug( __METHOD__ . ": {$error[0]} ($saveName)\n" );
132	136	return false;
133	137	}
	138	+ $len = strlen( serialize( $meta ) );
	139	+ if ( ( $len + 1 ) > $wgTiffMaxMetaSize ) {
	140	+ $error = array( 'tiff_too_much_meta', $len, $wgTiffMaxMetaSize );
	141	+ wfDebug( __METHOD__ . ": {$error[0]} ($saveName)\n" );
	142	+ return false;
	143	+ }
	144	+
	145	+ wfDebug( __METHOD__ . ": metadata is ok ($saveName)\n" );
134	146	return true;
135	147	}
136	148
—	—	@@ -327,7 +339,7 @@
328	340	return new ThumbnailImage( $image, $dstUrl, $width, $height, $dstPath, $page );
329	341	}
330	342
331		~~- if ( !self::extCheck( $meta, $error, $dstPath ) ) {~~
	343	+ if ( !self::verifyMetaData( $meta, $error, $dstPath ) ) {
332	344	return $this->doThumbError( $params, $error );
333	345	}
334	346
—	—	@@ -502,7 +514,9 @@
503	515	* If it returns false, Image will reload the metadata from the file and update the database
504	516	*/
505	517	function isMetadataValid( $image, $metadata ) {
506		~~- if ( is_string( $metadata ) ) $metadata = unserialize( $metadata );~~
	518	+ if ( is_string( $metadata ) ) {
	519	+ $metadata = unserialize( $metadata );
	520	+ }
507	521
508	522	if ( !isset( $metadata['TIFF_METADATA_VERSION'] ) ) {
509	523	return false;
Index: trunk/extensions/PagedTiffHandler/PagedTiffHandler.i18n.php
—	—	@@ -27,7 +27,7 @@
28	28	'tiff_error_cached' => 'This file can only be rerendered after the caching interval.',
29	29	'tiff_size_error' => 'The reported file size does not match the actual file size.',
30	30	'tiff_script_detected' => 'The uploaded file contains scripts.',
31		~~- 'tiff_bad_file' => 'The uploaded file contains errors.',~~
	31	+ 'tiff_bad_file' => 'The uploaded file contains errors: $1',
32	32	'tiff-file-info-size' => '(page $5, $1 × $2 pixel, file size: $3, MIME type: $4)',
33	33	);
34	34
—	—	@@ -47,7 +47,7 @@
48	48	'tiff_error_cached' => 'Error message shown when a error occurres and it is cached.',
49	49	'tiff_size_error' => 'Error message shown when the reported file size does not match the actual file size.',
50	50	'tiff_script_detected' => 'Error message shown when the uploaded file contains scripts.',
51		~~- 'tiff_bad_file' => 'Error message shown when the uploaded file contains errors.',~~
	51	+ 'tiff_bad_file' => 'Error message shown when the uploaded file contains errors. First parameter contains error messages',
52	52	'tiff-file-info-size' => 'Information about the image dimensions etc. on image page. Extended by page information',
53	53	);
54	54
—	—	@@ -67,7 +67,7 @@
68	68	'tiff_error_cached' => 'Error message shown when a error occurres and it is cached.',
69	69	'tiff_size_error' => 'Error message shown when the reported file size does not match the actual file size.',
70	70	'tiff_script_detected' => 'Error message shown when the uploaded file contains scripts.',
71		~~- 'tiff_bad_file' => 'Error message shown when the uploaded file contains errors.',~~
	71	+ 'tiff_bad_file' => 'Error message shown when the uploaded file contains errors. First parameter contains error messages.',
72	72	'tiff-file-info-size' => 'Information about the image dimensions etc. on image page. Extended by page information',
73	73	);
74	74
—	—	@@ -151,7 +151,7 @@
152	152	'tiff_error_cached' => 'Diese Datei kann erst nach Ablauf der Caching-Periode neu gerendert werden.',
153	153	'tiff_size_error' => 'Die errechnete Größe der Datei stimmt nicht mit der tatsächlichen überein.',
154	154	'tiff_script_detected' => 'Die hochgeladene Datei enthält Skripte.',
155		~~- 'tiff_bad_file' => 'Die hochgeladene Datei ist fehlerhaft.',~~
	155	+ 'tiff_bad_file' => 'Die hochgeladene Datei ist fehlerhaft: $1',
156	156	'tiff-file-info-size' => '(Seite $5, $1 × $2 Pixel, Dateigröße: $3, MIME-Typ: $4)',
157	157	);
158	158
Index: trunk/extensions/PagedTiffHandler/PagedTiffHandler.php
—	—	@@ -121,7 +121,7 @@
122	122	$wgAutoloadClasses['PagedTiffHandlerSeleniumTestSuite'] = $dir . 'selenium/PagedTiffHandlerTestSuite.php';
123	123
124	124	$wgMediaHandlers['image/tiff'] = 'PagedTiffHandler';
125		~~-$wgHooks['UploadVerification'][] = 'PagedTiffHandler::check';~~
	125	+$wgHooks['UploadVerifyFile'][] = 'PagedTiffHandler::verifyFile';
126	126	$wgHooks['LanguageGetMagic'][] = 'PagedTiffHandler::addTiffLossyMagicWordLang';
127	127
128	128	define('TIFF_METADATA_VERSION', '1.0');

Follow-up revisions

Revision	Commit summary	Author	Date
r71977	removed some dead wood, as per comment on r71943	daniel	20:53, 30 August 2010

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r71942	interoduced new hoop in Uploadbase::verifyFile, as per comments to r71789	daniel	12:10, 30 August 2010

Comments

#Comment by Duesentrieb (talk | contribs) 12:19, 30 August 2010

requires r71942

#Comment by TheDJ (talk | contribs) 14:53, 30 August 2010

This is not new, but I notice that there is no return after meta==-1. This will lead to meta and error being passed to verifyMetaData() (and potentially overwriting error[0] ?) is this the desired behavior, or is it an oversight ?

Status & tagging log

02:59, 8 February 2011 😂 (talk | contribs) changed the tags for r71943 [removed: 1.16wmf4]
02:48, 13 September 2010 Tim Starling (talk | contribs) changed the status of r71943 [removed: new added: ok]
12:22, 30 August 2010 Duesentrieb (talk | contribs) changed the tags for r71943 [added: 1.16wmf4]