r90035 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r90034‎ | r90035 | r90036 >
Date:06:07, 14 June 2011
Author:tstarling
Status:ok (Comments)
Tags:
Comment:
Revert r89925, setting disable_functions in php.ini is definitely the wrong way to fix hash mismatches.
Modified paths:
  • /trunk/phase3/includes/HistoryBlob.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/HistoryBlob.php
——@@ -516,7 +516,7 @@
517517 $header = unpack( 'Vofp/Vcsize', substr( $diff, 0, 8 ) );
518518
519519 # Check the checksum if mhash is available
520 - if ( function_exists( 'mhash' ) ) {
 520+ if ( extension_loaded( 'mhash' ) ) {
521521 $ofp = mhash( MHASH_ADLER32, $base );
522522 if ( $ofp !== substr( $diff, 0, 4 ) ) {
523523 wfDebug( __METHOD__. ": incorrect base checksum\n" );

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r89925safer check if mhash is available (and I'm about to make it unavailable by di...ariel12:02, 12 June 2011

Comments

#Comment by ArielGlenn (talk | contribs)   06:11, 14 June 2011

The extension_loaded check is broader than it needs to be; the function_exists check is better, regardless of other issues.

#Comment by Tim Starling (talk | contribs)   06:31, 14 June 2011

You mean just in case there is some other extension that provides mhash(), other than the mhash extension? But if that was the situation, how would you know whether the function had the same parameters, and had the same definitions of the hash functions?

#Comment by ArielGlenn (talk | contribs)   06:39, 14 June 2011

No; I mean that it is possible for the extension to be enabled (at build time) but the function to be disabled (at run time). In that case the extension_loaded() test will not do the job. I note that in general we seem to prefer calls to function_exists() instead of extension_loaded(), I assume for this reason.

Status & tagging log