Fixed Output buffering clearing causes notice error

[AlexT]

Got this notice error in my log files (XF 1.2 beta 3):

Application.php(186)    ob_end_clean(): failed to delete buffer. No buffer to delete    E_NOTICE

// see http://bugs.php.net/bug.php?id=36514
if (!@ini_get('output_handler')) while (@ob_end_clean());

It is caused by calling ob_end_clean() even if there is not a buffer.

Not sure if this workaround fix is still needed for older PHP versions, but if it is, perhaps it could be enhanced with this line of code:

if (!@ini_get('output_handler')) while (ob_get_level() > 0) { ob_end_clean(); };

This will clean output buffering if and only if it's already started.

 

[AlexT]

Forgot to mention, this test server runs:

PHP 5.3.26 with output_buffering set to Off. Gzip is enabled by the web server (Nginx).

 

[Mike]

That's generally the reason we use the shut up operator there. I'm not sure why that should be logged...

 

[AlexT]

That's generally the reason we use the shut up operator there. I'm not sure why that should be logged...

Mike, I am using a debugger that shows all errors even with the dog-slow @ prefix . With the suggested fix I believe it should not be necessary to suppress the error anymore and still achieve what you wanted to.

 

[AlexT]

I found this remark in the Mediawiki SVN:

// There's another bug in ob_gzhandler (see also the comment at
// the top of this function) that causes it to gzip even empty
// responses, meaning it's impossible to produce a truly empty
// response (because the gzip header is always there). This is
// a problem because 304 responses have to be completely empty
// per the HTTP spec, and Firefox behaves buggily when they're not.
// See also http://bugs.php.net/bug.php?id=51579
// To work around this, we tear down all output buffering before
// sending the 304.
// On some setups, ob_get_level() doesn't seem to go down to zero
// no matter how often we call ob_get_clean(), so instead of doing
// the more intuitive while ( ob_get_level() > 0 ) ob_get_clean();
// we have to be safe here and avoid an infinite loop.
// Caching the level is not an option, need to allow it to
// shorten the loop on-the-fly (bug 46836)

So perhaps above suggested code should be changed to:

if (!@ini_get('output_handler'))
{
    for ($i = 0; $i < ob_get_level(); $i++)
    {
      ob_end_clean();
    }
}

 

[Kier]

I've applied the fix you noted in your last post there, but we're still testing...

 

[Mike]

Yeah, I think that code is bugged. If it's deep within an output buffer, I don't think it cleans it properly. I'm looking at an alternative approach that doesn't use the shut up operator.

 

[Mike]

Trying:

            $level = ob_get_level();
            while ($level)
            {
                ob_end_clean();
                $newLevel = ob_get_level();
                if ($newLevel >= $level)
                {
                    break;
                }
                $level = $newLevel;
            }

As that should handle any infinite loop possibility (by the get level not changing correctly).

 

[Sim]

@Mike and @Kier - is this fix still required now that the minimum version of PHP is 5.6 in XF 2.1 ?

I ask because arbitrarily turning off output buffering is breaking other systems which instantiate the XF framework while using their own output buffering.

In my specific use-case, I am instantiating the XF framework from within PHPUnit, and by arbitrarily closing all output buffers in the \XF::standardizeEnvironment() function - it is causing a heap of warnings from PHPUnit about "risky" code: Test code or tested code did not (only) close its own output buffers

Are you still able to reproduce the problematic behaviour which required that code when using PHP 5.6? I can't find any more recent references to the bugs discussed here and in the code comments - and there are suggestions that more recent versions of PHP have fixed it.

If for some reason the code is still required, is there perhaps a way we can be selective about when to run this code? Maybe add an extra parameter to \XF::start() which is passed on to the standardizeEnvironment() function?

For example:

    public static function start($rootDirectory, $cleanOutputBuffers = true)
    {
        ...

        self::standardizeEnvironment($cleanOutputBuffers);
    }

    public static function standardizeEnvironment($cleanOutputBuffers = true)
    {
        ...

        // see http://bugs.php.net/bug.php?id=36514
        // and http://xenforo.com/community/threads/53637/
        if (!@ini_get('output_handler') && $cleanOutputBuffers)
        {
            $level = ob_get_level();
            while ($level)
            {
                @ob_end_clean();
                $newLevel = ob_get_level();
                if ($newLevel >= $level)
                {
                    break;
                }
                $level = $newLevel;
            }
        }
    }

... that way I can simply call \XF::start($rootDir, false); to disable output buffer cleaning while not affecting the default behaviour in any way.

I've confirmed that removing the output cleaning code from standardizeEnvironment fixes the warnings I'm getting in PHPUnit.