openstack/swift
Code Issues Proposed changes

FormPost logging bugfix and slight refactor

Browse Source 
In the interest of getting us to pep8 1.3.1 compliance I also fixed
all violations in the two files I was modifying anyway.

Fixes bug 1019051

Change-Id: I52eb7d59d2e2810a8cee2461f7ca869124e734e7
This commit is contained in:
gholt 2012-06-28 20:27:15 +00:00
parent 329b1da07b
commit d8c2d0e1bc
2 changed files with 125 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
swift/common/middleware/formpost.py
View File

@ -107,7 +107,6 @@ import rfc822
from hashlib import sha1 from hashlib import sha1
from StringIO import StringIO from StringIO import StringIO
from time import gmtime, strftime, time from time import gmtime, strftime, time
from time import time
from urllib import quote, unquote from urllib import quote, unquote
from swift.common.utils import get_logger, streq_const_time from swift.common.utils import get_logger, streq_const_time
@ -315,32 +314,29 @@ class FormPost(object):
_parse_attrs(env.get('CONTENT_TYPE') or '') _parse_attrs(env.get('CONTENT_TYPE') or '')
if content_type == 'multipart/form-data' and \ if content_type == 'multipart/form-data' and \
'boundary' in attrs: 'boundary' in attrs:
resp_status = [0] status, headers, body = self._translate_form(
env, attrs['boundary'])
def _start_response(status, headers, exc_info=None): self._log_request(env, int(status.split(' ', 1)[0]))
resp_status[0] = int(status.split(' ', 1)[0]) start_response(status, headers)
start_response(status, headers, exc_info) return body
self._log_request(env, resp_status)
return self._translate_form(env, start_response,
attrs['boundary'])
except (FormInvalid, EOFError), err: except (FormInvalid, EOFError), err:
self._log_request(env, HTTP_BAD_REQUEST) self._log_request(env, HTTP_BAD_REQUEST)
body = 'FormPost: %s' % err body = 'FormPost: %s' % err
start_response('400 Bad Request', start_response(
'400 Bad Request',
(('Content-Type', 'text/plain'), (('Content-Type', 'text/plain'),
('Content-Length', str(len(body))))) ('Content-Length', str(len(body)))))
return [body] return [body]
return self.app(env, start_response) return self.app(env, start_response)
def _translate_form(self, env, start_response, boundary): def _translate_form(self, env, boundary):
""" """
Translates the form data into subrequests and issues a Translates the form data into subrequests and issues a
response. response.
:param env: The WSGI environment dict. :param env: The WSGI environment dict.
:param start_response: The WSGI start_response hook. :param boundary: The MIME type boundary to look for.
:returns: Response as per WSGI. :returns: status_line, headers_list, body
""" """
key = self._get_key(env) key = self._get_key(env)
status = message = '' status = message = ''
@ -363,8 +359,8 @@ class FormPost(object):
if 'content-type' not in attributes and 'content-type' in hdrs: if 'content-type' not in attributes and 'content-type' in hdrs:
attributes['content-type'] = \ attributes['content-type'] = \
hdrs['Content-Type'] or 'application/octet-stream' hdrs['Content-Type'] or 'application/octet-stream'
status, message = self._perform_subrequest(env, start_response, status, message = self._perform_subrequest(env, attributes, fp,
attributes, fp, key) key)
if status[:1] != '2': if status[:1] != '2':
break break
else: else:
@ -387,29 +383,29 @@ class FormPost(object):
body = status body = status
if message: if message:
body = status + '\r\nFormPost: ' + message.title() body = status + '\r\nFormPost: ' + message.title()
start_response(status, [('Content-Type', 'text/plain'), headers = [('Content-Type', 'text/plain'),
('Content-Length', len(body))]) ('Content-Length', len(body))]
return [body] return status, headers, body
status = status.split(' ', 1)[0] status = status.split(' ', 1)[0]
body = '<html><body><p><a href="%s?status=%s&message=%s">Click to ' \ body = '<html><body><p><a href="%s?status=%s&message=%s">Click to ' \
'continue...</a></p></body></html>' % \ 'continue...</a></p></body></html>' % \
(attributes['redirect'], quote(status), quote(message)) (attributes['redirect'], quote(status), quote(message))
start_response('303 See Other', headers = [
[('Location', '%s?status=%s&message=%s' % ('Location', '%s?status=%s&message=%s' % (
(attributes['redirect'], quote(status), quote(message))), attributes['redirect'], quote(status), quote(message))),
('Content-Length', str(len(body)))]) ('Content-Length', str(len(body)))]
return [body] return '303 See Other', headers, body
def _perform_subrequest(self, env, start_response, attributes, fp, key): def _perform_subrequest(self, orig_env, attributes, fp, key):
""" """
Performs the subrequest and returns a new response. Performs the subrequest and returns the response.
:param env: The WSGI environment dict. :param orig_env: The WSGI environment dict; will only be used
:param start_response: The WSGI start_response hook. to form a new env for the subrequest.
:param attributes: dict of the attributes of the form so far. :param attributes: dict of the attributes of the form so far.
:param fp: The file-like object containing the request body. :param fp: The file-like object containing the request body.
:param key: The account key to validate the signature with. :param key: The account key to validate the signature with.
:returns: Response as per WSGI. :returns: (status_line, message)
""" """
if not key: if not key:
return '401 Unauthorized', 'invalid signature' return '401 Unauthorized', 'invalid signature'
@ -417,7 +413,7 @@ class FormPost(object):
max_file_size = int(attributes.get('max_file_size') or 0) max_file_size = int(attributes.get('max_file_size') or 0)
except ValueError: except ValueError:
raise FormInvalid('max_file_size not an integer') raise FormInvalid('max_file_size not an integer')
subenv = make_pre_authed_env(env, 'PUT', agent=self.agent) subenv = make_pre_authed_env(orig_env, 'PUT', agent=self.agent)
subenv['HTTP_TRANSFER_ENCODING'] = 'chunked' subenv['HTTP_TRANSFER_ENCODING'] = 'chunked'
subenv['wsgi.input'] = _CappedFileLikeObject(fp, max_file_size) subenv['wsgi.input'] = _CappedFileLikeObject(fp, max_file_size)
if subenv['PATH_INFO'][-1] != '/' and \ if subenv['PATH_INFO'][-1] != '/' and \
@ -435,12 +431,11 @@ class FormPost(object):
except ValueError: except ValueError:
raise FormInvalid('expired not an integer') raise FormInvalid('expired not an integer')
hmac_body = '%s\n%s\n%s\n%s\n%s' % ( hmac_body = '%s\n%s\n%s\n%s\n%s' % (
env['PATH_INFO'], orig_env['PATH_INFO'],
attributes.get('redirect') or '', attributes.get('redirect') or '',
attributes.get('max_file_size') or '0', attributes.get('max_file_size') or '0',
attributes.get('max_file_count') or '0', attributes.get('max_file_count') or '0',
attributes.get('expires') or '0' attributes.get('expires') or '0')
)
sig = hmac.new(key, hmac_body, sha1).hexdigest() sig = hmac.new(key, hmac_body, sha1).hexdigest()
if not streq_const_time(sig, (attributes.get('signature') or if not streq_const_time(sig, (attributes.get('signature') or
'invalid')): 'invalid')):

97
test/unit/common/middleware/test_formpost.py
View File

@ -75,8 +75,9 @@ class FakeApp(object):
self.requests.append(Request.blank('', environ=env)) self.requests.append(Request.blank('', environ=env))
if env.get('swift.authorize_override') and \ if env.get('swift.authorize_override') and \
env.get('REMOTE_USER') != '.wsgi.pre_authed': env.get('REMOTE_USER') != '.wsgi.pre_authed':
raise Exception('Invalid REMOTE_USER %r with ' raise Exception(
'swift.authorize_override' % (env.get('REMOTE_USER'),)) 'Invalid REMOTE_USER %r with swift.authorize_override' % (
env.get('REMOTE_USER'),))
if 'swift.authorize' in env: if 'swift.authorize' in env:
resp = env['swift.authorize'](self.requests[-1]) resp = env['swift.authorize'](self.requests[-1])
if resp: if resp:
@ -204,8 +205,9 @@ class TestIterRequests(unittest.TestCase):
self.assertTrue(exc is not None) self.assertTrue(exc is not None)
def test_readline(self): def test_readline(self):
it = formpost._iter_requests(StringIO('--unique\r\nab\r\ncd\ref\ng\r\n' it = formpost._iter_requests(
'--unique\r\nhi\r\n\r\njkl\r\n\r\n--unique--'), 'unique') StringIO('--unique\r\nab\r\ncd\ref\ng\r\n--unique\r\nhi\r\n\r\n'
'jkl\r\n\r\n--unique--'), 'unique')
fp = it.next() fp = it.next()
self.assertEquals(fp.readline(), 'ab\r\n') self.assertEquals(fp.readline(), 'ab\r\n')
self.assertEquals(fp.readline(), 'cd\ref\ng') self.assertEquals(fp.readline(), 'cd\ref\ng')
@ -225,8 +227,9 @@ class TestIterRequests(unittest.TestCase):
orig_read_chunk_size = formpost.READ_CHUNK_SIZE orig_read_chunk_size = formpost.READ_CHUNK_SIZE
try: try:
formpost.READ_CHUNK_SIZE = 2 formpost.READ_CHUNK_SIZE = 2
it = formpost._iter_requests(StringIO('--unique\r\nab\r\ncd\ref\ng' it = formpost._iter_requests(
'\r\n--unique\r\nhi\r\n\r\njkl\r\n\r\n--unique--'), 'unique') StringIO('--unique\r\nab\r\ncd\ref\ng\r\n--unique\r\nhi\r\n'
'\r\njkl\r\n\r\n--unique--'), 'unique')
fp = it.next() fp = it.next()
self.assertEquals(fp.readline(), 'ab\r\n') self.assertEquals(fp.readline(), 'ab\r\n')
self.assertEquals(fp.readline(), 'cd\ref\ng') self.assertEquals(fp.readline(), 'cd\ref\ng')
@ -298,8 +301,11 @@ class TestFormPost(unittest.TestCase):
def _make_sig_env_body(self, path, redirect, max_file_size, max_file_count, def _make_sig_env_body(self, path, redirect, max_file_size, max_file_count,
expires, key): expires, key):
sig = hmac.new(key, '%s\n%s\n%s\n%s\n%s' % (path, redirect, sig = hmac.new(
max_file_size, max_file_count, expires), sha1).hexdigest() key,
'%s\n%s\n%s\n%s\n%s' % (
path, redirect, max_file_size, max_file_count, expires),
sha1).hexdigest()
body = [ body = [
'------WebKitFormBoundaryNcxTqxSlX7t4TDkR', '------WebKitFormBoundaryNcxTqxSlX7t4TDkR',
'Content-Disposition: form-data; name="redirect"', 'Content-Disposition: form-data; name="redirect"',
@ -373,7 +379,8 @@ class TestFormPost(unittest.TestCase):
def test_passthrough(self): def test_passthrough(self):
for method in ('HEAD', 'GET', 'PUT', 'POST', 'DELETE'): for method in ('HEAD', 'GET', 'PUT', 'POST', 'DELETE'):
resp = self._make_request('/v1/a/c/o', resp = self._make_request(
'/v1/a/c/o',
environ={'REQUEST_METHOD': method}).get_response(self.formpost) environ={'REQUEST_METHOD': method}).get_response(self.formpost)
self.assertEquals(resp.status_int, 401) self.assertEquals(resp.status_int, 401)
self.assertTrue('FormPost' not in resp.body) self.assertTrue('FormPost' not in resp.body)
@ -385,8 +392,11 @@ class TestFormPost(unittest.TestCase):
max_file_size = 1024 max_file_size = 1024
max_file_count = 10 max_file_count = 10
expires = int(time() + 86400) expires = int(time() + 86400)
sig = hmac.new(key, '%s\n%s\n%s\n%s\n%s' % (path, redirect, sig = hmac.new(
max_file_size, max_file_count, expires), sha1).hexdigest() key,
'%s\n%s\n%s\n%s\n%s' % (
path, redirect, max_file_size, max_file_count, expires),
sha1).hexdigest()
memcache = FakeMemcache() memcache = FakeMemcache()
memcache.set('temp-url-key/AUTH_test', key) memcache.set('temp-url-key/AUTH_test', key)
wsgi_input = StringIO('\r\n'.join([ wsgi_input = StringIO('\r\n'.join([
@ -496,8 +506,11 @@ class TestFormPost(unittest.TestCase):
max_file_size = 1024 max_file_size = 1024
max_file_count = 10 max_file_count = 10
expires = int(time() + 86400) expires = int(time() + 86400)
sig = hmac.new(key, '%s\n%s\n%s\n%s\n%s' % (path, redirect, sig = hmac.new(
max_file_size, max_file_count, expires), sha1).hexdigest() key,
'%s\n%s\n%s\n%s\n%s' % (
path, redirect, max_file_size, max_file_count, expires),
sha1).hexdigest()
memcache = FakeMemcache() memcache = FakeMemcache()
memcache.set('temp-url-key/AUTH_test', key) memcache.set('temp-url-key/AUTH_test', key)
wsgi_input = StringIO('\r\n'.join([ wsgi_input = StringIO('\r\n'.join([
@ -606,8 +619,11 @@ class TestFormPost(unittest.TestCase):
max_file_size = 1024 max_file_size = 1024
max_file_count = 10 max_file_count = 10
expires = int(time() + 86400) expires = int(time() + 86400)
sig = hmac.new(key, '%s\n%s\n%s\n%s\n%s' % (path, redirect, sig = hmac.new(
max_file_size, max_file_count, expires), sha1).hexdigest() key,
'%s\n%s\n%s\n%s\n%s' % (
path, redirect, max_file_size, max_file_count, expires),
sha1).hexdigest()
memcache = FakeMemcache() memcache = FakeMemcache()
memcache.set('temp-url-key/AUTH_test', key) memcache.set('temp-url-key/AUTH_test', key)
wsgi_input = StringIO('\r\n'.join([ wsgi_input = StringIO('\r\n'.join([
@ -719,8 +735,11 @@ class TestFormPost(unittest.TestCase):
max_file_size = 1024 max_file_size = 1024
max_file_count = 10 max_file_count = 10
expires = int(time() + 86400) expires = int(time() + 86400)
sig = hmac.new(key, '%s\n%s\n%s\n%s\n%s' % (path, redirect, sig = hmac.new(
max_file_size, max_file_count, expires), sha1).hexdigest() key,
'%s\n%s\n%s\n%s\n%s' % (
path, redirect, max_file_size, max_file_count, expires),
sha1).hexdigest()
memcache = FakeMemcache() memcache = FakeMemcache()
memcache.set('temp-url-key/AUTH_test', key) memcache.set('temp-url-key/AUTH_test', key)
wsgi_input = StringIO('\r\n'.join([ wsgi_input = StringIO('\r\n'.join([
@ -823,8 +842,9 @@ class TestFormPost(unittest.TestCase):
def test_messed_up_start(self): def test_messed_up_start(self):
key = 'abc' key = 'abc'
sig, env, body = self._make_sig_env_body('/v1/AUTH_test/container', sig, env, body = self._make_sig_env_body(
'http://brim.net', 5, 10, int(time() + 86400), key) '/v1/AUTH_test/container', 'http://brim.net', 5, 10,
int(time() + 86400), key)
env['wsgi.input'] = StringIO('XX' + '\r\n'.join(body)) env['wsgi.input'] = StringIO('XX' + '\r\n'.join(body))
env['swift.cache'] = FakeMemcache() env['swift.cache'] = FakeMemcache()
env['swift.cache'].set('temp-url-key/AUTH_test', key) env['swift.cache'].set('temp-url-key/AUTH_test', key)
@ -832,6 +852,11 @@ class TestFormPost(unittest.TestCase):
('201 Created', {}, '')])) ('201 Created', {}, '')]))
self.auth = tempauth.filter_factory({})(self.app) self.auth = tempauth.filter_factory({})(self.app)
self.formpost = formpost.filter_factory({})(self.auth) self.formpost = formpost.filter_factory({})(self.auth)
def log_assert_int_status(env, response_status_int):
self.assertTrue(isinstance(response_status_int, int))
self.formpost._log_request = log_assert_int_status
status = [None] status = [None]
headers = [None] headers = [None]
exc_info = [None] exc_info = [None]
@ -852,8 +877,9 @@ class TestFormPost(unittest.TestCase):
def test_max_file_size_exceeded(self): def test_max_file_size_exceeded(self):
key = 'abc' key = 'abc'
sig, env, body = self._make_sig_env_body('/v1/AUTH_test/container', sig, env, body = self._make_sig_env_body(
'http://brim.net', 5, 10, int(time() + 86400), key) '/v1/AUTH_test/container', 'http://brim.net', 5, 10,
int(time() + 86400), key)
env['wsgi.input'] = StringIO('\r\n'.join(body)) env['wsgi.input'] = StringIO('\r\n'.join(body))
env['swift.cache'] = FakeMemcache() env['swift.cache'] = FakeMemcache()
env['swift.cache'].set('temp-url-key/AUTH_test', key) env['swift.cache'].set('temp-url-key/AUTH_test', key)
@ -881,8 +907,9 @@ class TestFormPost(unittest.TestCase):
def test_max_file_count_exceeded(self): def test_max_file_count_exceeded(self):
key = 'abc' key = 'abc'
sig, env, body = self._make_sig_env_body('/v1/AUTH_test/container', sig, env, body = self._make_sig_env_body(
'http://brim.net', 1024, 1, int(time() + 86400), key) '/v1/AUTH_test/container', 'http://brim.net', 1024, 1,
int(time() + 86400), key)
env['wsgi.input'] = StringIO('\r\n'.join(body)) env['wsgi.input'] = StringIO('\r\n'.join(body))
env['swift.cache'] = FakeMemcache() env['swift.cache'] = FakeMemcache()
env['swift.cache'].set('temp-url-key/AUTH_test', key) env['swift.cache'].set('temp-url-key/AUTH_test', key)
@ -908,7 +935,8 @@ class TestFormPost(unittest.TestCase):
for h, v in headers: for h, v in headers:
if h.lower() == 'location': if h.lower() == 'location':
location = v location = v
self.assertEquals(location, self.assertEquals(
location,
'http://brim.net?status=400&message=max%20file%20count%20exceeded') 'http://brim.net?status=400&message=max%20file%20count%20exceeded')
self.assertEquals(exc_info, None) self.assertEquals(exc_info, None)
self.assertTrue( self.assertTrue(
@ -919,8 +947,9 @@ class TestFormPost(unittest.TestCase):
def test_subrequest_fails(self): def test_subrequest_fails(self):
key = 'abc' key = 'abc'
sig, env, body = self._make_sig_env_body('/v1/AUTH_test/container', sig, env, body = self._make_sig_env_body(
'http://brim.net', 1024, 10, int(time() + 86400), key) '/v1/AUTH_test/container', 'http://brim.net', 1024, 10,
int(time() + 86400), key)
env['wsgi.input'] = StringIO('\r\n'.join(body)) env['wsgi.input'] = StringIO('\r\n'.join(body))
env['swift.cache'] = FakeMemcache() env['swift.cache'] = FakeMemcache()
env['swift.cache'].set('temp-url-key/AUTH_test', key) env['swift.cache'].set('temp-url-key/AUTH_test', key)
@ -957,8 +986,9 @@ class TestFormPost(unittest.TestCase):
max_file_size = 1024 max_file_size = 1024
max_file_count = 10 max_file_count = 10
expires = int(time() + 86400) expires = int(time() + 86400)
sig, env, body = self._make_sig_env_body('/v1/AUTH_test/container', sig, env, body = self._make_sig_env_body(
redirect, max_file_size, max_file_count, expires, key) '/v1/AUTH_test/container', redirect, max_file_size, max_file_count,
expires, key)
# Tack on an extra char to redirect, but shouldn't matter since it # Tack on an extra char to redirect, but shouldn't matter since it
# should get truncated off on read. # should get truncated off on read.
redirect += 'b' redirect += 'b'
@ -1027,7 +1057,8 @@ class TestFormPost(unittest.TestCase):
for h, v in headers: for h, v in headers:
if h.lower() == 'location': if h.lower() == 'location':
location = v location = v
self.assertEquals(location, self.assertEquals(
location,
('a' * formpost.MAX_VALUE_LENGTH) + '?status=201&message=') ('a' * formpost.MAX_VALUE_LENGTH) + '?status=201&message=')
self.assertEquals(exc_info, None) self.assertEquals(exc_info, None)
self.assertTrue( self.assertTrue(
@ -1042,8 +1073,9 @@ class TestFormPost(unittest.TestCase):
max_file_size = 1024 max_file_size = 1024
max_file_count = 10 max_file_count = 10
expires = int(time() + 86400) expires = int(time() + 86400)
sig, env, body = self._make_sig_env_body('/v1/AUTH_test/container', sig, env, body = self._make_sig_env_body(
redirect, max_file_size, max_file_count, expires, key) '/v1/AUTH_test/container', redirect, max_file_size, max_file_count,
expires, key)
env['wsgi.input'] = StringIO('\r\n'.join([ env['wsgi.input'] = StringIO('\r\n'.join([
'------WebKitFormBoundaryNcxTqxSlX7t4TDkR', '------WebKitFormBoundaryNcxTqxSlX7t4TDkR',
'Content-Disposition: form-data; name="redirect"', 'Content-Disposition: form-data; name="redirect"',
@ -1092,7 +1124,8 @@ class TestFormPost(unittest.TestCase):
for h, v in headers: for h, v in headers:
if h.lower() == 'location': if h.lower() == 'location':
location = v location = v
self.assertEquals(location, self.assertEquals(
location,
'http://brim.net?status=400&message=no%20files%20to%20process') 'http://brim.net?status=400&message=no%20files%20to%20process')
self.assertEquals(exc_info, None) self.assertEquals(exc_info, None)
self.assertTrue( self.assertTrue(