Skip to content

[HttpKernel] Fix Apache mod_expires Session Cache-Control issue #33487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 8, 2019
Merged

[HttpKernel] Fix Apache mod_expires Session Cache-Control issue #33487

merged 1 commit into from
Sep 8, 2019

Conversation

pbowyer
Copy link
Contributor

@pbowyer pbowyer commented Sep 6, 2019

Q A
Branch? 3.4 for bug fixes
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
License MIT

Apaches's mod_expires is a widely used module to set HTTP caching headers. It allows you to set a default cache lifetime as well as lifetimes by mime_type.

When an application server has set a Cache-Control header, mod_expires ignores this and sets its own, resulting in duplicate Cache-Control headers and conflicting information. It does this unless the application server sets an Expires header, in which case mod_expires does nothing. This is documented on the link above:

When the Expires header is already part of the response generated by the server, for example when generated by a CGI script or proxied from an origin server, this module does not change or add an Expires or Cache-Control header.

Symfony automatically sets a Cache-Control header if a session exists. This patch adds an Expires header to ensure it's respected by mod_expires.

Example 1

With the following Apache config:

<IfModule mod_expires.c>
    ExpiresActive on
    ExpiresDefault                                      "access plus 1 month"
</IfModule>

The HTTP response headers are:

Without the patch

HTTP/1.1 200 OK
Date: Fri, 06 Sep 2019 08:02:02 GMT
Server: Apache/2.4.37 (Ubuntu)
Cache-Control: max-age=0, must-revalidate, private
Cache-Control: max-age=2592000
Expires: Sun, 06 Oct 2019 08:02:00 GMT
Vary: Accept-Encoding
Content-Encoding: gzip
Content-Length: 13099
Connection: close
Content-Type: text/html; charset=UTF-8

With the patch

HTTP/1.1 200 OK
Date: Fri, 06 Sep 2019 08:21:34 GMT
Server: Apache/2.4.37 (Ubuntu)
Cache-Control: max-age=0, must-revalidate, private
Expires: Fri, 06 Sep 2019 08:21:34 GMT
Vary: Accept-Encoding
Content-Encoding: gzip
Content-Length: 13098
Connection: close
Content-Type: text/html; charset=UTF-8

Example 2

With the following Apache config:

<IfModule mod_expires.c>
    ExpiresActive on
    ExpiresDefault                                      "access plus 1 month"
    ExpiresByType text/html                             "access plus 0 seconds"
</IfModule>

Without the patch

HTTP/1.1 200 OK
Date: Fri, 06 Sep 2019 08:18:40 GMT
Server: Apache/2.4.37 (Ubuntu)
Cache-Control: max-age=0, must-revalidate, private
Cache-Control: max-age=0
Expires: Fri, 06 Sep 2019 08:18:39 GMT
Vary: Accept-Encoding
Content-Encoding: gzip
Content-Length: 13099
Connection: close
Content-Type: text/html; charset=UTF-8

With the patch

HTTP/1.1 200 OK
Date: Fri, 06 Sep 2019 08:20:40 GMT
Server: Apache/2.4.37 (Ubuntu)
Cache-Control: max-age=0, must-revalidate, private
Expires: Fri, 06 Sep 2019 08:20:40 GMT
Vary: Accept-Encoding
Content-Encoding: gzip
Content-Length: 13100
Connection: close
Content-Type: text/html; charset=UTF-8

@pbowyer pbowyer changed the title Fix Apache mod_expires Cache-Control issue Fix Apache mod_expires Session Cache-Control issue Sep 6, 2019
@nicolas-grekas nicolas-grekas added this to the 4.3 milestone Sep 6, 2019
@nicolas-grekas
Copy link
Member

That's for 3.4, right? Could you please add a test case also?

@nicolas-grekas nicolas-grekas changed the title Fix Apache mod_expires Session Cache-Control issue [HttpKernel] Fix Apache mod_expires Session Cache-Control issue Sep 7, 2019
@pbowyer
Copy link
Contributor Author

pbowyer commented Sep 7, 2019

Yes, will redo against 3.4. The code didn't apply to both versions, so it needs a separate patch for each.

@pbowyer pbowyer changed the base branch from 4.3 to 3.4 September 7, 2019 14:25
@pbowyer
Copy link
Contributor Author

pbowyer commented Sep 7, 2019

@nicolas-grekas ✔️ Now against 3.4 ✔️ Test coverage added

Had to force-push a second time to get Appveyor to rerun, as I force-pushed the first time before changing the base branch for the PR.

@fabpot
Copy link
Member

fabpot commented Sep 8, 2019

Thank you @pbowyer.

fabpot added a commit that referenced this pull request Sep 8, 2019
…issue (pbowyer)

This PR was squashed before being merged into the 3.4 branch (closes #33487).

Discussion
----------

[HttpKernel] Fix Apache mod_expires Session Cache-Control issue

| Q             | A
| ------------- | ---
| Branch?       | 3.4 for bug fixes <!-- see below -->
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| License       | MIT

Apaches's [mod_expires](https://httpd.apache.org/docs/current/mod/mod_expires.html) is a widely used module to set HTTP caching headers. It allows you to set a default cache lifetime as well as lifetimes by mime_type.

When an application server has set a `Cache-Control` header, mod_expires ignores this and sets its own, resulting in duplicate `Cache-Control` headers and conflicting information. It does this _unless_ the application server sets an `Expires` header, in which case mod_expires does nothing. This is documented on the link above:

> When the `Expires` header is already part of the response generated by the server, for example when generated by a CGI script or proxied from an origin server, this module does not change or add an `Expires` or `Cache-Control` header.

Symfony automatically sets a `Cache-Control` header if a session exists. This patch adds an `Expires` header to ensure it's respected by mod_expires.

## Example 1
With the following Apache config:
```apache
<IfModule mod_expires.c>
    ExpiresActive on
    ExpiresDefault                                      "access plus 1 month"
</IfModule>
```
The HTTP response headers are:
### Without the patch
```
HTTP/1.1 200 OK
Date: Fri, 06 Sep 2019 08:02:02 GMT
Server: Apache/2.4.37 (Ubuntu)
Cache-Control: max-age=0, must-revalidate, private
Cache-Control: max-age=2592000
Expires: Sun, 06 Oct 2019 08:02:00 GMT
Vary: Accept-Encoding
Content-Encoding: gzip
Content-Length: 13099
Connection: close
Content-Type: text/html; charset=UTF-8
```
### With the patch
```
HTTP/1.1 200 OK
Date: Fri, 06 Sep 2019 08:21:34 GMT
Server: Apache/2.4.37 (Ubuntu)
Cache-Control: max-age=0, must-revalidate, private
Expires: Fri, 06 Sep 2019 08:21:34 GMT
Vary: Accept-Encoding
Content-Encoding: gzip
Content-Length: 13098
Connection: close
Content-Type: text/html; charset=UTF-8
```

## Example 2
With the following Apache config:
```apache
<IfModule mod_expires.c>
    ExpiresActive on
    ExpiresDefault                                      "access plus 1 month"
    ExpiresByType text/html                             "access plus 0 seconds"
</IfModule>
```
### Without the patch
```
HTTP/1.1 200 OK
Date: Fri, 06 Sep 2019 08:18:40 GMT
Server: Apache/2.4.37 (Ubuntu)
Cache-Control: max-age=0, must-revalidate, private
Cache-Control: max-age=0
Expires: Fri, 06 Sep 2019 08:18:39 GMT
Vary: Accept-Encoding
Content-Encoding: gzip
Content-Length: 13099
Connection: close
Content-Type: text/html; charset=UTF-8
```
### With the patch
```
HTTP/1.1 200 OK
Date: Fri, 06 Sep 2019 08:20:40 GMT
Server: Apache/2.4.37 (Ubuntu)
Cache-Control: max-age=0, must-revalidate, private
Expires: Fri, 06 Sep 2019 08:20:40 GMT
Vary: Accept-Encoding
Content-Encoding: gzip
Content-Length: 13100
Connection: close
Content-Type: text/html; charset=UTF-8
```

Commits
-------

9e94276 [HttpKernel] Fix Apache mod_expires Session Cache-Control issue
@fabpot fabpot merged commit 9e94276 into symfony:3.4 Sep 8, 2019
This was referenced Oct 7, 2019
@@ -56,6 +56,7 @@ public function onKernelResponse(FilterResponseEvent $event)

if ($session instanceof Session ? $session->getUsageIndex() !== end($this->sessionUsageStack) : $session->isStarted()) {
$event->getResponse()
->setExpires(new \DateTime())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it on purpose, that this even overrides the expires header that has been set before this line?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is about forcing the page to be uncacheable anyway. So yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants