From abe1791acf0de492a461e8e8c449277bc0d1750d Mon Sep 17 00:00:00 2001 From: cheng Date: Fri, 30 Jun 2017 08:10:11 +0000 Subject: [PATCH] Fix url_path valid check Closes-Bug: #1701456 Change-Id: Ia4595413d9f08e0b0198fba2b3c4932abec7a67a Signed-off-by: cheng --- octavia/api/common/types.py | 14 ++++++++++ octavia/api/v2/types/health_monitor.py | 6 ++--- octavia/common/exceptions.py | 5 ++++ octavia/common/validate.py | 16 ++++++++++++ .../functional/api/v2/test_health_monitor.py | 26 +++++++++++++++++++ .../unit/api/v2/types/test_health_monitors.py | 26 +++++++++++++++++-- 6 files changed, 88 insertions(+), 5 deletions(-) diff --git a/octavia/api/common/types.py b/octavia/api/common/types.py index 2e810dc9e6..4d51c76eaa 100644 --- a/octavia/api/common/types.py +++ b/octavia/api/common/types.py @@ -60,6 +60,20 @@ class URLType(wtypes.UserType): return value +class URLPathType(wtypes.UserType): + basetype = unicode + name = 'url_path' + + @staticmethod + def validate(value): + try: + validate.url_path(value) + except exceptions.InvalidURLPath: + error = 'Value must be a valid URL Path string' + raise ValueError(error) + return value + + class BaseType(wtypes.Base): @classmethod def _full_response(cls): diff --git a/octavia/api/v2/types/health_monitor.py b/octavia/api/v2/types/health_monitor.py index 973d3d83bd..76458aa157 100644 --- a/octavia/api/v2/types/health_monitor.py +++ b/octavia/api/v2/types/health_monitor.py @@ -92,7 +92,7 @@ class HealthMonitorPOST(BaseHealthMonitorType): wtypes.Enum(str, *constants.SUPPORTED_HEALTH_MONITOR_HTTP_METHODS), default=constants.HEALTH_MONITOR_HTTP_DEFAULT_METHOD) url_path = wtypes.wsattr( - types.URLType(require_scheme=False), + types.URLPathType(), default=constants.HEALTH_MONITOR_DEFAULT_URL_PATH) expected_codes = wtypes.wsattr( wtypes.StringType(pattern=r'^(\d{3}(\s*,\s*\d{3})*)$|^(\d{3}-\d{3})$'), @@ -120,7 +120,7 @@ class HealthMonitorPUT(BaseHealthMonitorType): maximum=constants.MAX_HM_RETRIES)) http_method = wtypes.wsattr( wtypes.Enum(str, *constants.SUPPORTED_HEALTH_MONITOR_HTTP_METHODS)) - url_path = wtypes.wsattr(types.URLType(require_scheme=False)) + url_path = wtypes.wsattr(types.URLPathType()) expected_codes = wtypes.wsattr( wtypes.StringType(pattern=r'^(\d{3}(\s*,\s*\d{3})*)$|^(\d{3}-\d{3})$')) admin_state_up = wtypes.wsattr(bool) @@ -149,7 +149,7 @@ class HealthMonitorSingleCreate(BaseHealthMonitorType): wtypes.Enum(str, *constants.SUPPORTED_HEALTH_MONITOR_HTTP_METHODS), default=constants.HEALTH_MONITOR_HTTP_DEFAULT_METHOD) url_path = wtypes.wsattr( - types.URLType(require_scheme=False), + types.URLPathType(), default=constants.HEALTH_MONITOR_DEFAULT_URL_PATH) expected_codes = wtypes.wsattr( wtypes.StringType(pattern=r'^(\d{3}(\s*,\s*\d{3})*)$|^(\d{3}-\d{3})$'), diff --git a/octavia/common/exceptions.py b/octavia/common/exceptions.py index 9869d65c17..792288b096 100644 --- a/octavia/common/exceptions.py +++ b/octavia/common/exceptions.py @@ -239,6 +239,11 @@ class InvalidURL(OctaviaException): message = _('Not a valid URL: %(url)s') +class InvalidURLPath(APIException): + msg = _('Not a valid URLPath: %(url_path)s') + code = 400 + + class InvalidString(OctaviaException): message = _('Invalid characters in %(what)s') diff --git a/octavia/common/validate.py b/octavia/common/validate.py index 9d9bfd7c58..28a149840e 100644 --- a/octavia/common/validate.py +++ b/octavia/common/validate.py @@ -46,6 +46,22 @@ def url(url, require_scheme=True): return True +def url_path(url_path): + """Raises an error if the url_path doesn't look like a URL Path.""" + try: + p_url = rfc3986.urlparse(rfc3986.normalize_uri(url_path)) + + if ( + p_url.scheme or p_url.userinfo or p_url.host or + p_url.port or + p_url.path is None + ): + raise exceptions.InvalidURLPath(url_path=url_path) + except Exception: + raise exceptions.InvalidURLPath(url_path=url_path) + return True + + def header_name(header, what=None): """Raises an error if header does not look like an HTML header name.""" p = re.compile(constants.HTTP_HEADER_NAME_REGEX) diff --git a/octavia/tests/functional/api/v2/test_health_monitor.py b/octavia/tests/functional/api/v2/test_health_monitor.py index 0ebbc799fe..0a9e91dfb2 100644 --- a/octavia/tests/functional/api/v2/test_health_monitor.py +++ b/octavia/tests/functional/api/v2/test_health_monitor.py @@ -572,6 +572,19 @@ class TestHealthMonitor(base.BaseAPITest): hm_prov_status=constants.PENDING_CREATE, hm_op_status=constants.OFFLINE) + def test_create_http_monitor_with_url_path(self): + api_hm = self.create_health_monitor( + self.pool_id, constants.HEALTH_MONITOR_HTTP, + 1, 1, 1, 1, url_path="/v2/api/index").get(self.root_tag) + self.assert_correct_status( + lb_id=self.lb_id, listener_id=self.listener_id, + pool_id=self.pool_id, hm_id=api_hm.get('id'), + lb_prov_status=constants.PENDING_UPDATE, + listener_prov_status=constants.ACTIVE, + pool_prov_status=constants.PENDING_UPDATE, + hm_prov_status=constants.PENDING_CREATE, + hm_op_status=constants.OFFLINE) + def test_create_sans_listener(self): api_hm = self.create_health_monitor( self.pool_id, constants.HEALTH_MONITOR_HTTP, @@ -704,6 +717,19 @@ class TestHealthMonitor(base.BaseAPITest): lb_id=self.lb_id, listener_id=self.listener_id, pool_id=self.pool_id) + def test_bad_create_with_invalid_url_path(self): + req_dict = {'pool_id': self.pool_id, + 'type': constants.HEALTH_MONITOR_HTTP, + 'delay': 1, + 'timeout': 1, + 'max_retries_down': 1, + 'max_retries': 1, + 'url_path': 'https://openstack.org'} + self.post(self.HMS_PATH, self._build_body(req_dict), status=400) + self.assert_correct_status( + lb_id=self.lb_id, listener_id=self.listener_id, + pool_id=self.pool_id) + def test_create_with_bad_handler(self): self.handler_mock().health_monitor.create.side_effect = Exception() api_hm = self.create_health_monitor( diff --git a/octavia/tests/unit/api/v2/types/test_health_monitors.py b/octavia/tests/unit/api/v2/types/test_health_monitors.py index d4a706f7cc..082e7d7e2d 100644 --- a/octavia/tests/unit/api/v2/types/test_health_monitors.py +++ b/octavia/tests/unit/api/v2/types/test_health_monitors.py @@ -75,8 +75,17 @@ class TestHealthMonitor(object): if self._type is hm_type.HealthMonitorPOST: body.update({"type": constants.PROTOCOL_HTTP, "pool_id": uuidutils.generate_uuid()}) - self.assertRaises(exc.InvalidInput, wsme_json.fromjson, self._type, - body) + self.assertRaises(exc.InvalidInput, wsme_json.fromjson, + self._type, body) + + def test_invalid_url_path_with_url(self): + body = {"delay": 1, "timeout": 1, "max_retries": 1, + "url_path": 'https://www.openstack.org'} + if self._type is hm_type.HealthMonitorPOST: + body.update({"type": constants.PROTOCOL_HTTP, + "pool_id": uuidutils.generate_uuid()}) + self.assertRaises(exc.InvalidInput, wsme_json.fromjson, + self._type, body) def test_invalid_expected_codes(self): body = {"delay": 1, "timeout": 1, "max_retries": 1, @@ -140,6 +149,19 @@ class TestHealthMonitorPOST(base.BaseTypesTest, TestHealthMonitor): self.assertEqual(3, hmpost.max_retries_down) self.assertTrue(hmpost.admin_state_up) + def test_url_path_with_query_and_fragment(self): + url_path = "/v2/index?a=12,b=34#123dd" + body = {"type": constants.HEALTH_MONITOR_HTTP, "delay": 1, + "timeout": 1, "max_retries": 1, + "pool_id": uuidutils.generate_uuid(), + "url_path": url_path} + hmpost = wsme_json.fromjson(self._type, body) + self.assertEqual('GET', hmpost.http_method) + self.assertEqual(url_path, hmpost.url_path) + self.assertEqual('200', hmpost.expected_codes) + self.assertEqual(3, hmpost.max_retries_down) + self.assertTrue(hmpost.admin_state_up) + def test_non_uuid_project_id(self): body = {"type": constants.HEALTH_MONITOR_HTTP, "delay": 1, "timeout": 1, "max_retries_down": 1, "max_retries": 1,