jshttp / fresh

HTTP request freshness testing
MIT License
161 stars 28 forks source link

latest change for max-age=0 #8

Closed eeid26 closed 10 years ago

eeid26 commented 10 years ago

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 commented 10 years ago

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

erin-noe-payne commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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

jeffpar commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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

jonathanong commented 10 years ago

anyone know if this has since been fixed by safari?

Dakuan commented 10 years ago

We did this to hack round it :-1:

https://github.com/Dakuan/jumanji

dougwilson commented 10 years ago

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 commented 10 years ago

@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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

@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 commented 10 years ago

@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.