r90036 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r90035‎ | r90036 | r90037 >
Date:06:10, 14 June 2011
Author:tstarling
Status:ok (Comments)
Tags:
Comment:
Reverted r89927 and applied a more appropriate fix for the reported hash mismatches.
Modified paths:
  • /branches/wmf/1.17wmf1/includes/HistoryBlob.php (modified) (history)

Diff [purge]

Index: branches/wmf/1.17wmf1/includes/HistoryBlob.php
——@@ -453,13 +453,17 @@
454454 $header = unpack( 'Vofp/Vcsize', substr( $diff, 0, 8 ) );
455455
456456 # Check the checksum if mhash is available
457 - if ( function_exists( 'mhash' ) ) {
 457+ # TEMPORARY WMF PATCH -- temporarily disable hash check, since a large number of
 458+ # revisions appear to have hash mismatches
 459+ /*
 460+ if ( extension_loaded( 'mhash' ) ) {
458461 $ofp = mhash( MHASH_ADLER32, $base );
459462 if ( $ofp !== substr( $diff, 0, 4 ) ) {
460463 wfDebug( __METHOD__. ": incorrect base checksum\n" );
461464 return false;
462465 }
463 - }
 466+ }*/
 467+ #### END WMF PATCH
464468 if ( $header['csize'] != strlen( $base ) ) {
465469 wfDebug( __METHOD__. ": incorrect base length\n" );
466470 return false;

Follow-up revisions

RevisionCommit summaryAuthorDate
r982201.18wmf1: Merge r90036 from 1.17wmf1, requested by Arielcatrope14:13, 27 September 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r89927mft r89925 so we can disable mhash in php.ini instead of at build timeariel12:29, 12 June 2011

Comments

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

I don't understand why this is better; it seems much more intrusive to me.

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

It's better because it has documentation, it's clear about what it's doing and why, and it doesn't subtly break unrelated code. By "less intrusive" I suppose you mean your way changes less bytes of code, but that's not a goal.

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

No, what I meant was: function_exists() is narrower as a check, and it also will not need to be removed later. The place for information about the hash issue is bugzilla; I will be opening a report with a few textids there so the issue can be looked at separately.

Status & tagging log