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

latest change for max-age=0 #8

Closed
eeid26 opened this issue Feb 19, 2014 · 19 comments
Closed

latest change for max-age=0 #8

eeid26 opened this issue Feb 19, 2014 · 19 comments

Comments

@eeid26
Copy link

eeid26 commented Feb 19, 2014

I think the latest change to look for max-age=0 in the request and decides if the cache is fresh or not, is not a valid assumptions.

the code I am talking about is in this line.

if (cc && (cc.indexOf('no-cache') !== -1 || cc.indexOf('max-age=0') !== -1)) return false;

The max-age=0 in the request is sent by the browser to validate if the cache information in the request is current or not. With this change the server will never return 304 for any request contains (max-age=0). The response will always return the content (200).

please read the meaning of max-age=0
from http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html

14.9.4 Cache Revalidation and Reload Controls

Specific end-to-end revalidation
The request includes a "max-age=0" cache-control directive, which forces each cache along the path to the origin server to revalidate its own entry, if any, with the next cache or server. The initial request includes a cache-validating conditional with the client's current validator.

max-age
When an intermediate cache is forced, by means of a max-age=0 directive, to revalidate its own cache entry, and the client has supplied its own validator in the request, the supplied validator might differ from the validator currently stored with the cache entry. In this case, the cache MAY use either validator in making its own request without affecting semantic transparency.
However, the choice of validator might affect performance. The best approach is for the intermediate cache to use its own validator when making its request. If the server replies with 304 (Not Modified), then the cache can return its now validated copy to the client with a 200 (OK) response. If the server replies with a new entity and cache validator, however, the intermediate cache can compare the returned validator with the one provided in the client's request, using the strong comparison function. If the client's validator is equal to the origin server's, then the intermediate cache simply returns 304 (Not Modified). Otherwise, it returns the new entity with a 200 (OK) response.
If a request includes the no-cache directive, it SHOULD NOT include min-fresh, max-stale, or max-age.

@tj
Copy link
Contributor

tj commented Feb 19, 2014

thought it was weird that I had never seen the mentioned behaviour (blank page), I've reverted that commit. thanks!

@tj tj closed this as completed Feb 19, 2014
@erin-noe-payne
Copy link

This discussion is based around a bug introduced in the latest rewrite of safari, which is explained here (the article discusses other cache headers but it also applies to max-age=0): http://tech.vg.no/2013/10/02/ios7-bug-shows-white-page-when-getting-304-not-modified-from-server/

The referenced PR does 'fix' the issue but only by breaking from the spec symmetrically with the safari bug. Hopefully apple will release a fix for the safari bug soon.

@jeffpar
Copy link

jeffpar commented Feb 26, 2014

Admittedly, I don't fully understand this issue, but at least the fix in fresh v0.2.1 made node+express work correctly with Safari. Even if this is actually a Safari bug, Safari works fine with apache (which returns 200 for max-age=0 requests), so why can't node do the same thing -- or at least target specific Safari agents with this or some other work-around?

Surely displaying blank pages to users of Safari is the wrong answer, so I've got to stick with fresh v0.2.1 for now.

@eeid26
Copy link
Author

eeid26 commented Feb 26, 2014

The change that was made, forces the server to return the content (http status code = 200) for all requests whether the content of the resource requested changed or not. (basically it screws up the caching).

max-age=0 sent in the request by the browser to the server is how the browser asks the server to look at the cache headers in the request (etag, If-Modified-Since, ..etc) and check if they are still current, if the content of the resource requested has not changed then the server will return 304 and if it did then it will return 200 and of course the new content.

for your statement about "Safari works fine with apache" is apache configured to look at the cache? or does it have custom code to look at the user agent and respond to fix the bug in that browser? It works with safari/apache does not mean alot.

If you really want disable the cache for that browser, then create a connect module that looks at all the requests coming from that user agent (safari) and add no-cache to the request header and then the other modules will behave like you want them to behave. Assuming you can identify that browser requests on the server.

@jeffpar
Copy link

jeffpar commented Feb 26, 2014

I just ran node and apache side-by-side, and I could see that Safari was sending "max-age=0" to apache with all its requests, and that apache was responding with 304 for all but the initial request. And I saw that node, with fresh v0.2.1, was responding with 200 on all requests. So I agree, that's bad, and I've since switched back to the latest version of fresh.

Maybe the original problem was not well understood, because all things being equal, and both apache and node are returning 304 on "max-age=0" requests, it's not clear why Safari would occasionally get a blank page stuck in its cache when using node but not when using apache.

@erin-noe-payne
Copy link

When apache returns a 304, does it come back with an html body or with no content?

@jeffpar
Copy link

jeffpar commented Feb 26, 2014

I'm not sure how to view the raw response in Safari. I'm attaching images of Apache and Node request/response data below. And unfortunately, I can't actually reproduce the "blank page" bug anymore either (I'm still in development mode, so I've been making lots of changes, and installing some other updates). So it's possible this is a moot issue now -- not sure. But I'll be keeping a close eye on it.

apache

node

@jeffpar
Copy link

jeffpar commented Feb 27, 2014

Aha, I was able to reproduce the "blank page" bug after all (screenshot below). apache never seems to return 304 on the first request; however, when node does, a blank page usually results.

In order to reproduce this, I had to turn off Express "strict routing" and disable the "express-slash" module that I recently added to my node server. This may be a clue, because I always run apache with the "mod_dir" module, which automatically performs trailing-slash redirects for directories, and yesterday I added the same feature to node, using the "express-slash" module.

So, with both servers running with trailing-slash redirects, the "blank page" issue seems to go away. I guess the next thing I should try is turning off apache's "mod_dir" module and see if I can repro the blank page bug under apache as well.

screen shot 2014-02-26 at 4 02 21 pm

@jonathanong
Copy link
Member

going to reopen this until we figure out what's going on.

@jonathanong jonathanong reopened this Jul 6, 2014
@jonathanong
Copy link
Member

anyone know if this has since been fixed by safari?

@Dakuan
Copy link

Dakuan commented Aug 8, 2014

We did this to hack round it 👎

https://github.com/Dakuan/jumanji

@dougwilson
Copy link
Contributor

Reading RFC 7232 and friends, the end-server should not do anything special for max-age=0 (except, perhaps, actually validate that the headers are still current, which this module actually always does). it's main purpose is that an intermediate proxy could be caching the upstream and responding by only checking it's cached last-modified date, for example. sending max-age=0 to this intermediate proxy is supposed to make this proxy then actually make a request to the upstream and then re-evaluate it's cache.

TL;DR this module actually does what max-age=0 asks for: re-evaluates if the cached representation is current (because it's the only mode this module works in). max-age=0 does not mean always send a 200.

@ericf
Copy link

ericf commented Sep 8, 2014

@dougwilson I don't think this issue should be closed, there's still an issues that causes apps using this module to break in Safari.

@dougwilson
Copy link
Contributor

Yes, but can you propose a PR, please? Also, can you please let me know what Apache and nginx are doing with Safai in these cases?

@dougwilson
Copy link
Contributor

What I'll do for you, @ericf , is re-open this issue pending when I can find and read the source code for Apache and Nginx to see exactly what their code is doing and will consider that. If anyone wants to find and link me to the source code for Apache/Nginx caching header handling, that would be helpful :)

@calvintwr
Copy link

This is affecting me as well. It seems to happen especially when inside my routing I have conditional checks, and calling a different res.render at the end of the conditions.

app.get('/', function(req, res, next) {
    if(req.isAuthenticated) {
        return res.render([some view]);
    }
    return res.render([ not authorised view ]);
});

The "blank page" observation in safari is consistent with my case. In mobile safari, the first get request after a complete cache and history clear will lead to it seemingly loading the page (progress bar shows maybe 1/3 progress with loading indicator turning), with a blank page showing. Only after say... 20secs - 1 minute, the load will complete. However, refreshing the page while it is stuck will immediately load up the page with no issues.

@dougwilson
Copy link
Contributor

I have looked into this at length and this is not an express-specific issue but affects nginx and Apache as well when caching is enabled (express enables etags by default). This library will not be changing, as the only solution is to do user-agent sniffing for the affects Safari versions, which is 100% beyond the scope of this module. The module https://github.com/Dakuan/jumanji will do this for you.

@Dakuan
Copy link

Dakuan commented Oct 14, 2014

@dougwilson is it worth putting a note about this in the documentation somewhere? Would be a shame if people wasted time trying to find out what's going on with this. They might not look in closed tickets first off.

@dougwilson
Copy link
Contributor

@Dakuan yea, I may be able to do that. This module is a very low-level module for doing freshness testing as the last entity in the request chain.

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

No branches or pull requests

9 participants