diff roundup/cgi/client.py @ 8185:e84d4585b16d

fix(web): issue2551356. Add etag header for not-modified (304) request. When a 304 is returned to a conditional request for a static file, print an ETag for the response. ETag was always sent with a 200 response. This also adds initial support for if-none-match conditional requests for static files. Changes: Refactors the if-modified-since code out to a method. It moves a file stat call from serve_static_file to _serve_file so that an etag can be generated by both serve_static_file and serve_file which call _serve_file. Tests added. This does not test the codepath where serve_file pulls content from the database rather than from a local file on disk. Test mocking _serve_file changed to account for 5th argument to serve_file BREAKING CHANGE: function signature for client.py-Client::_serve_file() now has 5 not 4 parameters (added etag param). Since this is a "hidden" method I am not too worried about it.
author John Rouillard <rouilj@ieee.org>
date Tue, 10 Dec 2024 16:06:13 -0500
parents bd628e64725f
children b938fd5223ae
line wrap: on
line diff
--- a/roundup/cgi/client.py	Sun Dec 08 21:36:29 2024 -0500
+++ b/roundup/cgi/client.py	Tue Dec 10 16:06:13 2024 -0500
@@ -1949,7 +1949,7 @@
 
         lmt = klass.get(nodeid, 'activity').timestamp()
 
-        self._serve_file(lmt, mime_type, content, filename)
+        self._serve_file(lmt, None, mime_type, content, filename)
 
     def serve_static_file(self, file):
         """ Serve up the file named from the templates dir
@@ -1989,9 +1989,6 @@
         if filename is None:  # we didn't find a filename
             raise NotFound(file)
 
-        # last-modified time
-        lmt = os.stat(filename)[stat.ST_MTIME]
-
         # detemine meta-type
         file = str(file)
         mime_type = mimetypes.guess_type(file)[0]
@@ -2009,28 +2006,59 @@
             self.additional_headers['Cache-Control'] = \
                             self.Cache_Control[mime_type]
 
-        self._serve_file(lmt, mime_type, '', filename)
-
-    def _serve_file(self, lmt, mime_type, content=None, filename=None):
-        """ guts of serve_file() and serve_static_file()
+        self._serve_file(None, None, mime_type, '', filename)
+
+    def _serve_file(self, lmt, etag, mime_type, content=None, filename=None):
+        """guts of serve_file() and serve_static_file()
+
+            if lmt or etag are None, derive them from file filename.
+
+            Handles if-modified-since and if-none-match etag
+            conditional gets.
+
+            It produces an raw etag header without encoding suffix.
+            But it adds Accept-Encoding to the vary header.
+
         """
-
-        # spit out headers
-        self.additional_headers['Last-Modified'] = email.utils.formatdate(lmt,
-	 usegmt=True)
-
-        ims = None
-        # see if there's an if-modified-since...
-        #    used if this is run behind a non-caching http proxy
-        if hasattr(self.request, 'headers'):
-            ims = self.request.headers.get('if-modified-since')
-        elif 'HTTP_IF_MODIFIED_SINCE' in self.env:
-            # cgi will put the header in the env var
-            ims = self.env['HTTP_IF_MODIFIED_SINCE']
-        if ims:
-            ims = email.utils.parsedate(ims)[:6]
-            lmtt = time.gmtime(lmt)[:6]
-            if lmtt <= ims:
+        if filename:
+            stat_info = os.stat(filename)
+
+            if lmt is None:
+                # last-modified time
+                lmt = stat_info[stat.ST_MTIME]
+            if etag is None:
+                # FIXME: maybe etag should depend on encoding.
+                # it is an apache compatible etag without encoding.
+                etag = '"%x-%x-%x"' % (stat_info[stat.ST_INO],
+                                       stat_info[stat.ST_SIZE],
+                                       stat_info[stat.ST_MTIME])
+
+            # spit out headers for conditional request
+            self.setHeader("ETag", etag)
+            self.additional_headers['Last-Modified'] = \
+                email.utils.formatdate(lmt, usegmt=True)
+
+            inm = None
+            # ETag is a more strict check than modified date. Use etag
+            # check if available. Skip testing modified data.
+            if hasattr(self.request, 'headers'):
+                inm = self.request.headers.get('if-none-match')
+            elif 'HTTP_IF_NONE_MATCH' in self.env:
+                # maybe the cgi will put the header in the env var
+                inm = self.env['HTTP_ETAG']
+            if inm and etag == inm:
+                # because we can compress, always set Accept-Encoding
+                # value. Otherwise caches can serve up the wrong info
+                # if their cached copy has no compression.
+                self.setVary("Accept-Encoding")
+                '''
+                to solve issue2551356 I may need to determine
+                the content encoding.
+                if (self.determine_content_encoding()):
+                '''
+                raise NotModified
+
+            if self.if_not_modified_since(lmt):
                 # because we can compress, always set Accept-Encoding
                 # value. Otherwise caches can serve up the wrong info
                 # if their cached copy has no compression.
@@ -2051,6 +2079,27 @@
             self.additional_headers['Content-Length'] = str(len(content))
             self.write(content)
 
+    def if_not_modified_since(self, lmt):
+        ims = None
+        # see if there's an if-modified-since...
+        if hasattr(self.request, 'headers'):
+            ims = self.request.headers.get('if-modified-since')
+        elif 'HTTP_IF_MODIFIED_SINCE' in self.env:
+            # cgi will put the header in the env var
+            ims = self.env['HTTP_IF_MODIFIED_SINCE']
+
+        if ims:
+            datestamp = email.utils.parsedate(ims)
+            if datestamp is not None:
+                ims = datestamp[:6]
+            else:
+                # set to beginning of time so whole file will be sent
+                ims = (0, 0, 0, 0, 0, 0)
+            lmtt = time.gmtime(lmt)[:6]
+            return lmtt <= ims
+
+        return False
+
     def send_error_to_admin(self, subject, html, txt):
         """Send traceback information to admin via email.
            We send both, the formatted html (with more information) and

Roundup Issue Tracker: http://roundup-tracker.org/