r101644 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r101643‎ | r101644 | r101645 >
Date:18:03, 2 November 2011
Author:demon
Status:resolved (Comments)
Tags:
Comment:
(bug 31822) Error during upgrade due to output buffer reset in stdout.

In various places in Maintenance, we're overwriting our output buffer rather than appending to it. This doesn't matter too much in the CLI, but it can corrupt headers when upgrading from the web. Thanks Woozle for pointing this out.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.18 (modified) (history)
  • /trunk/phase3/maintenance/Maintenance.php (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/Maintenance.php
@@ -310,7 +310,7 @@
311311 if ( $channel === null ) {
312312 $this->cleanupChanneled();
313313
314 - $f = fopen( 'php://stdout', 'w' );
 314+ $f = fopen( 'php://stdout', 'a' );
315315 fwrite( $f, $out );
316316 fclose( $f );
317317 }
@@ -331,7 +331,7 @@
332332 if ( php_sapi_name() == 'cli' ) {
333333 fwrite( STDERR, $err . "\n" );
334334 } else {
335 - $f = fopen( 'php://stderr', 'w' );
 335+ $f = fopen( 'php://stderr', 'a' );
336336 fwrite( $f, $err . "\n" );
337337 fclose( $f );
338338 }
@@ -370,7 +370,7 @@
371371 return;
372372 }
373373
374 - $handle = fopen( 'php://stdout', 'w' );
 374+ $handle = fopen( 'php://stdout', 'a' );
375375
376376 // End the current line if necessary
377377 if ( !$this->atLineStart && $channel !== $this->lastChannel ) {
Index: trunk/phase3/RELEASE-NOTES-1.18
@@ -478,6 +478,7 @@
479479 already set above 1024 bytes
480480 * (bug 32086) Special:PermanentLink now show an error message when no subpage
481481 was specified.
 482+* (bug 31822) Error during upgrade due to output buffer reset in stdout
482483
483484 === API changes in 1.18 ===
484485 * BREAKING CHANGE: action=watch now requires POST and token.

Follow-up revisions

RevisionCommit summaryAuthorDate
r103179Address fixme on r101644 (bug 32325, bug 32263), originally for bug 31822. PH...demon15:04, 15 November 2011
r103196MFT r101644, 103179. Release notes seem to be already mergeddemon17:26, 15 November 2011

Comments

#Comment by Hashar (talk | contribs)   15:11, 10 November 2011

That one is buggy before PHP 5.2.7!

PHP BUG #45303 : Opening php:// wrapper in append mode results in a warning https://bugs.php.net/bug.php?id=45303

Our bugs are bug 32325 and bug 32263

#Comment by 😂 (talk | contribs)   15:26, 10 November 2011

And if you switch from 'a' back to 'w' you'll reintroduce the bug I was fixing. Maybe people should stop using old 5.2 releases.

#Comment by Hashar (talk | contribs)   15:43, 10 November 2011

I have asked in wikitech-l to raise the requirement to 5.2.7 : http://permalink.gmane.org/gmane.science.linguistics.wikipedia.technical/56572

#Comment by G.Hagedorn (talk | contribs)   20:35, 10 November 2011

I think raising to 5.2.7 is a bit steep, given that this is not supported by oldstable debian or older longterm ubuntu distros... I our case, Debian 5 is for the moment the last version we can run before upgrading the entire XEN virtualization environment, later Debian require a newer Kernel, not supported in older XEN.

#Comment by Tim Starling (talk | contribs)   22:26, 10 November 2011

What's wrong with print?

#Comment by 😂 (talk | contribs)   22:29, 10 November 2011

The main difference was so I could differ stderr vs. stdout. But I suppose as a practical concern it doesn't really matter.

#Comment by Tim Starling (talk | contribs)   22:40, 10 November 2011

You could switch based on php_sapi_name(). Even if you could write to stderr in the web mode, you wouldn't want to.

Status & tagging log