Skip to content

Commit

Permalink
Fixed formpost QUERY_STRING bugs.
Browse files Browse the repository at this point in the history 
Ensures that any QUERY_STRING to FormPost is not passed onward.
Handles a redirect with a query string properly.

Change-Id: If0a7d9b0a17314c6cd3852175362d4633f828d81
  • Loading branch information
@gholt
gholt committed Feb 19, 2013
1 parent e88ff34 commit ffada71
Show file tree
Hide file tree
Showing 2 changed files with 131 additions and 9 deletions.
22 changes: 14 additions & 8 deletions swift/common/middleware/formpost.py
View file
Expand Up @@ -377,21 +377,23 @@ def _translate_form(self, env, boundary):
if not status:
status = '400 Bad Request'
message = 'no files to process'
if not attributes.get('redirect'):
redirect = attributes.get('redirect')
if not redirect:
body = status
if message:
body = status + '\r\nFormPost: ' + message.title()
headers = [('Content-Type', 'text/plain'),
('Content-Length', len(body))]
return status, headers, body
status = status.split(' ', 1)[0]
body = '<html><body><p><a href="%s?status=%s&message=%s">Click to ' \
'continue...</a></p></body></html>' % \
(attributes['redirect'], quote(status), quote(message))
headers = [
('Location', '%s?status=%s&message=%s' % (
attributes['redirect'], quote(status), quote(message))),
('Content-Length', str(len(body)))]
if '?' in redirect:
redirect += '&'
else:
redirect += '?'
redirect += 'status=%s&message=%s' % (quote(status), quote(message))
body = '<html><body><p><a href="%s">' \
'Click to continue...</a></p></body></html>' % redirect
headers = [('Location', redirect), ('Content-Length', str(len(body)))]
return '303 See Other', headers, body

def _perform_subrequest(self, orig_env, attributes, fp, key):
Expand All @@ -413,6 +415,8 @@ def _perform_subrequest(self, orig_env, attributes, fp, key):
raise FormInvalid('max_file_size not an integer')
subenv = make_pre_authed_env(orig_env, 'PUT', agent=None,
swift_source='FP')
if 'QUERY_STRING' in subenv:
del subenv['QUERY_STRING']
subenv['HTTP_TRANSFER_ENCODING'] = 'chunked'
subenv['wsgi.input'] = _CappedFileLikeObject(fp, max_file_size)
if subenv['PATH_INFO'][-1] != '/' and \
Expand Down Expand Up @@ -471,6 +475,8 @@ def _get_key(self, env):
if not key:
newenv = make_pre_authed_env(env, 'HEAD', '/v1/' + account,
agent=None, swift_source='FP')
if 'QUERY_STRING' in newenv:
del newenv['QUERY_STRING']
newenv['CONTENT_LENGTH'] = '0'
newenv['wsgi.input'] = StringIO('')
key = [None]
Expand Down
118 changes: 117 additions & 1 deletion test/unit/common/middleware/test_formpost.py
View file
Expand Up @@ -54,16 +54,21 @@ def delete(self, key):

class FakeApp(object):

def __init__(self, status_headers_body_iter=None):
def __init__(self, status_headers_body_iter=None,
check_no_query_string=True):
self.status_headers_body_iter = status_headers_body_iter
if not self.status_headers_body_iter:
self.status_headers_body_iter = iter([('404 Not Found', {
'x-test-header-one-a': 'value1',
'x-test-header-two-a': 'value2',
'x-test-header-two-b': 'value3'}, '')])
self.requests = []
self.check_no_query_string = check_no_query_string

def __call__(self, env, start_response):
if self.check_no_query_string and env.get('QUERY_STRING'):
raise Exception('Query string %s should have been discarded!' %
env['QUERY_STRING'])
body = ''
while True:
chunk = env['wsgi.input'].read()
Expand Down Expand Up @@ -944,6 +949,42 @@ def start_response(s, h, e=None):
self.assertEquals(len(self.app.requests), 1)
self.assertEquals(self.app.requests[0].body, 'Test File\nOne\n')

def test_subrequest_does_not_pass_query(self):
key = 'abc'
sig, env, body = self._make_sig_env_body(
'/v1/AUTH_test/container', '', 1024, 10, int(time() + 86400), key)
env['QUERY_STRING'] = 'this=should&not=get&passed'
env['wsgi.input'] = StringIO('\r\n'.join(body))
env['swift.cache'] = FakeMemcache()
# We don't cache the key so that it's asked for (and FakeApp verifies
# that no QUERY_STRING got passed).
self.app = FakeApp(
iter([('200 Ok', {'x-account-meta-temp-url-key': 'abc'}, ''),
('201 Created', {}, ''),
('201 Created', {}, '')]),
check_no_query_string=True)
self.auth = tempauth.filter_factory({})(self.app)
self.formpost = formpost.filter_factory({})(self.auth)
status = [None]
headers = [None]
exc_info = [None]

def start_response(s, h, e=None):
status[0] = s
headers[0] = h
exc_info[0] = e

body = ''.join(self.formpost(env, start_response))
status = status[0]
headers = headers[0]
exc_info = exc_info[0]
# Make sure we 201 Created, which means we made the final subrequest
# (and FakeAp verifies that no QUERY_STRING got passed).
self.assertEquals(status, '201 Created')
self.assertEquals(exc_info, None)
self.assertTrue('201 Created' in body)
self.assertEquals(len(self.app.requests), 3)

def test_subrequest_fails(self):
key = 'abc'
sig, env, body = self._make_sig_env_body(
Expand Down Expand Up @@ -1132,6 +1173,81 @@ def start_response(s, h, e=None):
in body)
self.assertEquals(len(self.app.requests), 0)

def test_redirect(self):
key = 'abc'
sig, env, body = self._make_sig_env_body(
'/v1/AUTH_test/container', 'http://redirect', 1024, 10,
int(time() + 86400), key)
env['wsgi.input'] = StringIO('\r\n'.join(body))
env['swift.cache'] = FakeMemcache()
env['swift.cache'].set('temp-url-key/AUTH_test', key)
self.app = FakeApp(iter([('201 Created', {}, ''),
('201 Created', {}, '')]))
self.auth = tempauth.filter_factory({})(self.app)
self.formpost = formpost.filter_factory({})(self.auth)
status = [None]
headers = [None]
exc_info = [None]

def start_response(s, h, e=None):
status[0] = s
headers[0] = h
exc_info[0] = e

body = ''.join(self.formpost(env, start_response))
status = status[0]
headers = headers[0]
exc_info = exc_info[0]
self.assertEquals(status, '303 See Other')
location = None
for h, v in headers:
if h.lower() == 'location':
location = v
self.assertEquals(location, 'http://redirect?status=201&message=')
self.assertEquals(exc_info, None)
self.assertTrue(location in body)
self.assertEquals(len(self.app.requests), 2)
self.assertEquals(self.app.requests[0].body, 'Test File\nOne\n')
self.assertEquals(self.app.requests[1].body, 'Test\nFile\nTwo\n')

def test_redirect_with_query(self):
key = 'abc'
sig, env, body = self._make_sig_env_body(
'/v1/AUTH_test/container', 'http://redirect?one=two', 1024, 10,
int(time() + 86400), key)
env['wsgi.input'] = StringIO('\r\n'.join(body))
env['swift.cache'] = FakeMemcache()
env['swift.cache'].set('temp-url-key/AUTH_test', key)
self.app = FakeApp(iter([('201 Created', {}, ''),
('201 Created', {}, '')]))
self.auth = tempauth.filter_factory({})(self.app)
self.formpost = formpost.filter_factory({})(self.auth)
status = [None]
headers = [None]
exc_info = [None]

def start_response(s, h, e=None):
status[0] = s
headers[0] = h
exc_info[0] = e

body = ''.join(self.formpost(env, start_response))
status = status[0]
headers = headers[0]
exc_info = exc_info[0]
self.assertEquals(status, '303 See Other')
location = None
for h, v in headers:
if h.lower() == 'location':
location = v
self.assertEquals(location,
'http://redirect?one=two&status=201&message=')
self.assertEquals(exc_info, None)
self.assertTrue(location in body)
self.assertEquals(len(self.app.requests), 2)
self.assertEquals(self.app.requests[0].body, 'Test File\nOne\n')
self.assertEquals(self.app.requests[1].body, 'Test\nFile\nTwo\n')

def test_no_redirect(self):
key = 'abc'
sig, env, body = self._make_sig_env_body(
Expand Down

0 comments on commit ffada71

Please sign in to comment.