Skip to content
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

Disable HTTP caches on preview links (for 2.x) #89

Merged

Conversation

chillu
Copy link
Member

@chillu chillu commented Jul 20, 2018

@chillu
Copy link
Member Author

chillu commented Jul 20, 2018

Tested with the following state:

.env

SS_ENVIRONMENT_TYPE="live"

Enabled caching on pages. Removed comments to avoid PHP sessions started.
Running silverstripe/silverstripe-framework#8267 and https://github.com/tractorcow/silverstripe-fluent/issues/440 patches for the same reason.

Result (confirming cache-control flags come through):

curl -I http://cwp2-installer.vagrant/about-us
HTTP/1.1 200 OK
Server: nginx
Date: Fri, 20 Jul 2018 03:44:18 GMT
Content-Type: text/html; charset=utf-8
Connection: keep-alive
vary: X-Requested-With,X-Forwarded-Protocol
cache-control: must-revalidate, max-age=10
expires: Fri, 20 Jul 2018 03:44:28 GMT
etag: "2e67c5bd2f0c456e7cb2b3285eecd741"
x-xss-protection: 1; mode=block

Result on preview link (note the no-cache bits):

curl -I http://cwp2-installer.vagrant/preview/c0432ed6ee045834/0a1413f336a1140b
HTTP/1.1 200 OK
Server: nginx
Date: Fri, 20 Jul 2018 04:09:20 GMT
Content-Type: text/html; charset=utf-8
Connection: keep-alive
Set-Cookie: PHPSESSID=c9vr8p5b2d1cfpntoncl6d8b2c; expires=Fri, 20-Jul-2018 05:09:20 GMT; Max-Age=3600; path=/; HttpOnly
Set-Cookie: PHPSESSID=c9vr8p5b2d1cfpntoncl6d8b2c; expires=Fri, 20-Jul-2018 05:09:20 GMT; Max-Age=3600; path=/; HttpOnly
vary: X-Requested-With,X-Forwarded-Protocol
cache-control: no-cache, no-store, must-revalidate
x-xss-protection: 1; mode=block

@@ -50,6 +51,9 @@ class ShareDraftController extends Controller
*/
public function preview(HTTPRequest $request)
{
// Ensure this URL doesn't get picked up by HTTP caches
HTTPCacheControlMiddleware::singleton()->disableCache();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not private cache?

Copy link
Member Author

Choose a reason for hiding this comment

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

Being conservative? Sure, you could treat repeat requests from the same browser as cacheable within this browser, in case something else in the request processing set a max age. But that's pretty low value. The important bit is to ensure there's no caching, and the default way of achieving this is through disableCache(). If you feel strongly about this, feel free to change it to privateCache() (for both 1.x and 2.x PRs)

@robbieaverill robbieaverill merged commit 192ff8b into silverstripe:master Jul 25, 2018
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.

None yet

3 participants