From f469d4214f0d60a02a2f03edf512ad22e17432d7 Mon Sep 17 00:00:00 2001 From: gholt Date: Tue, 22 Apr 2014 15:00:09 +0000 Subject: [PATCH] TempURL: Fixed bug with \r or \n in disposition. If an object had a \r or \n in its name, it would end up creating an invalid HTTP Content-Disposition header. Reviewer consensus was to use URL encoding. Fixes bug 1306250 Change-Id: Ibccaaed5152b4d09d6aee4966a1982cc0a0da07d --- swift/common/middleware/tempurl.py | 29 ++++++++++++---- test/unit/common/middleware/test_tempurl.py | 37 +++++++++++++++++++-- 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/swift/common/middleware/tempurl.py b/swift/common/middleware/tempurl.py index 57486941b5..61af1acfb7 100644 --- a/swift/common/middleware/tempurl.py +++ b/swift/common/middleware/tempurl.py @@ -1,4 +1,19 @@ -# Copyright (c) 2010-2012 OpenStack Foundation +# Copyright (c) 2011-2014 Greg Holt +# Copyright (c) 2012-2013 John Dickinson +# Copyright (c) 2012 Felipe Reyes +# Copyright (c) 2012 Peter Portante +# Copyright (c) 2012 Victor Rodionov +# Copyright (c) 2013-2014 Samuel Merritt +# Copyright (c) 2013 Chuck Thier +# Copyright (c) 2013 David Goetz +# Copyright (c) 2013 Dirk Mueller +# Copyright (c) 2013 Donagh McCabe +# Copyright (c) 2013 Fabien Boucher +# Copyright (c) 2013 Greg Lange +# Copyright (c) 2013 Kun Huang +# Copyright (c) 2013 Richard Hawkins +# Copyright (c) 2013 Tong Li +# Copyright (c) 2013 ZhiQiang Fan # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -106,7 +121,7 @@ from urlparse import parse_qs from swift.proxy.controllers.base import get_account_info from swift.common.swob import HeaderKeyDict, HTTPUnauthorized from swift.common.utils import split_path, get_valid_utf8_str, \ - register_swift_info, get_hmac, streq_const_time + register_swift_info, get_hmac, streq_const_time, quote #: Default headers to remove from incoming requests. Simply a whitespace @@ -147,6 +162,10 @@ def get_tempurl_keys_from_metadata(meta): if key.lower() in ('temp-url-key', 'temp-url-key-2')] +def disposition_format(filename): + return 'attachment; filename="%s"' % quote(filename, safe='/ ') + + class TempURL(object): """ WSGI Middleware to grant temporary URLs specific access to Swift @@ -319,14 +338,12 @@ class TempURL(object): if inline_disposition: disposition_value = 'inline' elif filename: - disposition_value = 'attachment; filename="%s"' % ( - filename.replace('"', '\\"')) + disposition_value = disposition_format(filename) elif existing_disposition: disposition_value = existing_disposition else: name = basename(env['PATH_INFO'].rstrip('/')) - disposition_value = 'attachment; filename="%s"' % ( - name.replace('"', '\\"')) + disposition_value = disposition_format(name) out_headers.append(('Content-Disposition', disposition_value)) headers = out_headers return start_response(status, headers, exc_info) diff --git a/test/unit/common/middleware/test_tempurl.py b/test/unit/common/middleware/test_tempurl.py index 157186aef4..f940434177 100644 --- a/test/unit/common/middleware/test_tempurl.py +++ b/test/unit/common/middleware/test_tempurl.py @@ -1,4 +1,19 @@ -# Copyright (c) 2011 OpenStack Foundation +# Copyright (c) 2011-2014 Greg Holt +# Copyright (c) 2012-2013 Peter Portante +# Copyright (c) 2012 Iryoung Jeong +# Copyright (c) 2012 Michael Barton +# Copyright (c) 2013 Alex Gaynor +# Copyright (c) 2013 Chuck Thier +# Copyright (c) 2013 David Goetz +# Copyright (c) 2013 Donagh McCabe +# Copyright (c) 2013 Greg Lange +# Copyright (c) 2013 John Dickinson +# Copyright (c) 2013 Kun Huang +# Copyright (c) 2013 Richard Hawkins +# Copyright (c) 2013 Samuel Merritt +# Copyright (c) 2013 Shri Javadekar +# Copyright (c) 2013 Tong Li +# Copyright (c) 2013 ZhiQiang Fan # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -142,7 +157,7 @@ class TestTempURL(unittest.TestCase): resp = req.get_response(self.tempurl) self.assertEquals(resp.status_int, 200) self.assertEquals(resp.headers['content-disposition'], - 'attachment; filename="bob \\\"killer\\\".txt"') + 'attachment; filename="bob %22killer%22.txt"') self.assertEquals(req.environ['swift.authorize_override'], True) self.assertEquals(req.environ['REMOTE_USER'], '.wsgi.tempurl') @@ -195,6 +210,24 @@ class TestTempURL(unittest.TestCase): self.assertEquals(req.environ['swift.authorize_override'], True) self.assertEquals(req.environ['REMOTE_USER'], '.wsgi.tempurl') + def test_obj_odd_chars(self): + method = 'GET' + expires = int(time() + 86400) + path = '/v1/a/c/a\r\nb' + 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, keys=[key], environ={ + 'QUERY_STRING': 'temp_url_sig=%s&temp_url_expires=%s' % ( + sig, expires)}) + 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="a%0D%0Ab"') + self.assertEquals(req.environ['swift.authorize_override'], True) + self.assertEquals(req.environ['REMOTE_USER'], '.wsgi.tempurl') + def test_obj_trailing_slash(self): method = 'GET' expires = int(time() + 86400)