User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:126.96.36.199) Gecko/20060508 Firefox/188.8.131.52 Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:184.108.40.206) Gecko/20060508 Firefox/220.127.116.11 Firefox does not appear to cache responses sent with "Cache-Control: public" header, if they are sent over SSL/TLS. While I'm aware that Firefox generally doesn't cache responses over TLS for security, in cases where the response is explicitely defined to be public, this would seem to be incorrect behaviour. I'm testing caching behaviour by use of the "Cache Source" entry from the page info window. For reference, we're using TLS in this instance to ensure integrity, rather than security (it's important that the user receives the response we're sending, it's not important if anyone else can read that). Reproducible: Always Steps to Reproduce: 1. Go to https://bowmore.dcs.st-andrews.ac.uk/mms/images/uparrow.png 2. Right click -> View Page Info Actual Results: Cache source is listed as "Not cached". Expected Results: Cache source should be "Disk cache". For example, go to http://bowmore.dcs.st-andrews.ac.uk/mms/images/uparrow.png and then View Page Info.
Reporter, do you still see this problem with the latest Firefox 2? If not, can you please close this bug as WORKSFORME. Thanks!
Still seeing same problem with 2.0.4 on OS X.
Just tested with 3.0a5 and present there too, if that's relevant.
Yes it is relevant, thanks!
Confirming bug, moving to networking component. Couldn't load the reporter's file, but tested with URIs from my webmail which can be loaded over SSL or not. The spec says Cache-Control: public "MAY" be cached, not MUST or SHOULD. We're not violating a spec, but we're also not taking advantage of optimization hints the site is giving us. http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9.1 bug 205921 added a pref that puts SSL files in the cache, but that stores more than just the explicitly "public" pages. Plus no one's going to find and use that pref, Firefox should "just work". Unless that pref is enabled we go ahead and set the load flag INHIBIT_PERSISTENT_CACHING on all SSL loads, overriding any Cache-Control header.
Adding "perf" keyword: this would speed loading of sites that use SSL to protect user-data, but also load large static resources that are not private (logos/images, stylesheets, scripts). If we're going to cache any sites at all, a site where a user has sensitive data stored is one they're likely to return to so caching what we can safely would be a win.
Moving to blocking for 1.9 as this might be a significant perf win...
10 years ago
Created attachment 290421 [details] [diff] [review] patch should I add a pref for this?
Comment on attachment 290421 [details] [diff] [review] patch >Index: netwerk/protocol/http/src/nsHttpChannel.cpp >+ // Only cache SSL content on disk if the server sent a >+ // Cache-Control: public header, or if the user set the pref >+ if (!gHttpHandler->IsPersistentHttpsCachingEnabled() && >+ mConnectionInfo->UsingSSL() && !mResponseHead->CacheControlPublic()) >+ mLoadFlags |= INHIBIT_PERSISTENT_CACHING; It seems like you want to always set INHIBIT_PERSISTENT_CACHING if IsPersistentHttpCachingEnabled() returns false. So, I think you want this condition instead: if (!gHttpHandler->IsPersistentHttpsCachingEnabled() || ( mConnectionInfo->UsingSSL() && !mResponseHead->CacheControlPublic())) Otherwise, this change looks good to me. r+sr=darin with that fix.
Well, those aren't the semantics I wanted to implement. I wanted: - If the pref is set, cache all SSL to disk (except no-store) - otherwise, cache only Cache-Control: public SSL content
per discussion with darin, I'll rename IsPersistentHttpsCachingEnabled to CanCacheAllSSLContent
Created attachment 290542 [details] [diff] [review] patch v2
Checking in nsHttpChannel.cpp; /cvsroot/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp,v <-- nsHttpChannel.cpp new revision: 1.324; previous revision: 1.323 done Checking in nsHttpHandler.h; /cvsroot/mozilla/netwerk/protocol/http/src/nsHttpHandler.h,v <-- nsHttpHandler.h new revision: 1.53; previous revision: 1.52 done Checking in nsHttpResponseHead.cpp; /cvsroot/mozilla/netwerk/protocol/http/src/nsHttpResponseHead.cpp,v <-- nsHttpResponseHead.cpp new revision: 1.48; previous revision: 1.47 done Checking in nsHttpResponseHead.h; /cvsroot/mozilla/netwerk/protocol/http/src/nsHttpResponseHead.h,v <-- nsHttpResponseHead.h new revision: 1.28; previous revision: 1.27 done
Is this worth taking on the 1.8 branch? We'd also need the fixes for minor regression 406869 and for bug 262116 which starts mattering if caching is the default.
It would be really nice if this got fixed in FF 2.0 As a result of this bug I have to dial-down security in my application in order to allow reasonable load times, which in return reduces security for the users. Please consider this! Thanks.
Not going to get a branch patch before the cut-off, and not clear this fits our branch criteria anyway. Will look in the next release.
(In reply to comment #16) > Not going to get a branch patch before the cut-off, and not clear this fits our > branch criteria anyway. Will look in the next release. > Agreed - we should be branch landing only critical security and stability issues - this is a perf win and there are hundreds of others in FF3 that are not easily backported. But don't worry - folks will upgrade to FF3 quickly...
Minusing for the 1.8 branch, although nice to have it doesn't really fit the criteria.