yiisoft / yii

Yii PHP Framework 1.1.x
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
4.84k stars 2.28k forks source link

after php 7.2-fix for CHttpCacheFilter::sendCacheControlHeader: last-modified-headers are overwritten #4484

Closed jannis701 closed 1 year ago

jannis701 commented 2 years ago

CHttpCacheFilter will not set last-modified-headers. they will be overwritten by sendCacheControlHeader

What steps will reproduce the problem?

use the httpcacheFilter to set lastmodified

    public function filters() {
        return array(
            array(
                'CHttpCacheFilter',
                'lastModified' => '2022-09-12 09:00'
            ),
        );
    }

What is the expected result?

Header:

Last-Modified: Mon, 12 Sep 2022 09:00:00 GMT

What do you get instead?

Header:

Last-Modified: Mon, 12 Jun 2020 08:21:00 GMT

(Date of last change of index.php)

Additional info

i think the Last-Modified-Header must be set AFTER calling $this->sendCacheControlHeader();

Q A
Yii version 1.1.25
PHP version 7.4.3
Operating system debian
marcovtwout commented 2 years ago

@jannis701 Could you point out the offending commit/change that broke this in the first place?

jannis701 commented 2 years ago

this one: https://github.com/yiisoft/yii/commit/e08802ea5c178386fb1749d4d65e0ef7b436981d

marcovtwout commented 2 years ago

@jannis701 This issue seems to be the same thing @rob006 noticed in his code review here: https://github.com/yiisoft/yii/pull/4203/files#r275127437

Could you perhaps add the raw HTTP response here for further clarification? Maybe the last-modified header is sent twice?

jannis701 commented 2 years ago

without my change:

HTTP/1.1 200 OK
Date: Fri, 30 Sep 2022 06:53:31 GMT
Server: Apache/2.4.41
Expires: Fri, 30 Sep 2022 09:53:31 GMT
Cache-Control: max-age=3600, public
Pragma: 
Last-Modified: Thu, 05 Jul 2018 08:51:40 GMT
Set-Cookie: PHPSESSID=xxxxxxxxxxxxxxxxxxxxx; path=/; HttpOnly
Connection: close
Transfer-Encoding: chunked
Content-Type: application/json

after my change:

HTTP/1.1 200 OK
Date: Fri, 30 Sep 2022 06:55:08 GMT
Server: Apache/2.4.41
Expires: Fri, 30 Sep 2022 09:55:08 GMT
Cache-Control: max-age=3600, public
Pragma: 
Set-Cookie: PHPSESSID=xxxxxxxxxxxxxxxxxxxxx; path=/; HttpOnly
Last-Modified: Mon, 12 Sep 2022 09:00:00 GMT
Connection: close
Transfer-Encoding: chunked
Content-Type: application/json

but yes, it seems to be the same thing. maybe there is a better fix for the issue?

marcovtwout commented 2 years ago

@jannis701 I can reproduce this issue here as well.

The PHP docs describe something about session_cache_limiter('public'); also setting the Last-Modified header:

Last-Modified: (the timestamp of when the session was last saved)

Before https://github.com/yiisoft/yii/commit/e08802ea5c178386fb1749d4d65e0ef7b436981d, calling session_cache_limiter('public'); does not set this header. After https://github.com/yiisoft/yii/commit/e08802ea5c178386fb1749d4d65e0ef7b436981d, calling session_cache_limiter('public'); does set this header, to the mtime of the entry script index.php (like described in php src)

I'm not entirely sure why the php function behaves this way, perhaps undefined behavior because Yii is freezing/unfreezing session mid-request. In any case, our custom last modified header must be set after calling sendCacheControlHeader(). This also needs to be addressed in the other cases in the same function. There are three cases that call $this->send304Header(); and then sendCacheControlHeader(), which will also sent the wrong last-modified header.

For reference see the Yii 2 implementation where last-modified header is always set after calling sendCacheControlHeader(): https://github.com/yiisoft/yii2/blob/master/framework/filters/HttpCache.php#L138-L153

marcovtwout commented 1 year ago

@jannis701 Could you confirm if https://github.com/yiisoft/yii/pull/4502 fixes your issue?

jannis701 commented 1 year ago

I confirm: #4502 fixes this issue

thank you