Mercurial > p > roundup > code
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
