From c4c66d81bdf41d7d509ddeae5a03a2b00ff26cc8 Mon Sep 17 00:00:00 2001 From: gholt Date: Mon, 25 Feb 2013 02:25:58 +0000 Subject: [PATCH] TempURL filename options; bug fixes - Prior to this commit, a Content-Disposition header was always set on responses to GET requests, with the filename based on the object name. Now, the header will only be set for 2xx responses and the filename can be overridden with a filename query parameter on the request. - Fixed a bug where all query parameters on the request were being passed down the WSGI pipeline. Now, just the query parameters useful in log-based debugging are included. This becomes important with things like the Bulk middleware that act upon query parameters. - Fixed bug where the Content-Disposition header wasn't following RFC spec. DocImpact Change-Id: I66ad809321dcdd03444324973c8b76869e3b0c8e --- swift/common/middleware/tempurl.py | 39 +++++++++++--- test/unit/common/middleware/test_tempurl.py | 57 ++++++++++++++++++--- 2 files changed, 80 insertions(+), 16 deletions(-) diff --git a/swift/common/middleware/tempurl.py b/swift/common/middleware/tempurl.py index 2cda87756e..5a05de748f 100644 --- a/swift/common/middleware/tempurl.py +++ b/swift/common/middleware/tempurl.py @@ -69,6 +69,16 @@ locations in Swift. Note that changing the X-Account-Meta-Temp-URL-Key will invalidate any previously generated temporary URLs within 60 seconds (the memcache time for the key). + +With GET TempURLs, a Content-Disposition header will be set on the +response so that browsers will interpret this as a file attachment to +be saved. The filename chosen is based on the object name, but you +can override this with a filename query parameter. Modifying the +above example:: + + https://swift-cluster.example.com/v1/AUTH_account/container/object? + temp_url_sig=da39a3ee5e6b4b0d3255bfef95601890afd80709& + temp_url_expires=1323479485&filename=My+Test+File.pdf """ __all__ = ['TempURL', 'filter_factory', @@ -83,7 +93,7 @@ from hashlib import sha1 from os.path import basename from StringIO import StringIO from time import gmtime, strftime, time -from urllib import quote, unquote +from urllib import unquote, urlencode from urlparse import parse_qs from swift.common.wsgi import make_pre_authed_env @@ -224,7 +234,7 @@ class TempURL(object): :param start_response: The WSGI start_response hook. :returns: Response as per WSGI. """ - temp_url_sig, temp_url_expires = self._get_temp_url_info(env) + temp_url_sig, temp_url_expires, filename = self._get_temp_url_info(env) if temp_url_sig is None and temp_url_expires is None: return self.app(env, start_response) if not temp_url_sig or not temp_url_expires: @@ -251,19 +261,30 @@ class TempURL(object): env['swift.authorize'] = lambda req: None env['swift.authorize_override'] = True env['REMOTE_USER'] = '.wsgi.tempurl' + qs = {'temp_url_sig': temp_url_sig, + 'temp_url_expires': temp_url_expires} + if filename: + qs['filename'] = filename + env['QUERY_STRING'] = urlencode(qs) def _start_response(status, headers, exc_info=None): headers = self._clean_outgoing_headers(headers) - if env['REQUEST_METHOD'] == 'GET': + if env['REQUEST_METHOD'] == 'GET' and status[0] == '2': already = False for h, v in headers: if h.lower() == 'content-disposition': already = True break + if already and filename: + headers = list((h, v) for h, v in headers + if h.lower() != 'content-disposition') + already = False if not already: - headers.append( - ('Content-Disposition', 'attachment; filename=%s' % - (quote(basename(env['PATH_INFO']))))) + headers.append(( + 'Content-Disposition', + 'attachment; filename="%s"' % ( + filename or + basename(env['PATH_INFO'])).replace('"', '\\"'))) return start_response(status, headers, exc_info) return self.app(env, _start_response) @@ -298,7 +319,7 @@ class TempURL(object): :param env: The WSGI environment for the request. :returns: (sig, expires) as described above. """ - temp_url_sig = temp_url_expires = None + temp_url_sig = temp_url_expires = filename = None qs = parse_qs(env.get('QUERY_STRING', '')) if 'temp_url_sig' in qs: temp_url_sig = qs['temp_url_sig'][0] @@ -309,7 +330,9 @@ class TempURL(object): temp_url_expires = 0 if temp_url_expires < time(): temp_url_expires = 0 - return temp_url_sig, temp_url_expires + if 'filename' in qs: + filename = qs['filename'][0] + return temp_url_sig, temp_url_expires, filename def _get_key(self, env, account): """ diff --git a/test/unit/common/middleware/test_tempurl.py b/test/unit/common/middleware/test_tempurl.py index 944532c406..0711685286 100644 --- a/test/unit/common/middleware/test_tempurl.py +++ b/test/unit/common/middleware/test_tempurl.py @@ -93,6 +93,44 @@ class TestTempURL(unittest.TestCase): self.assertTrue('Temp URL invalid' not in resp.body) def test_get_valid(self): + method = 'GET' + expires = int(time() + 86400) + path = '/v1/a/c/o' + key = 'abc' + hmac_body = '%s\n%s\n%s' % (method, expires, path) + sig = hmac.new(key, hmac_body, sha1).hexdigest() + req = self._make_request(path, + environ={'QUERY_STRING': + 'temp_url_sig=%s&temp_url_expires=%s' % (sig, expires)}) + req.environ['swift.cache'].set('temp-url-key/a', key) + self.tempurl.app = FakeApp(iter([('200 Ok', (), '123')])) + resp = req.get_response(self.tempurl) + self.assertEquals(resp.status_int, 200) + self.assertEquals(resp.headers['content-disposition'], + 'attachment; filename="o"') + self.assertEquals(req.environ['swift.authorize_override'], True) + self.assertEquals(req.environ['REMOTE_USER'], '.wsgi.tempurl') + + def test_get_valid_with_filename(self): + method = 'GET' + expires = int(time() + 86400) + path = '/v1/a/c/o' + key = 'abc' + hmac_body = '%s\n%s\n%s' % (method, expires, path) + sig = hmac.new(key, hmac_body, sha1).hexdigest() + req = self._make_request(path, environ={ + 'QUERY_STRING': 'temp_url_sig=%s&temp_url_expires=%s&' + 'filename=bob%%20%%22killer%%22.txt' % (sig, expires)}) + req.environ['swift.cache'].set('temp-url-key/a', key) + self.tempurl.app = FakeApp(iter([('200 Ok', (), '123')])) + resp = req.get_response(self.tempurl) + self.assertEquals(resp.status_int, 200) + self.assertEquals(resp.headers['content-disposition'], + 'attachment; filename="bob \\\"killer\\\".txt"') + self.assertEquals(req.environ['swift.authorize_override'], True) + self.assertEquals(req.environ['REMOTE_USER'], '.wsgi.tempurl') + + def test_get_valid_but_404(self): method = 'GET' expires = int(time() + 86400) path = '/v1/a/c/o' @@ -105,8 +143,7 @@ class TestTempURL(unittest.TestCase): req.environ['swift.cache'].set('temp-url-key/a', key) resp = req.get_response(self.tempurl) self.assertEquals(resp.status_int, 404) - self.assertEquals(resp.headers['content-disposition'], - 'attachment; filename=o') + self.assertFalse('content-disposition' in resp.headers) self.assertEquals(req.environ['swift.authorize_override'], True) self.assertEquals(req.environ['REMOTE_USER'], '.wsgi.tempurl') @@ -491,17 +528,21 @@ class TestTempURL(unittest.TestCase): s = 'f5d5051bddf5df7e27c628818738334f' e = int(time() + 86400) self.assertEquals(self.tempurl._get_temp_url_info({'QUERY_STRING': - 'temp_url_sig=%s&temp_url_expires=%s' % (s, e)}), (s, e)) - self.assertEquals(self.tempurl._get_temp_url_info({}), (None, None)) + 'temp_url_sig=%s&temp_url_expires=%s' % (s, e)}), (s, e, None)) + self.assertEquals(self.tempurl._get_temp_url_info({ + 'QUERY_STRING': 'temp_url_sig=%s&temp_url_expires=%s&' + 'filename=bobisyouruncle' % (s, e)}), (s, e, 'bobisyouruncle')) + self.assertEquals(self.tempurl._get_temp_url_info({}), + (None, None, None)) self.assertEquals(self.tempurl._get_temp_url_info({'QUERY_STRING': - 'temp_url_expires=%s' % e}), (None, e)) + 'temp_url_expires=%s' % e}), (None, e, None)) self.assertEquals(self.tempurl._get_temp_url_info({'QUERY_STRING': - 'temp_url_sig=%s' % s}), (s, None)) + 'temp_url_sig=%s' % s}), (s, None, None)) self.assertEquals(self.tempurl._get_temp_url_info({'QUERY_STRING': - 'temp_url_sig=%s&temp_url_expires=bad' % s}), (s, 0)) + 'temp_url_sig=%s&temp_url_expires=bad' % s}), (s, 0, None)) e = int(time() - 1) self.assertEquals(self.tempurl._get_temp_url_info({'QUERY_STRING': - 'temp_url_sig=%s&temp_url_expires=%s' % (s, e)}), (s, 0)) + 'temp_url_sig=%s&temp_url_expires=%s' % (s, e)}), (s, 0, None)) def test_get_key_memcache(self): self.app.status_headers_body_iter = iter([('404 Not Found', {}, '')])