Repository: yiisoft/yii (language: php)

Issue:
after php 7.2-fix for CHttpCacheFilter::sendCacheControlHeader: last-modified-headers are overwritten
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

```php
    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

after php 7.2-fix for CHttpCacheFilter::sendCacheControlHeader: last-modified-headers are overwritten
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

```php
    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



Discussion / hints:
@jannis701 Could you point out the offending commit/change that broke this in the first place?
this one: https://github.com/yiisoft/yii/commit/e08802ea5c178386fb1749d4d65e0ef7b436981d
@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?
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?
@jannis701 I can reproduce this issue here as well.

The[ PHP docs](https://www.php.net/manual/en/function.session-cache-limiter.php) 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](https://github.com/php/php-src/blame/4b4a439c62444e8be198559406cc6b5e1601c828/ext/session/session.c#L1129-L1147))

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
@jannis701 Could you point out the offending commit/change that broke this in the first place?
this one: https://github.com/yiisoft/yii/commit/e08802ea5c178386fb1749d4d65e0ef7b436981d
@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?
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?
@jannis701 I can reproduce this issue here as well.

The[ PHP docs](https://www.php.net/manual/en/function.session-cache-limiter.php) 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](https://github.com/php/php-src/blame/4b4a439c62444e8be198559406cc6b5e1601c828/ext/session/session.c#L1129-L1147))

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

Fix patch:
diff --git a/CHANGELOG b/CHANGELOG
index 4a0d528a10..40a5fd19a4 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -5,6 +5,7 @@ Version 1.1.28 under development
 --------------------------------
 
 - Enh #4503: Update tests and code for PHP 8.1 compatibility, stop suppressing deprecation warnings in tests (wtommyw)
+- Bug #4484: Set Last-Modified after sending cache control headers (jannis701, wtommyw)
 - Bug #4491: Fixed limit and Offset not working correctly with MSSQL version 11 (2012) and newer (shnoulle, wtommyw)
 - Bug #4497: PHP 8.1 compatibility: Fix unserialize null in CRedisCache (kenguest, wtommyw)
 - Bug #4500: PHP 8.1 compatibility: Fix deprecation warnings in CMysql classes (csears123)
diff --git a/framework/web/filters/CHttpCacheFilter.php b/framework/web/filters/CHttpCacheFilter.php
index b16e28a43f..7f7bd2d29a 100644
--- a/framework/web/filters/CHttpCacheFilter.php
+++ b/framework/web/filters/CHttpCacheFilter.php
@@ -75,39 +75,39 @@ public function preFilter($filterChain)
 		if($etag)
 			header('ETag: '.$etag);
 
+		$this->sendCacheControlHeader();
+
+		$cacheValid = false;
 		if(isset($_SERVER['HTTP_IF_MODIFIED_SINCE'])&&isset($_SERVER['HTTP_IF_NONE_MATCH']))
 		{
 			if($this->checkLastModified($lastModified)&&$this->checkEtag($etag))
 			{
-				$this->send304Header();
-				$this->sendCacheControlHeader();
-				return false;
+				$cacheValid=true;
 			}
 		}
 		elseif(isset($_SERVER['HTTP_IF_MODIFIED_SINCE']))
 		{
 			if($this->checkLastModified($lastModified))
 			{
-				$this->send304Header();
-				$this->sendCacheControlHeader();
-				return false;
+				$cacheValid=true;
 			}
 		}
 		elseif(isset($_SERVER['HTTP_IF_NONE_MATCH']))
 		{
 			if($this->checkEtag($etag))
 			{
-				$this->send304Header();
-				$this->sendCacheControlHeader();
-				return false;
+				$cacheValid=true;
 			}
-
 		}
 
 		if($lastModified)
 			header('Last-Modified: '.gmdate('D, d M Y H:i:s', $lastModified).' GMT');
 
-		$this->sendCacheControlHeader();
+		if ($cacheValid) {
+			$this->send304Header();
+			return false;
+		}
+
 		return true;
 	}
 


Test patch: