Skip to content

Commit

Permalink
Adds name_check filter
Browse files Browse the repository at this point in the history
Bug 926048.

Filter checks path for user-defined forbidden characters, and for
user-defined maximum length.

Includes changes to reflect gholt's latest comments to Patch Set 4
Also includes a change to a unit-test, renames another unit-test,
and removes one superfluous unit-test.

Added section to the example proxy config

Fixed-up unit test pep8 warnings

Changed error response code to 400 (Bad Request)

Change-Id: Iace719d6a3d00fb3dda1b9d0bc185b8c4cbc00ca
  • Loading branch information
Eamonn O'Toole committed Mar 8, 2012
1 parent 097cca1 commit cf1aa3c
Show file tree
Hide file tree
Showing 4 changed files with 188 additions and 0 deletions.
6 changes: 6 additions & 0 deletions etc/proxy-server.conf-sample
Expand Up @@ -220,3 +220,9 @@ use = egg:swift#tempurl
# Note: Put formpost just before your auth filter(s) in the pipeline
[filter:formpost]
use = egg:swift#formpost

# Note: Just needs to be placed before the proxy-server in the pipeline.
[filter:name_check]
use = egg:swift#name_check
# forbidden_chars = '"`<>
# maximum_length = 255
1 change: 1 addition & 0 deletions setup.py
Expand Up @@ -93,6 +93,7 @@
'recon=swift.common.middleware.recon:filter_factory',
'tempurl=swift.common.middleware.tempurl:filter_factory',
'formpost=swift.common.middleware.formpost:filter_factory',
'name_check=swift.common.middleware.name_check:filter_factory',
],
},
)
109 changes: 109 additions & 0 deletions swift/common/middleware/name_check.py
@@ -0,0 +1,109 @@
# Copyright (c) 2012 OpenStack, LLC.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
# implied.
# See the License for the specific language governing permissions and
# limitations under the License.
'''
Created on February 27, 2012
A filter that disallows any paths that contain defined forbidden characters
or that exceed a defined length.
Place in proxy filter before proxy, e.g.
[pipeline:main]
pipeline = catch_errors healthcheck name_check cache tempauth sos proxy-server
[filter:name_check]
use = egg:swift#name_check
forbidden_chars = '"`<>
maximum_length = 255
There are default settings for forbidden_chars (FORBIDDEN_CHARS) and
maximum_length (MAX_LENGTH)
The filter returns HTTPBadRequest if path is invalid.
@author: eamonn-otoole
'''

from swift.common.utils import get_logger
from webob import Request
from webob.exc import HTTPBadRequest
from urllib2 import unquote

FORBIDDEN_CHARS = "\'\"`<>"
MAX_LENGTH = 255


class NameCheckMiddleware(object):

def __init__(self, app, conf):
self.app = app
self.conf = conf
self.forbidden_chars = self.conf.get('forbidden_chars',
FORBIDDEN_CHARS)
self.maximum_length = self.conf.get('maximum_length', MAX_LENGTH)
self.logger = get_logger(self.conf, log_route='name_check')

def check_character(self, req):
'''
Checks req.path for any forbidden characters
Returns True if there are any forbidden characters
Returns False if there aren't any forbidden characters
'''
self.logger.debug("name_check: path %s" % req.path)
self.logger.debug("name_check: self.forbidden_chars %s" %
self.forbidden_chars)

for c in unquote(req.path):
if c in self.forbidden_chars:
return True
else:
pass
return False

def check_length(self, req):
'''
Checks that req.path doesn't exceed the defined maximum length
Returns True if the length exceeds the maximum
Returns False if the length is <= the maximum
'''
length = len(unquote(req.path))
if length > self.maximum_length:
return True
else:
return False

def __call__(self, env, start_response):
req = Request(env)

if self.check_character(req):
return HTTPBadRequest(request=req,
body=("Object/Container name contains forbidden chars from %s"
% self.forbidden_chars))(env, start_response)
elif self.check_length(req):
return HTTPBadRequest(request=req,
body=("Object/Container name longer than the allowed maximum %s"
% self.maximum_length))(env, start_response)
else:
# Pass on to downstream WSGI component
return self.app(env, start_response)


def filter_factory(global_conf, **local_conf):
conf = global_conf.copy()
conf.update(local_conf)

def name_check_filter(app):
return NameCheckMiddleware(app, conf)
return name_check_filter
72 changes: 72 additions & 0 deletions test/unit/common/middleware/test_name_check.py
@@ -0,0 +1,72 @@
# Copyright (c) 2012 OpenStack, LLC.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
# implied.
# See the License for the specific language governing permissions and
# limitations under the License.

'''
Unit tests for Name_check filter
Created on February 29, 2012
@author: eamonn-otoole
'''

import unittest
from webob import Request, Response
from swift.common.middleware import name_check

MAX_LENGTH = 255
FORBIDDEN_CHARS = '\'\"<>`'


class FakeApp(object):

def __call__(self, env, start_response):
return Response(body="OK")(env, start_response)


class TestNameCheckMiddleware(unittest.TestCase):

def setUp(self):
self.conf = {'maximum_length': MAX_LENGTH, 'forbidden_chars':
FORBIDDEN_CHARS}
self.test_check = name_check.filter_factory(self.conf)(FakeApp())

def test_valid_length_and_character(self):
path = '/V1.0/' + 'c' * (MAX_LENGTH - 6)
resp = Request.blank(path, environ={'REQUEST_METHOD': 'PUT'}
).get_response(self.test_check)
self.assertEquals(resp.body, 'OK')

def test_invalid_character(self):
for c in self.conf['forbidden_chars']:
path = '/V1.0/1234' + c + '5'
resp = Request.blank(path, environ={'REQUEST_METHOD': 'PUT'}
).get_response(self.test_check)
self.assertEquals(resp.body,
("Object/Container name contains forbidden chars from %s"
% self.conf['forbidden_chars']))
self.assertEquals(resp.status_int, 400)

def test_invalid_length(self):
path = '/V1.0/' + 'c' * (MAX_LENGTH - 5)
resp = Request.blank(path, environ={'REQUEST_METHOD': 'PUT'}
).get_response(self.test_check)
self.assertEquals(resp.body,
("Object/Container name longer than the allowed maximum %s"
% self.conf['maximum_length']))
self.assertEquals(resp.status_int, 400)


if __name__ == '__main__':
unittest.main()

0 comments on commit cf1aa3c

Please sign in to comment.