This is an automated email from the ASF dual-hosted git repository.
maximebeauchemin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 959c35d bugfix: Improve support for special characters in schema and
table names (#7297)
959c35d is described below
commit 959c35d506759bdbea48db55ef61677f4c1ebd7d
Author: Ville Brofeldt <[email protected]>
AuthorDate: Wed May 8 08:37:44 2019 +0300
bugfix: Improve support for special characters in schema and table names
(#7297)
* Bugfix to SQL Lab to support tables and schemas with characters that
require quoting
* Remove debugging prints
* Add uri encoding to secondary tables call
* Quote schema names for presto
* Quote selected_schema on Snowflake, MySQL and Hive
* Remove redundant parens
* Add python unit tests
* Add js unit test
* Fix flake8 linting error
---
.../javascripts/components/TableSelector_spec.jsx | 20 ++++++++++++++++++++
superset/assets/src/SqlLab/actions/sqlLab.js | 6 ++++--
superset/assets/src/components/TableSelector.jsx | 10 +++++-----
superset/db_engine_specs.py | 7 +++++--
superset/utils/core.py | 15 +++++++++++++--
superset/views/core.py | 18 ++++++++++--------
tests/utils_tests.py | 16 ++++++++++++++++
7 files changed, 73 insertions(+), 19 deletions(-)
diff --git a/superset/assets/spec/javascripts/components/TableSelector_spec.jsx
b/superset/assets/spec/javascripts/components/TableSelector_spec.jsx
index 46d5763..70e2cca 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 b3d61a8..f6ac455 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 c3e405e..ba2cebb 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 5fed480..35a591f 100644
--- a/superset/db_engine_specs.py
+++ b/superset/db_engine_specs.py
@@ -37,6 +37,7 @@ import re
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 @@ class SnowflakeEngineSpec(PostgresBaseEngineSpec):
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 @@ class MySQLEngineSpec(BaseEngineSpec):
@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 @@ class PrestoEngineSpec(BaseEngineSpec):
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 @@ class HiveEngineSpec(PrestoEngineSpec):
@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 3e80c76..3b41457 100644
--- a/superset/utils/core.py
+++ b/superset/utils/core.py
@@ -33,6 +33,7 @@ import smtplib
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 memoized(func=None, watch=None):
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 eb25cd0..1ccfa10 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -1566,8 +1566,8 @@ class Superset(BaseSupersetView):
"""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 @@ class Superset(BaseSupersetView):
}))
@has_access
- @expose('/table/<database_id>/<path:table_name>/<schema>/')
+ @expose('/table/<database_id>/<table_name>/<schema>/')
@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 @@ class Superset(BaseSupersetView):
return json_success(json.dumps(tbl))
@has_access
- @expose('/extra_table_metadata/<database_id>/<path:table_name>/<schema>/')
+ @expose('/extra_table_metadata/<database_id>/<table_name>/<schema>/')
@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 @@ class Superset(BaseSupersetView):
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 7e29089..40dedaf 100644
--- a/tests/utils_tests.py
+++ b/tests/utils_tests.py
@@ -35,6 +35,7 @@ from superset.utils.core import (
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 @@ class UtilsTestCase(unittest.TestCase):
}
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'))
