VCL: Removing max-age directives when using user context hash

[gmanen]

We experienced a cache issue on a recent project using the User Context feature of the FOSHttpCache component.

In our use case, we use the User Context to contextualize cache according to the currency the user chooses to browse in. That is, default is we display price in USD, but you can choose to have them be displayed in EUR for example. That is stored in a cookie and we vary the cache by generating a different user hash for different currencies.

Problem was, when a user goes to a page, then changes currency, we refresh that page, and the currency switch does not work. Refreshing the page with F5 displays the correct, newly selected currency.

The reason is we set a max-age (in addition to s-max-age) on all our page Cache-Control header. The page having the same URL on every currency, the browser served content from its own cache without requesting the content from Varnish.

We hotfixed the issue by setting a max-age=0 on every cached action, but this made light to what I think is an issue with the component itself. There is no way a client, or a subsequent gateway, intervening after the initial gateway handling the user context hash, can properly cache a content subject to the User Context feature, because the Vary header and the hash itself is removed from the response.
In that case, I believe we should always override the response header and force a no-cache. Symfony HTTPCache component does this with ESI: if there are any ESI in a given page, it forces a no-cache, must-revalidate header for the same reasons.

Therefore I suggest the following changes to the Varnish configuration files:

  sub fos_user_context_deliver {
    [...]
    # If we get here, this is a real response that gets sent to the client.

    # Remove cache ttl directives.
    # Subsequent Reverse proxies and clients from this point forward cannot Vary
    # on user context hash and should not attempt to cache
    if (resp.http.X-Cache-Debug) {
        # For debug purposes, adds previous ttl to the response headers
        set resp.http.X-Original-Cache-Control = resp.http.Cache-Control;
    }

    set resp.http.Cache-Control = "must-revalidate, no-cache, private";
    [...]
  }

The debug header is optionnal, I found it useful to make sure the initial cache directives are correct despite the override.
If I'm correct in this, the same override should be implemented with Symfony HTTPCache implementation. It's not part of this PR because I believe it should reside in the FOSHttpCacheBundle (and our use case was using Varnish).

[dbu]

hm, interesting point. you are right, max-age needs to be 0 when the user can change his context. but i see no reason why this could not be just specifyed in the backend. for intermediary caches, i would expect the request to carry a session cookie which hopefully makes those caches not cache. (and you can set s-maxage: 0 in the application when using the custom ttl feature (was just in the documentation of the bundle, we add it to the library in #214). together that should be enough to not create caching problems. we should make the documentation more explicit about that.

if we hardcode overwriting the caching, we for example also stop caching routes that have no user context.

ideally, we would have something to distinguish between the vary that matters for varnish and another vary for the clients. in your case, you could then tell the client that his response vary on the cookie and use a max-age again.

[gmanen]

In our case we could vary on cookie and avoid this problem, but we would also vary on more data than required and potentially generate more cache.

I'm not sure I agree with the fact it should be left to be handled by the developer, because as is, the cache is contextualized for every single route as soon as you activate the feature. On simple applications, it might be the case that you know that some content is contextualized and some other is not, but it's not necessarily obvious.
In our use case, we might say "we don't display any price on the homepage, so it's okay to have it cached". But then what if the application evolves and we display a price on that page? We'd have to think about the cache contextualization and edit the configuration appropriately. While I agree it's possible, it adds a lot of room for errors.

To me it feels like it lessens greatly the useability of that feature, at least for our projects, which is why I would suggest a general "fix". Moreover I think the simple fact there is a Vary header that is processed and then discarded warrants overwriting, from a purely logical standpoint.

At the very least, it indeed should be explicited in the documentation. I don't think this behaviour is very obvious.

[dbu]

if the backend would do vary on cookie, you break the context caching as you (at most) cache by user. unless the currency cookie would be the only cookie. the problem with vary and cookies is that you can't vary on specific cookies, only on all of them.

the cache is only contextualized for the routes where you send the right vary header. i agree that it dangerous to not vary the whole page on it and hope that devs remember to change vary information accordingly when changing the application. but the same is with all other caching information, so i don't see what we can do apart from making the documentation more explicit about this. a pull request to improve the documentation is definitely appreciated, if you want to try to make it more clear.

@ddeboer what is your opinion on this topic? should we have an additional X-User-Vary?

i am afraid of force-changing caching instruction in the vcl. the backend application already can set max-age and s-maxage to 0. do you think we need something more? like a warning when configuring the vary: context hash header but having a max-age > 0?

[gmanen]

I was under the impresion that, as long as you activate the user context cache in your configuration, the Vary: X-User-Context-Hash is added on all responses.

If that's not the case, then I can ammend this PR this way:

sub fos_user_context_deliver {
  if (resp.http.Vary ~ "X-User-Context-Hash") {
    if (resp.http.X-Cache-Debug) {
        set resp.http.X-Original-Cache-Control = resp.http.Cache-Control;
    }
    set resp.http.Cache-Control = "must-revalidate, no-cache, private";
  }
}

The thing is, if you set a s-max-age = 0 on the application side, then even your own gateway won't cache it, and varying on the user context hash is useless anyway. So even if the developer is responsible for setting a max-age=0 for the client, you still have a problem with multiple cache gateways setup.

The problem lies with the very concept of having a Vary header interpreted by only the first cache layer and then have it discarded. You can't expect a consistent behaviour from any other cache manager handling your response after that (client or intervening gateway).

With this change, you still cache your content the way you want it to. There is no unexpected behavour. The only downside is the cache is done only by your cache manager (be it Symfony HTTPCache layer or Varnish), and can't be done by the client. Which it can't anyway.

[dbu]

we have a solution for the s-maxage problem that we will make more prominent in #234

i don't want to change the default vcl for this, but clearly we need to improve the documentation to explain the s-maxage + custom ttl thing and be more clear about the vary on context header. after calling our vcl method, the user can still manipulate the vary or cache-control header to their needs if custom ttl + s-maxage are not enough.

[gmanen]

OK, I understand your point of view, but how about putting some sort of default configuration in maybe another vcl file ? That way, if you know what you're doing and want to handle matters yourself you can do whatever in your vcl file and just import the current fos_user_context.vcl, and if you want some default consistent behaviour you can import that new fos_user_context_override.vcl.

I feel this is a tricky issue, and that it would be better to offer some kind of solution out of the box. Resolving this kind of issue and modifying vcl files can be daunting for anyone not versed in Varnish. Which I assume is why you added the ready-to-import vcl files in this repository.

[ddeboer]

There is no way a client, or a subsequent gateway, intervening after the initial gateway handling the user context hash, can properly cache a content subject to the User Context feature, because the Vary header and the hash itself is removed from the response.

This is correct. We remove them as a security measure. After all, having the hash you would be able to retrieve responses that are meant for other groups of users. This is slightly different in your case, however, where all content is accessible to all, but only differs in the price currency. I can imagine that in this case, exposing the hash is no problem. While I prefer to have it removed from the response by default so users won’t accidentally expose sensitive hashes, we could make removing it optional in our VCL.

The problem lies with the very concept of having a Vary header interpreted by only the first cache layer and then have it discarded. You can't expect a consistent behaviour from any other cache manager handling your response after that (client or intervening gateway).

Well, other caches shouldn’t need to worry about that, because user context caching happens internal to the FOSHttpCache lib and its VCL. The lib’s VCL restarts the request, asks for the hash and sets in on the response and then varies content responses on the hash.

@gmanen, does http://foshttpcache.readthedocs.org/en/latest/varnish-configuration.html?highlight=ttl#custom-ttl solve your problem? In your application, you could then set s-maxage: 0 to make sure no other proxies cache the contextualised responses.

[gmanen]

Yes, that would do it.
Although I still find the fact the user context feature "hides" caching information to subsequent proxies should warrant an automatic override. If only conceptually. But I'm fine if we agree to disagree on that point.