Forward port to trunk from 3.0.x [1703]. Added checking of 'Vary' header before responding with cached content.

3 files changed, 54 insertions, 5 deletions

diff --git a/cherrypy/lib/caching.py b/cherrypy/lib/caching.py
index c49f315b..2eab8447 100644
--- a/cherrypy/lib/caching.py
+++ b/cherrypy/lib/caching.py
@@ -123,9 +123,28 @@ def get(invalid_methods=("POST", "PUT", "DELETE"), **kwargs):
     request.cacheable = not c
     if c:
         response = cherrypy.response
-        s, h, b, create_time = cache_data
+        s, h, b, create_time, original_req_headers = cache_data
         
-        # Make a copy. See http://www.cherrypy.org/ticket/721
+        # Check 'Vary' selecting headers. If any headers mentioned in "Vary"
+        # differ between the cached and current request, bail out and
+        # let the rest of CP handle the request. This should properly
+        # mimic the behavior of isolated caches as RFC 2616 assumes:
+        # "If the selecting request header fields for the cached entry
+        # do not match the selecting request header fields of the new
+        # request, then the cache MUST NOT use a cached entry to satisfy
+        # the request unless it first relays the new request to the origin
+        # server in a conditional request and the server responds with
+        # 304 (Not Modified), including an entity tag or Content-Location
+        # that indicates the entity to be used.
+        # TODO: can we store multiple variants based on Vary'd headers?
+        for header_element in h.elements('Vary'):
+            key = header_element.value
+            if original_req_headers[key] != request.headers.get(key, 'missing'):
+                request.cached = False
+                request.cacheable = True
+                return False
+        
+        # Copy the response headers. See http://www.cherrypy.org/ticket/721.
         response.headers = rh = http.HeaderMap()
         for k in h:
             dict.__setitem__(rh, k, dict.__getitem__(h, k))
@@ -161,8 +180,16 @@ def tee_output():
         if response.headers.get('Pragma', None) != 'no-cache':
             # save the cache data
             body = ''.join(output)
+            vary = [he.value for he in
+                    cherrypy.response.headers.elements('Vary')]
+            if vary:
+                sel_headers = dict([(k, v) for k, v
+                                    in cherrypy.request.headers.iteritems()
+                                    if k in vary])
+            else:
+                sel_headers = {}
             cherrypy._cache.put((response.status, response.headers or {},
-                                 body, response.time))
+                                 body, response.time, sel_headers))
     
     response = cherrypy.response
     response.body = tee(response.body)
diff --git a/cherrypy/test/test_caching.py b/cherrypy/test/test_caching.py
index cf938298..b83f446c 100644
--- a/cherrypy/test/test_caching.py
+++ b/cherrypy/test/test_caching.py
@@ -1,8 +1,10 @@
 from cherrypy.test import test
 test.prefer_parent_path()
 
+import gzip
 import os
 curdir = os.path.join(os.getcwd(), os.path.dirname(__file__))
+import StringIO
 
 import cherrypy
 from cherrypy.lib import http
@@ -68,7 +70,8 @@ def setup_server():
     
     cherrypy.tree.mount(Root())
     cherrypy.tree.mount(UnCached(), "/expires")
-    cherrypy.config.update({'environment': 'test_suite'})
+    cherrypy.config.update({'environment': 'test_suite',
+                            'tools.gzip.on': True})
 
 
 from cherrypy.test import helper
@@ -99,10 +102,28 @@ class CacheTest(helper.CPWebCase):
         self.assertBody('visit #3')
         self.getPage("/", method="DELETE")
         self.assertBody('visit #4')
+        
         # The previous request should have invalidated the cache,
         # so this request will recalc the response.
+        zbuf = StringIO.StringIO()
+        zfile = gzip.GzipFile(mode='wb', fileobj=zbuf, compresslevel=9)
+        zfile.write("visit #5")
+        zfile.close()
+        
+        self.getPage("/", method="GET", headers=[('Accept-Encoding', 'gzip')])
+        self.assertHeader('Content-Encoding', 'gzip')
+        self.assertInBody(zbuf.getvalue()[:3])
+        
+        # Now check that a second request gets the gzip header and gzipped body
+        self.getPage("/", method="GET", headers=[('Accept-Encoding', 'gzip')])
+        self.assertHeader('Content-Encoding', 'gzip')
+        self.assertInBody(zbuf.getvalue()[:3])
+        
+        # Now check that a third request that doesn't accept gzip
+        # gets another hit.
         self.getPage("/", method="GET")
-        self.assertBody('visit #5')
+        self.assertNoHeader('Content-Encoding')
+        self.assertBody('visit #6')
     
     def testExpiresTool(self):
         
diff --git a/cherrypy/test/test_encoding.py b/cherrypy/test/test_encoding.py
index 548729a3..4d8fb73b 100644
--- a/cherrypy/test/test_encoding.py
+++ b/cherrypy/test/test_encoding.py
@@ -117,6 +117,7 @@ class EncodingTests(helper.CPWebCase):
         self.getPage('/gzip/', headers=[("Accept-Encoding", "gzip")])
         self.assertInBody(zbuf.getvalue()[:3])
         self.assertHeader("Vary", "Accept-Encoding")
+        self.assertHeader("Content-Encoding", "gzip")
         
         # Test when gzip is denied.
         self.getPage('/gzip/', headers=[("Accept-Encoding", "identity")])