You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/09/24 20:47:16 UTC
(superset) branch fix/isdigit-unicode-sibling-instances created (now d10e9a258e3)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a change to branch fix/isdigit-unicode-sibling-instances
in repository https://gitbox.apache.org/repos/asf/superset.git
at d10e9a258e3 fix: guard the isdigit()-then-int() pattern against non-decimal digits
This branch includes the following new commits:
new d10e9a258e3 fix: guard the isdigit()-then-int() pattern against non-decimal digits
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: fix: guard the isdigit()-then-int() pattern against non-decimal digits
Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch fix/isdigit-unicode-sibling-instances
in repository https://gitbox.apache.org/repos/asf/superset.git
commit d10e9a258e3ec2165921ff4459c19a094006335d
Author: Evan Rusackas <ev...@rusackas.com>
AuthorDate: Thu Sep 24 13:46:35 2026 -0700
fix: guard the isdigit()-then-int() pattern against non-decimal digits
#44615 fixed cast_to_num() for strings like "²" or "①": str.isdigit()
is true for Unicode "digit" characters (category No), but int() only
accepts "decimal" characters (category Nd), so isdigit()-then-int()
raises ValueError on them. The identical pattern repeats across DAO
and mcp_service code resolving a string identifier to a numeric ID:
isdigit() says "looks numeric", then int() crashes.
Swapped isdigit() -> isdecimal() at each unguarded call site:
- superset/daos/base.py, daos/datasource.py, models/slice.py: shared
id-or-uuid resolution used across every DAO and Slice lookup.
- superset/utils/date_parser.py: ordinal parsing in handle_nth_of().
- superset/mcp_service/chart/{chart_helpers,chart_utils}.py and the
chart/dashboard/explore mcp_service tool modules (generate_chart,
get_chart_sql, update_chart_preview, dataset_validator,
delete_dashboard, generate_explore_link): dataset_id/identifier
resolution on MCP tool entry points, directly reachable from
external LLM-client input.
Left three other isdigit() call sites alone: one already wraps int()
in try/except (commands/dashboard/export.py), one is a heuristic with
no int() call (get_chart_preview.py's form-data-key guess), and one
is a SQL/KQL tokenizer regex branch (sql/parse.py) that's a different
risk shape and deserves its own review rather than a mechanical swap.
Added regression tests for the three most widely-shared call sites
(BaseDAO.find_by_id_or_uuid, DatasourceDAO.get_datasource, Slice's
id_or_uuid_filter) confirming a non-decimal digit string falls
through to the uuid branch instead of crashing.
Co-Authored-By: Evan Rusackas <ev...@preset.io>
Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
superset/daos/base.py | 2 +-
superset/daos/datasource.py | 2 +-
superset/mcp_service/chart/chart_helpers.py | 4 +--
superset/mcp_service/chart/chart_utils.py | 2 +-
superset/mcp_service/chart/tool/generate_chart.py | 7 +++---
superset/mcp_service/chart/tool/get_chart_sql.py | 4 +--
.../mcp_service/chart/tool/update_chart_preview.py | 4 +--
.../chart/validation/dataset_validator.py | 2 +-
.../mcp_service/dashboard/tool/delete_dashboard.py | 2 +-
.../explore/tool/generate_explore_link.py | 2 +-
superset/models/slice.py | 2 +-
superset/utils/date_parser.py | 2 +-
tests/unit_tests/dao/base_dao_test.py | 29 +++++++++++++++++++++-
tests/unit_tests/datasource/dao_tests.py | 16 ++++++++++++
tests/unit_tests/models/slice_test.py | 4 +++
15 files changed, 66 insertions(+), 18 deletions(-)
diff --git a/superset/daos/base.py b/superset/daos/base.py
index 46528206819..c3966c93dc1 100644
--- a/superset/daos/base.py
+++ b/superset/daos/base.py
@@ -265,7 +265,7 @@ class BaseDAO(CoreBaseDAO[T], Generic[T]):
id_column = getattr(cls.model_cls, cls.id_column_name)
uuid_column = getattr(cls.model_cls, cls.uuid_column_name)
- if model_id_or_uuid.isdigit():
+ if model_id_or_uuid.isdecimal():
filter = id_column == int(model_id_or_uuid)
else:
filter = uuid_column == model_id_or_uuid
diff --git a/superset/daos/datasource.py b/superset/daos/datasource.py
index 93c52c88e25..eb3832e8979 100644
--- a/superset/daos/datasource.py
+++ b/superset/daos/datasource.py
@@ -65,7 +65,7 @@ class DatasourceDAO(BaseDAO[Datasource]):
model = cls.sources[datasource_type]
- if str(database_id_or_uuid).isdigit():
+ if str(database_id_or_uuid).isdecimal():
filter = model.id == int(database_id_or_uuid)
else:
try:
diff --git a/superset/mcp_service/chart/chart_helpers.py b/superset/mcp_service/chart/chart_helpers.py
index f191697e72c..47e6927cb51 100644
--- a/superset/mcp_service/chart/chart_helpers.py
+++ b/superset/mcp_service/chart/chart_helpers.py
@@ -151,7 +151,7 @@ def find_chart_by_identifier(
{"query_options": query_options} if query_options is not None else {}
)
if isinstance(identifier, int) or (
- isinstance(identifier, str) and identifier.isdigit()
+ isinstance(identifier, str) and identifier.isdecimal()
):
chart_id = int(identifier) if isinstance(identifier, str) else identifier
return ChartDAO.find_by_id(chart_id, **extra)
@@ -958,7 +958,7 @@ def resolve_form_data_datasource(
if not datasource_id and (combined := form_data.get("datasource")):
if isinstance(combined, str) and "__" in combined:
parts = combined.split("__", 1)
- datasource_id = int(parts[0]) if parts[0].isdigit() else parts[0]
+ datasource_id = int(parts[0]) if parts[0].isdecimal() else parts[0]
datasource_type = parts[1] if len(parts) > 1 else None
if not datasource_id and chart:
diff --git a/superset/mcp_service/chart/chart_utils.py b/superset/mcp_service/chart/chart_utils.py
index adf06de1acc..5bbf38c21eb 100644
--- a/superset/mcp_service/chart/chart_utils.py
+++ b/superset/mcp_service/chart/chart_utils.py
@@ -217,7 +217,7 @@ def generate_explore_link(
try:
if isinstance(dataset_id, int) or (
- isinstance(dataset_id, str) and dataset_id.isdigit()
+ isinstance(dataset_id, str) and dataset_id.isdecimal()
):
numeric_dataset_id = (
int(dataset_id) if isinstance(dataset_id, str) else dataset_id
diff --git a/superset/mcp_service/chart/tool/generate_chart.py b/superset/mcp_service/chart/tool/generate_chart.py
index 8f5aff64cf2..2e9948d5a46 100644
--- a/superset/mcp_service/chart/tool/generate_chart.py
+++ b/superset/mcp_service/chart/tool/generate_chart.py
@@ -344,7 +344,8 @@ async def generate_chart( # noqa: C901
with event_logger.log_context(action="mcp.generate_chart.dataset_lookup"):
dataset = None
if isinstance(request.dataset_id, int) or (
- isinstance(request.dataset_id, str) and request.dataset_id.isdigit()
+ isinstance(request.dataset_id, str)
+ and request.dataset_id.isdecimal()
):
dataset_id = (
int(request.dataset_id)
@@ -618,7 +619,7 @@ async def generate_chart( # noqa: C901
from superset.daos.dataset import DatasetDAO
if isinstance(request.dataset_id, int) or (
- isinstance(request.dataset_id, str) and request.dataset_id.isdigit()
+ isinstance(request.dataset_id, str) and request.dataset_id.isdecimal()
):
candidate_id = (
int(request.dataset_id)
@@ -748,7 +749,7 @@ async def generate_chart( # noqa: C901
# Convert dataset_id to int only if numeric
if (
isinstance(request.dataset_id, str)
- and request.dataset_id.isdigit()
+ and request.dataset_id.isdecimal()
):
dataset_id_for_preview = int(request.dataset_id)
elif isinstance(request.dataset_id, int):
diff --git a/superset/mcp_service/chart/tool/get_chart_sql.py b/superset/mcp_service/chart/tool/get_chart_sql.py
index 82cec075c41..d441b3a02df 100644
--- a/superset/mcp_service/chart/tool/get_chart_sql.py
+++ b/superset/mcp_service/chart/tool/get_chart_sql.py
@@ -125,7 +125,7 @@ def _find_chart_by_identifier(
from superset.daos.chart import ChartDAO
if isinstance(identifier, int) or (
- isinstance(identifier, str) and identifier.isdigit()
+ isinstance(identifier, str) and identifier.isdecimal()
):
chart_id = int(identifier) if isinstance(identifier, str) else identifier
return ChartDAO.find_by_id(chart_id)
@@ -275,7 +275,7 @@ def _resolve_datasource_name(
if not datasource_id and (combined := form_data.get("datasource")):
if isinstance(combined, str) and "__" in combined:
parts = combined.split("__", 1)
- datasource_id = int(parts[0]) if parts[0].isdigit() else parts[0]
+ datasource_id = int(parts[0]) if parts[0].isdecimal() else parts[0]
datasource_type = parts[1] if len(parts) > 1 else "table"
if not datasource_id:
diff --git a/superset/mcp_service/chart/tool/update_chart_preview.py b/superset/mcp_service/chart/tool/update_chart_preview.py
index ff278144717..5b458ba26af 100644
--- a/superset/mcp_service/chart/tool/update_chart_preview.py
+++ b/superset/mcp_service/chart/tool/update_chart_preview.py
@@ -81,7 +81,7 @@ def _find_dataset(dataset_id: int | str) -> Any | None:
from superset.mcp_service.auth import has_dataset_access
if isinstance(dataset_id, int) or (
- isinstance(dataset_id, str) and dataset_id.isdigit()
+ isinstance(dataset_id, str) and dataset_id.isdecimal()
):
dataset = DatasetDAO.find_by_id(int(dataset_id))
else:
@@ -264,7 +264,7 @@ def update_chart_preview( # noqa: C901
from superset.daos.dataset import DatasetDAO
if isinstance(request.dataset_id, int) or (
- isinstance(request.dataset_id, str) and request.dataset_id.isdigit()
+ isinstance(request.dataset_id, str) and request.dataset_id.isdecimal()
):
dataset = DatasetDAO.find_by_id(int(request.dataset_id))
else:
diff --git a/superset/mcp_service/chart/validation/dataset_validator.py b/superset/mcp_service/chart/validation/dataset_validator.py
index d7cb7158b56..0872abe733d 100644
--- a/superset/mcp_service/chart/validation/dataset_validator.py
+++ b/superset/mcp_service/chart/validation/dataset_validator.py
@@ -437,7 +437,7 @@ class DatasetValidator:
from superset.daos.dataset import DatasetDAO
if isinstance(dataset_id, int) or (
- isinstance(dataset_id, str) and dataset_id.isdigit()
+ isinstance(dataset_id, str) and dataset_id.isdecimal()
):
dataset = DatasetDAO.find_by_id(int(dataset_id))
else:
diff --git a/superset/mcp_service/dashboard/tool/delete_dashboard.py b/superset/mcp_service/dashboard/tool/delete_dashboard.py
index b078ebe7ffa..4f7754903ff 100644
--- a/superset/mcp_service/dashboard/tool/delete_dashboard.py
+++ b/superset/mcp_service/dashboard/tool/delete_dashboard.py
@@ -51,7 +51,7 @@ def _find_dashboard_by_identifier(identifier: int | str) -> "Dashboard | None":
from superset.daos.dashboard import DashboardDAO
if isinstance(identifier, int) or (
- isinstance(identifier, str) and identifier.isdigit()
+ isinstance(identifier, str) and identifier.isdecimal()
):
return DashboardDAO.find_by_id(int(identifier))
# Try UUID, then fall back to slug.
diff --git a/superset/mcp_service/explore/tool/generate_explore_link.py b/superset/mcp_service/explore/tool/generate_explore_link.py
index 12c4c87f1b3..95799596514 100644
--- a/superset/mcp_service/explore/tool/generate_explore_link.py
+++ b/superset/mcp_service/explore/tool/generate_explore_link.py
@@ -125,7 +125,7 @@ async def generate_explore_link(
with event_logger.log_context(action="mcp.generate_explore_link.dataset_check"):
dataset = None
if isinstance(request.dataset_id, int) or (
- isinstance(request.dataset_id, str) and request.dataset_id.isdigit()
+ isinstance(request.dataset_id, str) and request.dataset_id.isdecimal()
):
dataset_id_int = (
int(request.dataset_id)
diff --git a/superset/models/slice.py b/superset/models/slice.py
index fcf81f1f4cc..4d3105b17d1 100644
--- a/superset/models/slice.py
+++ b/superset/models/slice.py
@@ -487,7 +487,7 @@ class Slice( # pylint: disable=too-many-public-methods
def id_or_uuid_filter(id_or_uuid: str | int) -> BinaryExpression:
if isinstance(id_or_uuid, int):
return Slice.id == id_or_uuid
- if id_or_uuid.isdigit():
+ if id_or_uuid.isdecimal():
return Slice.id == int(id_or_uuid)
return Slice.uuid == id_or_uuid
diff --git a/superset/utils/date_parser.py b/superset/utils/date_parser.py
index 19c6452c315..dcb85e75e65 100644
--- a/superset/utils/date_parser.py
+++ b/superset/utils/date_parser.py
@@ -349,7 +349,7 @@ def handle_nth_of(
"DATETRUNC(..., year) : DATEADD(DATETRUNC(..., year), 1, week)"
"""
# Convert ordinal to number
- n = ORDINAL_MAP.get(ordinal.lower(), int(ordinal) if ordinal.isdigit() else 1)
+ n = ORDINAL_MAP.get(ordinal.lower(), int(ordinal) if ordinal.isdecimal() else 1)
relative_base = get_relative_base(unit, relative_start)
effective_scope = scope.lower() if scope else "this"
diff --git a/tests/unit_tests/dao/base_dao_test.py b/tests/unit_tests/dao/base_dao_test.py
index 578bf59bb64..a04eb7eb7cc 100644
--- a/tests/unit_tests/dao/base_dao_test.py
+++ b/tests/unit_tests/dao/base_dao_test.py
@@ -19,7 +19,7 @@
Unit tests for BaseDAO functionality using mocks and no database operations.
"""
-from unittest.mock import Mock, patch
+from unittest.mock import MagicMock, Mock, patch
import pytest
from sqlalchemy import Boolean, Column, Integer, String
@@ -335,6 +335,33 @@ def test_find_by_id_or_uuid_statement_error_still_returns_none():
assert TestDAO.find_by_id_or_uuid("not-a-uuid") is None
+def test_find_by_id_or_uuid_non_decimal_digit_treated_as_uuid():
+ """A string like "²" (superscript 2) is Unicode "digit" but not
+ "decimal": str.isdigit() is True for it, yet int() rejects it. Must be
+ routed to the uuid branch instead of crashing on int(model_id_or_uuid)."""
+
+ with (
+ patch("superset.daos.base.db") as mock_db,
+ patch("superset.daos.base.getattr") as mock_getattr,
+ ):
+ mock_session = Mock()
+ mock_db.session = mock_session
+ id_column, uuid_column = MagicMock(), MagicMock()
+ id_filter, uuid_filter = Mock(name="id_filter"), Mock(name="uuid_filter")
+ id_column.__eq__.return_value = id_filter
+ uuid_column.__eq__.return_value = uuid_filter
+ mock_getattr.side_effect = [id_column, uuid_column]
+
+ mock_query = Mock()
+ mock_session.query.return_value = mock_query
+ mock_query.filter.return_value = mock_query
+ mock_query.one_or_none.return_value = None
+
+ TestDAO.find_by_id_or_uuid("²")
+
+ mock_query.filter.assert_called_once_with(uuid_filter)
+
+
def test_find_by_column_operational_error_propagates():
"""_find_by_column catches StatementError to absorb coercion errors;
an OperationalError (a StatementError subclass) must still propagate."""
diff --git a/tests/unit_tests/datasource/dao_tests.py b/tests/unit_tests/datasource/dao_tests.py
index 5e106164051..783796186f7 100644
--- a/tests/unit_tests/datasource/dao_tests.py
+++ b/tests/unit_tests/datasource/dao_tests.py
@@ -142,6 +142,22 @@ def test_not_found_datasource(session_with_data: Session) -> None:
)
+def test_get_datasource_non_decimal_digit_raises_value_incorrect(
+ session_with_data: Session,
+) -> None:
+ """ "²" (superscript 2) is str.isdigit() but not str.isdecimal(), so
+ it must be routed to the uuid branch (and fail as an invalid uuid) rather
+ than crash on int(database_id_or_uuid)."""
+ from superset.daos.datasource import DatasourceDAO
+ from superset.daos.exceptions import DatasourceValueIsIncorrect
+
+ with pytest.raises(DatasourceValueIsIncorrect):
+ DatasourceDAO.get_datasource(
+ datasource_type="table",
+ database_id_or_uuid="²",
+ )
+
+
def test_escape_ilike_fragment() -> None:
from superset.daos.datasource import _escape_ilike_fragment
diff --git a/tests/unit_tests/models/slice_test.py b/tests/unit_tests/models/slice_test.py
index 128d6a165de..21990f0a8ba 100644
--- a/tests/unit_tests/models/slice_test.py
+++ b/tests/unit_tests/models/slice_test.py
@@ -81,6 +81,10 @@ class TestSlice:
("uuid_format", "550e8400-e29b-41d4-a716-446655440000"),
("invalid_string", "not-a-number"),
("integer_id", 123),
+ # str.isdigit() is True for "²" (superscript 2) but int()
+ # rejects it; must not raise and must fall through to the uuid
+ # branch instead of the int(id_or_uuid) branch.
+ ("non_decimal_digit", "²"),
]
)
def test_id_or_uuid_filter(self, test_name, input_value):