monterail / guidelines

[DEPRECATED] We are Ruby on Rails experts from Poland. Think hussars. Solid & winged. These are our guidelines.
71 stars 17 forks source link

Disable Rack::Cache for rails < 4 #176

Closed teamon closed 10 years ago

teamon commented 11 years ago

In Rails prior to 4 when there is set

config.action_controller.perform_caching = true

Rails will automagicly add Rack::Cache middleware to the top of stack. This will cause request with cache headers to be cached which can break e.g. authentication when you want to send cache headers but also always require http basic auth. It is also much better to use nginx or varnish as cache in case auth is not an issue.

For Rails 4 - https://github.com/rails/rails/pull/7838

@jandudulski, @sheerun, @chytreg, @teamon, @szajbus, @Ostrzy, @jcieslar, @tallica

jandudulski commented 11 years ago

Never got this pain, but :+1:

szajbus commented 11 years ago

But you don't always send cache headers, do you?

Not sure if this should be off by default. A guideline for when to use it and when not would be better.

teamon commented 11 years ago

If you do not send cache headers Rack::Cache is noop.

On Friday, 9 August 2013, Michał Szajbe wrote:

But you don't always send cache headers, do you?

Not sure if this should be off by default. A guideline for when to use it and when not would be better.

— Reply to this email directly or view it on GitHubhttps://github.com/monterail/guidelines/issues/176#issuecomment-22361494 .

szajbus commented 11 years ago

I meant that for some request you want to send it and for others not.

teamon commented 11 years ago
  1. Rack::Cache on + sending cache headers = pain
  2. Rack::Cache on + not sending cache headers = doesn't matter (Rack::Cache is transparent)
  3. Rack::Cache off + sending cache headers = profit
  4. Rack::Cache off + not sending cache headers = doesn't matter (Rack::Cache is transparent)
jandudulski commented 11 years ago

So.. what is the purpose of this Rack::Cache?

teamon commented 11 years ago
  1. Rack::Cache on + sending cache headers + auth = pain
  2. Rack::Cache on + sending cache headers + no auth = ok
  3. Rack::Cache on + not sending cache headers + auth = doesn't matter (Rack::Cache is transparent)
  4. Rack::Cache on + not sending cache headers + no auth = doesn't matter (Rack::Cache is transparent)
  5. Rack::Cache off + sending cache headers + auth = ok
  6. Rack::Cache off + sending cache headers + no auth = ok
  7. Rack::Cache off + not sending cache headers + auth = doesn't matter (Rack::Cache is transparent)
  8. Rack::Cache off + not sending cache headers + no auth = doesn't matter (Rack::Cache is transparent)
jandudulski commented 11 years ago

Wait... so whether you use or not Rack::Cache you will cache when cache headers were sent?

teamon commented 11 years ago

Rack::Cache will only cache if it is enabled

sheerun commented 11 years ago

we have 3 ok and only one pain ;) why you send cache headers on auth?

On Fri, Aug 9, 2013 at 11:59 AM, Tymon Tobolski notifications@github.comwrote:

Rack::Cache will only cache in it is on

— Reply to this email directly or view it on GitHubhttps://github.com/monterail/guidelines/issues/176#issuecomment-22385581 .

teamon commented 11 years ago

I want to cache api secured with basic auth.

chytreg commented 11 years ago

@teamon What about this guideline https://github.com/monterail/guidelines/blob/master/rails.md#rack-cache should we modify it in prior of disable rack cache?

sheerun commented 11 years ago

@szajbus What's the decision?

teamon commented 11 years ago

@chytreg There are the correct settings if you do use rack cache, but the general guideline will be to disable it completely.

szajbus commented 11 years ago

I am still not convinced. If does cause pain in some situations why not disable cache only for them by sending appropriate headers. By default all non-GET requests should not be cached I guess.

I understand your use case. Requests with Basic Auth headers indeed will not play well here. But maybe a better solution would be to set appropriate headers for such requests.

http://stackoverflow.com/questions/49547/making-sure-a-web-page-is-not-cached-across-all-browsers/2068407#2068407

teamon commented 11 years ago

My use case is strictly http API. I do want to have internal server side cache, basic authentication and client side cache using ETag, Cache-Control and Last-Modified. Rack::Cache is just put in wrong place, before authentication. And it is disabled completely in rails 4 which I'd take as a suggestion not to use it at all.

Tymon Tobolski Monterail.com (http://Monterail.com) – Ruby on Rails development agency

On Wednesday, September 18, 2013 at 12:33 AM, Michał Szajbe wrote:

I am still not convinced. If does cause pain in some situations why not disable cache only for them by sending appropriate headers. By default all non-GET requests should not be cached I guess. I understand your use case. Requests with Basic Auth headers indeed will not play well here. But maybe a better solution would be to set appropriate headers for such requests. http://stackoverflow.com/questions/49547/making-sure-a-web-page-is-not-cached-across-all-browsers/2068407#2068407

— Reply to this email directly or view it on GitHub (https://github.com/monterail/guidelines/issues/176#issuecomment-24628047).

szajbus commented 11 years ago

Even when it is before authentication it would not cache its responses if the responses set appropriate headers from the start, right? Or I still don't see something.

Rails4 is a very good hint though! So I lean towards :+1:

teamon commented 11 years ago

If response have no headers rack::cache will not cache it, nor will client ;]

charlesdeb commented 10 years ago

Hi folks, this will also disable Rack::Cache in your app apparently:

config.action_dispatch.rack_cache =  nil

from the bottom of this page: http://blog.tonycode.com/archives/418