diff --git a/superset/assets/spec/javascripts/components/TableSelector_spec.jsx b/superset/assets/spec/javascripts/components/TableSelector_spec.jsx index 46d57631dacae..70e2cca1f1925 100644 --- a/superset/assets/spec/javascripts/components/TableSelector_spec.jsx +++ b/superset/assets/spec/javascripts/components/TableSelector_spec.jsx @@ -85,6 +85,7 @@ describe('TableSelector', () => { .getTableNamesBySubStr('') .then((data) => { expect(data).toEqual({ options: [] }); + return Promise.resolve(); })); it('should handle table name', () => { @@ -104,6 +105,23 @@ describe('TableSelector', () => { .then((data) => { expect(fetchMock.calls(GET_TABLE_NAMES_GLOB)).toHaveLength(1); expect(data).toEqual(mockTableOptions); + return Promise.resolve(); + }); + }); + + it('should escape schema and table names', () => { + const GET_TABLE_GLOB = 'glob:*/superset/tables/1/*/*'; + const mockTableOptions = { options: [table] }; + wrapper.setProps({ schema: 'slashed/schema' }); + fetchMock.get(GET_TABLE_GLOB, mockTableOptions, { overwriteRoutes: true }); + + return wrapper + .instance() + .getTableNamesBySubStr('slashed/table') + .then(() => { + expect(fetchMock.lastUrl(GET_TABLE_GLOB)) + .toContain('/slashed%252Fschema/slashed%252Ftable'); + return Promise.resolve(); }); }); }); @@ -125,6 +143,7 @@ describe('TableSelector', () => { .fetchTables(true, 'birth_names') .then(() => { expect(wrapper.state().tableOptions).toHaveLength(3); + return Promise.resolve(); }); }); @@ -138,6 +157,7 @@ describe('TableSelector', () => { expect(wrapper.state().tableOptions).toEqual([]); expect(wrapper.state().tableOptions).toHaveLength(0); expect(mockedProps.handleError.callCount).toBe(1); + return Promise.resolve(); }); }); }); diff --git a/superset/assets/src/SqlLab/actions/sqlLab.js b/superset/assets/src/SqlLab/actions/sqlLab.js index b3d61a8ee0f31..f6ac455fff4f2 100644 --- a/superset/assets/src/SqlLab/actions/sqlLab.js +++ b/superset/assets/src/SqlLab/actions/sqlLab.js @@ -285,7 +285,8 @@ export function addTable(query, tableName, schemaName) { }), ); - SupersetClient.get({ endpoint: `/superset/table/${query.dbId}/${tableName}/${schemaName}/` }) + SupersetClient.get({ endpoint: encodeURI(`/superset/table/${query.dbId}/` + + `${encodeURIComponent(tableName)}/${encodeURIComponent(schemaName)}/`) }) .then(({ json }) => { const dataPreviewQuery = { id: shortid.generate(), @@ -322,7 +323,8 @@ export function addTable(query, tableName, schemaName) { ); SupersetClient.get({ - endpoint: `/superset/extra_table_metadata/${query.dbId}/${tableName}/${schemaName}/`, + endpoint: encodeURI(`/superset/extra_table_metadata/${query.dbId}/` + + `${encodeURIComponent(tableName)}/${encodeURIComponent(schemaName)}/`), }) .then(({ json }) => dispatch(mergeTable({ ...table, ...json, isExtraMetadataLoading: false })), diff --git a/superset/assets/src/components/TableSelector.jsx b/superset/assets/src/components/TableSelector.jsx index c3e405ed48313..ba2cebb2799d8 100644 --- a/superset/assets/src/components/TableSelector.jsx +++ b/superset/assets/src/components/TableSelector.jsx @@ -90,7 +90,7 @@ export default class TableSelector extends React.PureComponent { onChange() { this.props.onChange({ dbId: this.state.dbId, - shema: this.state.schema, + schema: this.state.schema, tableName: this.state.tableName, }); } @@ -101,9 +101,8 @@ export default class TableSelector extends React.PureComponent { return Promise.resolve({ options }); } return SupersetClient.get({ - endpoint: ( - `/superset/tables/${this.props.dbId}/` + - `${this.props.schema}/${input}`), + endpoint: encodeURI(`/superset/tables/${this.props.dbId}/` + + `${encodeURIComponent(this.props.schema)}/${encodeURIComponent(input)}`), }).then(({ json }) => ({ options: this.addOptionIfMissing(json.options, tableName) })); } dbMutator(data) { @@ -123,7 +122,8 @@ export default class TableSelector extends React.PureComponent { const { dbId, schema } = this.props; if (dbId && schema) { this.setState(() => ({ tableLoading: true, tableOptions: [] })); - const endpoint = `/superset/tables/${dbId}/${schema}/${substr}/${forceRefresh}/`; + const endpoint = encodeURI(`/superset/tables/${dbId}/` + + `${encodeURIComponent(schema)}/${encodeURIComponent(substr)}/${forceRefresh}/`); return SupersetClient.get({ endpoint }) .then(({ json }) => { const filterOptions = createFilterOptions({ options: json.options }); diff --git a/superset/db_engine_specs.py b/superset/db_engine_specs.py index 5fed48049107f..35a591fa10202 100644 --- a/superset/db_engine_specs.py +++ b/superset/db_engine_specs.py @@ -37,6 +37,7 @@ import textwrap import time from typing import List, Tuple +from urllib import parse from flask import g from flask_babel import lazy_gettext as _ @@ -577,6 +578,7 @@ def adjust_database_uri(cls, uri, selected_schema=None): if '/' in uri.database: database = uri.database.split('/')[0] if selected_schema: + selected_schema = parse.quote(selected_schema, safe='') uri.database = database + '/' + selected_schema return uri @@ -757,7 +759,7 @@ def convert_dttm(cls, target_type, dttm): @classmethod def adjust_database_uri(cls, uri, selected_schema=None): if selected_schema: - uri.database = selected_schema + uri.database = parse.quote(selected_schema, safe='') return uri @classmethod @@ -1081,6 +1083,7 @@ def select_star(cls, my_db, table_name: str, engine: Engine, schema: str = None, def adjust_database_uri(cls, uri, selected_schema=None): database = uri.database if selected_schema and database: + selected_schema = parse.quote(selected_schema, safe='') if '/' in database: database = database.split('/')[0] + '/' + selected_schema else: @@ -1484,7 +1487,7 @@ def convert_dttm(cls, target_type, dttm): @classmethod def adjust_database_uri(cls, uri, selected_schema=None): if selected_schema: - uri.database = selected_schema + uri.database = parse.quote(selected_schema, safe='') return uri @classmethod diff --git a/superset/utils/core.py b/superset/utils/core.py index 3e80c76355d29..3b4145793939a 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -33,6 +33,7 @@ import sys from time import struct_time from typing import List, Optional, Tuple +from urllib.parse import unquote_plus import uuid import zlib @@ -141,8 +142,18 @@ def wrapper(f): return wrapper -def js_string_to_python(item: str) -> Optional[str]: - return None if item in ('null', 'undefined') else item +def parse_js_uri_path_item(item: Optional[str], unquote: bool = True, + eval_undefined: bool = False) -> Optional[str]: + """Parse a uri path item made with js. + + :param item: a uri path component + :param unquote: Perform unquoting of string using urllib.parse.unquote_plus() + :param eval_undefined: When set to True and item is either 'null' or 'undefined', + assume item is undefined and return None. + :return: Either None, the original item or unquoted item + """ + item = None if eval_undefined and item in ('null', 'undefined') else item + return unquote_plus(item) if unquote and item else item def string_to_num(s: str): diff --git a/superset/views/core.py b/superset/views/core.py index eb25cd0d12dda..1ccfa1037ab83 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1566,8 +1566,8 @@ def tables(self, db_id, schema, substr, force_refresh='false'): """Endpoint to fetch the list of tables for given database""" db_id = int(db_id) force_refresh = force_refresh.lower() == 'true' - schema = utils.js_string_to_python(schema) - substr = utils.js_string_to_python(substr) + schema = utils.parse_js_uri_path_item(schema, eval_undefined=True) + substr = utils.parse_js_uri_path_item(substr, eval_undefined=True) database = db.session.query(models.Database).filter_by(id=db_id).one() if schema: @@ -2350,11 +2350,11 @@ def sqllab_viz(self): })) @has_access - @expose('/table////') + @expose('/table////') @log_this def table(self, database_id, table_name, schema): - schema = utils.js_string_to_python(schema) - table_name = parse.unquote_plus(table_name) + schema = utils.parse_js_uri_path_item(schema, eval_undefined=True) + table_name = utils.parse_js_uri_path_item(table_name) mydb = db.session.query(models.Database).filter_by(id=database_id).one() payload_columns = [] indexes = [] @@ -2410,11 +2410,11 @@ def table(self, database_id, table_name, schema): return json_success(json.dumps(tbl)) @has_access - @expose('/extra_table_metadata////') + @expose('/extra_table_metadata////') @log_this def extra_table_metadata(self, database_id, table_name, schema): - schema = utils.js_string_to_python(schema) - table_name = parse.unquote_plus(table_name) + schema = utils.parse_js_uri_path_item(schema, eval_undefined=True) + table_name = utils.parse_js_uri_path_item(table_name) mydb = db.session.query(models.Database).filter_by(id=database_id).one() payload = mydb.db_engine_spec.extra_table_metadata( mydb, table_name, schema) @@ -2427,6 +2427,8 @@ def extra_table_metadata(self, database_id, table_name, schema): def select_star(self, database_id, table_name, schema=None): mydb = db.session.query( models.Database).filter_by(id=database_id).first() + schema = utils.parse_js_uri_path_item(schema, eval_undefined=True) + table_name = utils.parse_js_uri_path_item(table_name) return json_success( mydb.select_star( table_name, diff --git a/tests/utils_tests.py b/tests/utils_tests.py index 7e2908984f424..40dedafaef597 100644 --- a/tests/utils_tests.py +++ b/tests/utils_tests.py @@ -35,6 +35,7 @@ merge_extra_filters, merge_request_params, parse_human_timedelta, + parse_js_uri_path_item, validate_json, zlib_compress, zlib_decompress_to_string, @@ -756,3 +757,18 @@ def test_convert_legacy_filters_into_adhoc_present_and_nonempty(self): } convert_legacy_filters_into_adhoc(form_data) self.assertEquals(form_data, expected) + + def test_parse_js_uri_path_items_eval_undefined(self): + self.assertIsNone(parse_js_uri_path_item('undefined', eval_undefined=True)) + self.assertIsNone(parse_js_uri_path_item('null', eval_undefined=True)) + self.assertEqual('undefined', parse_js_uri_path_item('undefined')) + self.assertEqual('null', parse_js_uri_path_item('null')) + + def test_parse_js_uri_path_items_unquote(self): + self.assertEqual('slashed/name', parse_js_uri_path_item('slashed%2fname')) + self.assertEqual('slashed%2fname', parse_js_uri_path_item('slashed%2fname', + unquote=False)) + + def test_parse_js_uri_path_items_item_optional(self): + self.assertIsNone(parse_js_uri_path_item(None)) + self.assertIsNotNone(parse_js_uri_path_item('item'))