diff --git a/ckanext/datastore/db.py b/ckanext/datastore/db.py index 5618d2a74f9..e4eb48e6704 100644 --- a/ckanext/datastore/db.py +++ b/ckanext/datastore/db.py @@ -83,19 +83,6 @@ def _is_valid_table_name(name): return _is_valid_field_name(name) -def _validate_int(i, field_name, non_negative=False): - try: - i = int(i) - except ValueError: - raise ValidationError({ - field_name: ['{0} is not an integer'.format(i)] - }) - if non_negative and i < 0: - raise ValidationError({ - field_name: ['{0} is not a non-negative integer'.format(i)] - }) - - def _get_engine(data_dict): '''Get either read or write engine.''' connection_url = data_dict['connection_url'] @@ -823,6 +810,9 @@ def validate(context, data_dict): if 'fields' in data_dict_copy: fields = datastore_helpers.get_list(data_dict_copy['fields']) data_dict_copy['fields'] = fields + if 'sort' in data_dict_copy: + fields = datastore_helpers.get_list(data_dict_copy['sort'], False) + data_dict_copy['sort'] = fields for plugin in p.PluginImplementations(interfaces.IDatastore): data_dict_copy = plugin.datastore_validate(context, @@ -832,19 +822,23 @@ def validate(context, data_dict): # Remove default elements in data_dict del data_dict_copy['connection_url'] del data_dict_copy['resource_id'] + data_dict_copy.pop('id', None) for key, values in data_dict_copy.iteritems(): if not values: continue - if key == 'fields': - raise ValidationError({ - 'fields': [u'field "{0}" not in table'.format(values[0])] - }) - elif key == 'filters': - the_filter = values.keys()[0] - raise ValidationError({ - 'filters': [u'filter "{0}" not in table'.format(the_filter)] - }) + if isinstance(values, basestring): + value = values + elif isinstance(values, (list, tuple)): + value = values[0] + elif isinstance(values, dict): + value = values.keys()[0] + else: + value = values + + raise ValidationError({ + key: [u'invalid value "{0}"'.format(value)] + }) return True @@ -879,10 +873,6 @@ def search_data(context, data_dict): else: sort_clause = '' - # FIXME: Limit could be ALL - _validate_int(limit, 'limit', non_negative=True) - _validate_int(offset, 'offset', non_negative=True) - sql_string = u'''SELECT {select} FROM "{resource}" {ts_query} {where} {sort} LIMIT {limit} OFFSET {offset}'''.format( diff --git a/ckanext/datastore/helpers.py b/ckanext/datastore/helpers.py index c6a9cc4407d..a5ead0f02b3 100644 --- a/ckanext/datastore/helpers.py +++ b/ckanext/datastore/helpers.py @@ -21,6 +21,14 @@ def is_single_statement(sql): return len(sqlparse.split(sql)) <= 1 +def validate_int(i, non_negative=False): + try: + i = int(i) + except ValueError: + return False + return i >= 0 or not non_negative + + def _strip(input): if isinstance(input, basestring) and len(input) and input[0] == input[-1]: return input.strip().strip('"') diff --git a/ckanext/datastore/logic/action.py b/ckanext/datastore/logic/action.py index 8f9a681f3a0..a440a1520d8 100644 --- a/ckanext/datastore/logic/action.py +++ b/ckanext/datastore/logic/action.py @@ -114,7 +114,9 @@ def datastore_create(context, data_dict): res['url_type'] = 'datastore' p.toolkit.get_action('resource_update')(context, res) else: - _check_read_only(context, data_dict) + if not data_dict.pop('force', False): + resource_id = data_dict['resource_id'] + _check_read_only(context, resource_id) data_dict['connection_url'] = pylons.config['ckan.datastore.write_url'] @@ -186,7 +188,9 @@ def datastore_upsert(context, data_dict): p.toolkit.check_access('datastore_upsert', context, data_dict) - _check_read_only(context, data_dict) + if not data_dict.pop('force', False): + resource_id = data_dict['resource_id'] + _check_read_only(context, resource_id) data_dict['connection_url'] = pylons.config['ckan.datastore.write_url'] @@ -234,7 +238,9 @@ def datastore_delete(context, data_dict): p.toolkit.check_access('datastore_delete', context, data_dict) - _check_read_only(context, data_dict) + if not data_dict.pop('force', False): + resource_id = data_dict['resource_id'] + _check_read_only(context, resource_id) data_dict['connection_url'] = pylons.config['ckan.datastore.write_url'] @@ -455,14 +461,12 @@ def _resource_exists(context, data_dict): return results.rowcount > 0 -def _check_read_only(context, data_dict): +def _check_read_only(context, resource_id): ''' Raises exception if the resource is read-only. Make sure the resource id is in resource_id ''' - if data_dict.get('force'): - return res = p.toolkit.get_action('resource_show')( - context, {'id': data_dict['resource_id']}) + context, {'id': resource_id}) if res.get('url_type') != 'datastore': raise p.toolkit.ValidationError({ 'read-only': ['Cannot edit read-only resource. Either pass' diff --git a/ckanext/datastore/plugin.py b/ckanext/datastore/plugin.py index 95d7898ad21..5f1654f2974 100644 --- a/ckanext/datastore/plugin.py +++ b/ckanext/datastore/plugin.py @@ -281,8 +281,64 @@ def datastore_validate(self, context, data_dict, all_field_ids): if key in all_field_ids: del filters[key] + q = data_dict.get('q') + if q: + if isinstance(q, basestring): + del data_dict['q'] + + language = data_dict.get('language') + if language: + if isinstance(language, basestring): + del data_dict['language'] + + plain = data_dict.get('plain') + if plain: + if isinstance(plain, bool): + del data_dict['plain'] + + sort_clauses = data_dict.get('sort') + if sort_clauses: + invalid_clauses = [c for c in sort_clauses + if not self._is_valid_sort(c, all_field_ids)] + data_dict['sort'] = invalid_clauses + + limit = data_dict.get('limit') + if limit: + is_positive_int = datastore_helpers.validate_int(limit, + non_negative=True) + is_all = isinstance(limit, basestring) and limit.lower() == 'all' + if is_positive_int or is_all: + del data_dict['limit'] + + offset = data_dict.get('offset') + if offset: + is_positive_int = datastore_helpers.validate_int(offset, + non_negative=True) + if is_positive_int: + del data_dict['offset'] + return data_dict + def _is_valid_sort(self, clause, all_field_ids): + clause = clause.encode('utf-8') + clause_parts = shlex.split(clause) + + if len(clause_parts) == 1: + field, sort = clause_parts[0], 'asc' + elif len(clause_parts) == 2: + field, sort = clause_parts + else: + return False + + field, sort = unicode(field, 'utf-8'), unicode(sort, 'utf-8') + + if field not in all_field_ids: + return False + if sort.lower() not in ('asc', 'desc'): + return False + + return True + def datastore_delete(self, context, data_dict, all_field_ids, query_dict): query_dict['where'] += self._where(data_dict, all_field_ids) return query_dict @@ -362,23 +418,9 @@ def _sort(self, data_dict, field_ids): field, sort = clause_parts[0], 'asc' elif len(clause_parts) == 2: field, sort = clause_parts - else: - raise ValidationError({ - 'sort': ['not valid syntax for sort clause'] - }) + field, sort = unicode(field, 'utf-8'), unicode(sort, 'utf-8') - if field not in field_ids: - raise ValidationError({ - 'sort': [u'field "{0}" not in table'.format( - field)] - }) - if sort.lower() not in ('asc', 'desc'): - raise ValidationError({ - 'sort': ['sorting can only be asc or desc'] - }) - clause_parsed.append(u'"{0}" {1}'.format( - field, sort) - ) + clause_parsed.append(u'"{0}" {1}'.format(field, sort)) return clause_parsed diff --git a/ckanext/datastore/tests/test_search.py b/ckanext/datastore/tests/test_search.py index 2f6fe407ba9..74176137817 100644 --- a/ckanext/datastore/tests/test_search.py +++ b/ckanext/datastore/tests/test_search.py @@ -13,11 +13,11 @@ import ckanext.datastore.db as db from ckanext.datastore.tests.helpers import extract, rebuild_all_dbs -import ckan.new_tests.factories as factories import ckan.new_tests.helpers as helpers assert_equals = nose.tools.assert_equals assert_raises = nose.tools.assert_raises +assert_in = nose.tools.assert_in class TestDatastoreSearch(tests.WsgiAppCase): @@ -281,7 +281,7 @@ def test_search_invalid(self): extra_environ=auth, status=409) res_dict = json.loads(res.body) assert res_dict['success'] is False - assert res_dict['error']['sort'][0] == u'field "f\xfc\xfc" not in table' + assert_in(u'f\xfc\xfc', res_dict['error']['sort'][0]) def test_search_limit(self): data = {'resource_id': self.data['resource_id'],