web/Response::sendHeaders does not warn if headers are already send

[cronfy]

I experienced problems with CSRF similar to #541 #7115 . CSRF cookie just was not sent in response to requests.

It turned out that one of my files echoed some content before View::beginPage() call.

To find it, I had to debug through all the application up to Response::sendHeaders(), where I could find the root cause of the issue dumping $file and $line from headers_sent($file, $line).

The question is: why Yii does not warn/throw an error if headers were already sent prior to Response::send()? This leads to silent and hard-to-debug problems.

I think simple check before $this->sendHeaders() in Response::send(), that will throw an Exception if headers_sent) is true, will solve this issue.

I can make a PR for this if this solutuion is ok.

[insperedia]

Had same issue, just forgot to put $this->beginPage() in layout file.
It could be serous issue for beginners.

[samdark]

@cronfy please do.

[irthunder]

So, now you can't call VarDumper in action. Because of:
echo static::dumpAsString($var, $depth, $highlight); in BaseVarDumper.php

[cronfy]

@irthunder hmm, I usually call die() right after echoing a variable dump, because I do not need to see site contents while debugging.

[irthunder]

@cronfy Sometimes you need, sometimes you don't. But this definitely will broke projects, who using (can use) echo instead of return, for example REST APIs. My opinion - that's not such critical error to throw an exception.

[cronfy]

@irthunder Yes, of course, if I do not need it, it does not mean everybody do not need it too. I just was wondering why I never met issue you mentioned.

I see the problem about REST apps with echo instead of return.

But.

Usually PHP sends a warning when trying to send headers after they were sent:

Warning: Cannot add header information - headers already sent by...

It protects from hard-to-debug bugs when session does not start, cookies not sent etc. But in yii\web\Response() this warning was silently converted into nothing by:

        if (headers_sent()) {
            return;
        }

Without this check PHP Warning would informed you that something goes wrong (actually it would be converted to Exception() by Yii anyway). But it does not happen. Basically Yii was silently disabling one of PHP features. I am convinced exactly this was Yii2 bug, and not a feature.

My opinion - that's not such critical error to throw an exception.

HeadersAlreadySentException can be replaced by trigger_error('Headers already sent', E_USER_WARNING) to mimic PHP default behavior. But as Yii will convert this to an Exception() anyway, so I do not think it makes sense..

Any other ideas to solve this?

[Patricy]

I has my own function which sends some headers (custom csv renderer for specific purposes).
Error was solved with adding "exit" after renderer call
But seems is good idea to do not throw exception. I agree with error logging to log

[Sammaye]

I just ran into this change when using Imagine to render images, because they call the imagepng() etc function directly returning it into Yii2 response actually creates errors now.

One method to solve this is to wrap the output ion ob()

[xiebruce]

@insperedia God, I've been checking this issue for hours, it turn out to be not having a $this->beginPage() in layout file, you save my ass man.

[irthunder]

I'm suggesting to do this:
if (headers_sent($file, $line)) {
if (YII_DEBUG) { throw new HeadersAlreadySentException($file, $line); } else { return; }
}