Merge "Add Expires header for successful GETs using tempurls"

swift/common/middleware/tempurl.py

@@ -163,7 +163,7 @@ __all__ = ['TempURL', 'filter_factory',
 
 
 from os.path import basename
-from time import time
+from time import time, strftime, gmtime
 
 from six.moves.urllib.parse import parse_qs
 from six.moves.urllib.parse import urlencode
@@ -425,6 +425,11 @@ class TempURL(object):
                 # newline into existing_disposition
                 value = disposition_value.replace('\n', '%0A')
                 out_headers.append(('Content-Disposition', value))
+
+                # include Expires header for better cache-control
+                out_headers.append(('Expires', strftime(
+                    "%a, %d %b %Y %H:%M:%S GMT",
+                    gmtime(temp_url_expires))))
                 headers = out_headers
             return start_response(status, headers, exc_info)
 

test/unit/common/middleware/test_tempurl.py

@@ -30,9 +30,10 @@
 
 import hmac
 import itertools
+import mock
 import unittest
 from hashlib import sha1
-from time import time
+from time import time, strftime, gmtime
 
 from swift.common.middleware import tempauth, tempurl
 from swift.common.header_key_dict import HeaderKeyDict
@@ -135,6 +136,9 @@ class TestTempURL(unittest.TestCase):
         self.assertEqual(resp.status_int, 200)
         self.assertEqual(resp.headers['content-disposition'],
                          'attachment; filename="o"; ' + "filename*=UTF-8''o")
+        self.assertEqual(resp.headers['expires'],
+                         strftime('%a, %d %b %Y %H:%M:%S GMT',
+                                  gmtime(expires)))
         self.assertEqual(req.environ['swift.authorize_override'], True)
         self.assertEqual(req.environ['REMOTE_USER'], '.wsgi.tempurl')
 
@@ -183,9 +187,10 @@ class TestTempURL(unittest.TestCase):
         for sig in (sig1, sig2):
             self.assert_valid_sig(expires, path, account_keys, sig, environ)
 
-    def test_get_valid_with_filename(self):
+    @mock.patch('swift.common.middleware.tempurl.time', return_value=0)
+    def test_get_valid_with_filename(self, mock_time):
         method = 'GET'
-        expires = int(time() + 86400)
+        expires = (((24 + 1) * 60 + 1) * 60) + 1
         path = '/v1/a/c/o'
         key = 'abc'
         hmac_body = '%s\n%s\n%s' % (method, expires, path)
@@ -199,6 +204,9 @@ class TestTempURL(unittest.TestCase):
         self.assertEqual(resp.headers['content-disposition'],
                          'attachment; filename="bob %22killer%22.txt"; ' +
                          "filename*=UTF-8''bob%20%22killer%22.txt")
+        self.assertIn('expires', resp.headers)
+        self.assertEqual('Fri, 02 Jan 1970 01:01:01 GMT',
+                         resp.headers['expires'])
         self.assertEqual(req.environ['swift.authorize_override'], True)
         self.assertEqual(req.environ['REMOTE_USER'], '.wsgi.tempurl')
 
@@ -245,9 +253,10 @@ class TestTempURL(unittest.TestCase):
         get_resp = get_req.get_response(self.tempurl)
         self.assertEqual(resp.headers, get_resp.headers)
 
-    def test_get_valid_with_filename_and_inline(self):
+    @mock.patch('swift.common.middleware.tempurl.time', return_value=0)
+    def test_get_valid_with_filename_and_inline(self, mock_time):
         method = 'GET'
-        expires = int(time() + 86400)
+        expires = 1
         path = '/v1/a/c/o'
         key = 'abc'
         hmac_body = '%s\n%s\n%s' % (method, expires, path)
@@ -259,6 +268,9 @@ class TestTempURL(unittest.TestCase):
         resp = req.get_response(self.tempurl)
         self.assertEqual(resp.status_int, 200)
         self.assertEqual(resp.headers['content-disposition'], 'inline')
+        self.assertIn('expires', resp.headers)
+        self.assertEqual('Thu, 01 Jan 1970 00:00:01 GMT',
+                         resp.headers['expires'])
         self.assertEqual(req.environ['swift.authorize_override'], True)
         self.assertEqual(req.environ['REMOTE_USER'], '.wsgi.tempurl')
 
@@ -276,6 +288,7 @@ class TestTempURL(unittest.TestCase):
         resp = req.get_response(self.tempurl)
         self.assertEqual(resp.status_int, 200)
         self.assertEqual(resp.headers['content-disposition'], 'inline')
+        self.assertIn('expires', resp.headers)
         self.assertEqual(req.environ['swift.authorize_override'], True)
         self.assertEqual(req.environ['REMOTE_USER'], '.wsgi.tempurl')
 
@@ -295,6 +308,7 @@ class TestTempURL(unittest.TestCase):
         self.assertEqual(resp.headers['content-disposition'],
                          'attachment; filename="a%0D%0Ab"; ' +
                          "filename*=UTF-8''a%0D%0Ab")
+        self.assertIn('expires', resp.headers)
         self.assertEqual(req.environ['swift.authorize_override'], True)
         self.assertEqual(req.environ['REMOTE_USER'], '.wsgi.tempurl')
 
@@ -314,6 +328,7 @@ class TestTempURL(unittest.TestCase):
         self.assertEqual(resp.status_int, 200)
         self.assertEqual(resp.headers['content-disposition'],
                          'attachment; filename="fu%0Abar"')
+        self.assertIn('expires', resp.headers)
         self.assertEqual(req.environ['swift.authorize_override'], True)
         self.assertEqual(req.environ['REMOTE_USER'], '.wsgi.tempurl')
 
@@ -333,6 +348,7 @@ class TestTempURL(unittest.TestCase):
         self.assertEqual(resp.headers['content-disposition'],
                          'attachment; filename="o"; ' +
                          "filename*=UTF-8''o")
+        self.assertIn('expires', resp.headers)
         self.assertEqual(req.environ['swift.authorize_override'], True)
         self.assertEqual(req.environ['REMOTE_USER'], '.wsgi.tempurl')
 
@@ -353,6 +369,7 @@ class TestTempURL(unittest.TestCase):
             resp.headers['content-disposition'],
             'attachment; filename="/i/want/this/just/as/it/is/"; ' +
             "filename*=UTF-8''/i/want/this/just/as/it/is/")
+        self.assertIn('expires', resp.headers)
         self.assertEqual(req.environ['swift.authorize_override'], True)
         self.assertEqual(req.environ['REMOTE_USER'], '.wsgi.tempurl')
 
@@ -369,7 +386,8 @@ class TestTempURL(unittest.TestCase):
                 sig, expires)})
         resp = req.get_response(self.tempurl)
         self.assertEqual(resp.status_int, 404)
-        self.assertFalse('content-disposition' in resp.headers)
+        self.assertNotIn('content-disposition', resp.headers)
+        self.assertNotIn('expires', resp.headers)
         self.assertEqual(req.environ['swift.authorize_override'], True)
         self.assertEqual(req.environ['REMOTE_USER'], '.wsgi.tempurl')