Repository: evansd/whitenoise

Issue:
HTTP 304 being returned when Etag doesn't match
Hi!

Currently WhiteNoise:
* includes both an `Etag` and `Last-Modified` header on all responses
* checks incoming requests to see if they specified a `If-None-Match` (used for Etag) or `If-Modified-Since` header, to determine whether to return an HTTP 304 response

However the current implementation falls back to `If-Modified-Since` even if there was a non-matching `If-None-Match`, rather than returning false:
https://github.com/evansd/whitenoise/blob/1b5a7f3a0f151ee1449d88da9e4813644e435fd4/whitenoise/responders.py#L169-L176

This is problematic since:
* browsers (at least Firefox and Chrome, haven't checked the rest) always send both sets of headers
* `If-Modified-Since` is unreliable in the case of deployment rollbacks (since by design the comparison is greater than or equal to modified time, rather than just equal to)

Demo:

```bash
curl -I https://treeherder.mozilla.org/ \
  -H 'If-None-Match: "this-will-not-match"' \
  -H 'If-Modified-Since: Thu, 25 Oct 2099 16:00:44 GMT'
```

Currently this returns HTTP 304 even though it has a non-matching `If-None-Match` header value.

This caused frontend breakage after a rollback we performed today, which persisted until users force-refreshed:

----

![screenshot_2018-11-12 12 autoland](https://user-images.githubusercontent.com/501702/48365062-b6745300-e6a1-11e8-8dd9-cc37aa00d5d8.png)

Discussion:

Patch:
diff --git a/whitenoise/responders.py b/whitenoise/responders.py
index c36c887..debedcc 100644
--- a/whitenoise/responders.py
+++ b/whitenoise/responders.py
@@ -167,8 +167,9 @@ class StaticFile(object):
         return alternatives
 
     def is_not_modified(self, request_headers):
-        if self.etag == request_headers.get('HTTP_IF_NONE_MATCH'):
-            return True
+        previous_etag = request_headers.get('HTTP_IF_NONE_MATCH')
+        if previous_etag is not None:
+            return previous_etag == self.etag
         try:
             last_requested = request_headers['HTTP_IF_MODIFIED_SINCE']
         except KeyError:


Test patch:
diff --git a/tests/test_whitenoise.py b/tests/test_whitenoise.py
index d186cc8..5bf70cd 100644
--- a/tests/test_whitenoise.py
+++ b/tests/test_whitenoise.py
@@ -102,6 +102,18 @@ class WhiteNoiseTest(TestCase):
         response = self.server.get(self.files.js_url, headers={'If-None-Match': etag})
         self.assertEqual(response.status_code, 200)
 
+    def test_etag_overrules_modified_since(self):
+        """
+        Browsers send both headers so it's important that the ETag takes precedence
+        over the last modified time, so that deploy-rollbacks are handled correctly.
+        """
+        headers = {
+            'If-None-Match': '"594bd1d1-36"',
+            'If-Modified-Since': 'Fri, 11 Apr 2100 11:47:06 GMT',
+        }
+        response = self.server.get(self.files.js_url, headers=headers)
+        self.assertEqual(response.status_code, 200)
+
     def test_max_age(self):
         response = self.server.get(self.files.js_url)
         self.assertEqual(response.headers['Cache-Control'], 'max-age=1000, public')