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
This commit is contained in:
Ville Brofeldt
Maxime Beauchemin
parent
a3f091263a
commit
959c35d506
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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 })),
|
||||
|
|
|
|||
|
|
@ -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 });
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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'))
|
||||
|
|
|
|||
Loading…
Reference in New Issue