So I happened to look in my php error log today. Tons of errors for months (back to Oct 2017) related to Subs.php and a function called dieGif. Note that none of these errors are in the Elk log. Always in the block of four errors below.
PHP Notice: ob_clean(): failed to delete buffer. No buffer to delete in [cut]/sources/Subs.php on line 2069
PHP Warning: Cannot modify header information - headers already sent in [cut]/sources/Subs.php on line 2079
PHP Warning: Cannot modify header information - headers already sent in [cut]/sources/Subs.php on line 2080
PHP Warning: Cannot modify header information - headers already sent in [cut]/sources/Subs.php on line 2083
Using php 7.1.
I assume it's 1.1.3.
Odd... well, let's change the test, instead of:
if (empty($filename))
{
ob_clean();
}
let's use:
if (empty($filename))
{
if (ob_get_level() > 0)
{
ob_clean();
}
}
Yes 1.1.3 although it's been happening before that. I made the changes. Will have to wait for the bots to come by as I'm not sure exactly what action triggers this code.
Ok, that fixes the error line, but the three warning lines about the headers already having been sent are still showing up. Not surprising given they're a completely different part of the function.
Okay, since this is related to the issue https://github.com/elkarte/Elkarte/issues/2391 can you have a look in your log if you have any other "header sent" warning before:
PHP Notice: ob_clean(): failed to delete buffer. No buffer to delete in [cut]/sources/Subs.php on line 2069
this code was put in place in order to try to log where the headers were actually sent.
So since you are finding it from the server error log, it may be worth investigating the origin of the issue rather than fix a workaround put in place to detect an issue! :)
The lines I posted are the only ones that appear in the error log back through Feb other than a typo I made making your first suggested change. The same four lines over and over and over.
hmm... okay, then let's try with the easy one: replace the whole block:
// The following logging is just for debug, it should be removed before final
// or at least once the bug is fixes #2391
$filename = '';
$linenum = '';
if (headers_sent($filename, $linenum))
{
if (empty($filename))
{
ob_clean();
}
else
{
Errors::instance()->log_error('Headers already sent in ' . $filename . ' at line ' . $linenum);
}
}
with just:
if (ob_get_level() > 0)
{
@ob_end_clean();
}
That does nothing. It's the header lines below that are throwing the errors and your change does not alter those.
if ($expired === true)
{
header('Expires: Mon, 26 Jul 1997 05:00:00 GMT');
header('Last-Modified: ' . gmdate('D, d M Y H:i:s') . ' GMT');
}
header('Content-Type: image/gif');
The call to dieGif that's causing most (maybe all, I'm not going to check all the log entries) of the error log entries is coming from ScheduledTasks.controller.php.
I know it's the two lines below, and that was one of the tries to make them stop.
Problem is: I am not able to reproduce it, and I have no idea where the heck another header could have been sent in the path that leads to this function.
But we can at least try to narrow down the paths, that is possible if you can help a moment. :)
Go to: admin > maintenance > scheduled tasks > task log
There you can see which task was run at which time. This should let you match the task that is generating the error (unless all of them are, that is a good information as well).
Comparing that log to the error log it seems that all of them are causing the problem.
Here's the response headers from the scheduled tasks. Certainly not the gif headers set in dieGif. Not sure where these are getting set though.
Response headers (264 B)
Cache-Control: max-age=1
Connection: keep-alive, Keep-Alive
Content-Length: 0
Content-Type: text/html; charset=UTF-8
Date: Thu, 03 May 2018 07:11:05 GMT
Expires: Thu, 03 May 2018 07:11:06 GMT
Keep-Alive: timeout=5, max=99
Server: Apache
Could it be the webserver is sending some header by itself?Would you mind checking index.php and change this line:
$controller = new ScheduledTasks_Controller(new Event_manager());
to:
$controller = new ScheduledTasks_Controller(new Event_Manager());
I think you're looking at the dev branch. The code I have and in master at at GitHub is just this.
$controller = new ScheduledTasks_Controller();
Trying both though the lower case m throws an error about Event_manager not found and the upper case M doesn't change the behavior.
yep, indeed, wrong directory, sorry... :-[
Anyway, this is odd because the event manager is initialized in the construct of the controller (in Action_Controller actually).
hmm... do you have fastcgi enabled?
Yes, php-fpm.
Then, from index.php try removing:
// Don't make people wait on us if we can help it.
if (function_exists('fastcgi_finish_request'))
fastcgi_finish_request();
That fixes it. No errors in the log and the response has gif headers.
So really since you know it's going to end in dieGif don't you want to send the gif right before you call fast_cgi_finish? Then go process the scheduled tasks and simply return w/o any further data where you previously called dieGif?
Honestly I didn't notice it because I don't use fastcgi.
It was added back in 2015.
I think we have two options here:
1) drop the fastcgi call,
2) drop the dying with a gif.
The gif was added back in 2011 with this commit message:
and the bug should be:
http://dev.simplemachines.org/mantis/view.php?id=4673
I feel like chrome was whining because a gif header was sent without the gif.
I'd be tempted to say: let's remove the gif header and send an empty page instead. Problem solved.
Effectively anyone running fastcgi has been sending an empty page. I never notice anything browser wise to tip me off that it was sending an empty page in an img tag. I agree the send a gif seems an odd choice over an empty page.
I think the 1px gif is an heritage of the old days when browsers may complain about an empty responses.
Okay, let's take a look at dropping this stuff once and for all! xD