This is an automated email from the ASF dual-hosted git repository.
Antonio-RiveroMartnez pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 98fac9104fb fix(datasets): validate metric d3format (#45633)
98fac9104fb is described below
commit 98fac9104fbf6765526fc2e29da9305cfa355839
Author: Shaitan <[email protected]>
AuthorDate: Thu Oct 22 21:49:15 2026 +0100
fix(datasets): validate metric d3format (#45633)
Co-authored-by: Claude Opus 4.8 <[email protected]>
---
superset/datasets/schemas.py | 17 ++++-
superset/mcp_service/dataset/schemas.py | 21 ++++++
superset/translations/messages.pot | 3 +
superset/utils/number_format.py | 94 +++++++++++++++++++++++++++
tests/integration_tests/datasets/api_tests.py | 45 +++++++++++++
tests/unit_tests/datasets/schema_tests.py | 80 +++++++++++++++++++++++
tests/unit_tests/utils/number_format_test.py | 31 +++++++++
7 files changed, 289 insertions(+), 2 deletions(-)
diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py
index 9001d02829b..163776b818a 100644
--- a/superset/datasets/schemas.py
+++ b/superset/datasets/schemas.py
@@ -36,6 +36,7 @@ from superset.constants import EPOCH_FORMATS
from superset.exceptions import SupersetMarshmallowValidationError
from superset.models.sql_types import parse_currency_string
from superset.utils import json
+from superset.utils.number_format import is_valid_d3_format
from superset.utils.schema import DiscardIsManagedExternallyMixin
get_delete_ids_schema = {
@@ -80,6 +81,16 @@ def validate_python_date_format(dt_format: str) -> bool:
return True
+def validate_d3format(value: str | None) -> None:
+ if not is_valid_d3_format(value):
+ raise ValidationError(
+ _(
+ "Invalid number format: must be a d3-format specifier or a "
+ "supported named format"
+ )
+ )
+
+
class ResolveIsCertifiedMixin(Schema):
"""
``is_certified`` is derived from
``certified_by``/``certification_details``.
@@ -165,7 +176,9 @@ class DatasetMetricsPutSchema(ResolveIsCertifiedMixin,
Schema):
metric_name = fields.String(required=True, validate=Length(1, 255))
metric_type = fields.String(allow_none=True, validate=Length(1, 32))
- d3format = fields.String(allow_none=True, validate=Length(1, 128))
+ d3format = fields.String(
+ allow_none=True, validate=[Length(1, 128), validate_d3format]
+ )
currency = CurrencyField(DatasetMetricCurrencyPutSchema, allow_none=True)
verbose_name = fields.String(allow_none=True, metadata={Length: (1, 1024)})
warning_text = fields.String(allow_none=True)
@@ -421,7 +434,7 @@ class ImportV1MetricSchema(Schema):
metric_type = fields.String(allow_none=True)
expression = fields.String(required=True)
description = fields.String(allow_none=True)
- d3format = fields.String(allow_none=True)
+ d3format = fields.String(allow_none=True, validate=validate_d3format)
currency = CurrencyField(ImportMetricCurrencySchema, allow_none=True)
extra = fields.Dict(allow_none=True)
warning_text = fields.String(allow_none=True)
diff --git a/superset/mcp_service/dataset/schemas.py
b/superset/mcp_service/dataset/schemas.py
index 7ac3e6fd8d6..d94c9873b8e 100644
--- a/superset/mcp_service/dataset/schemas.py
+++ b/superset/mcp_service/dataset/schemas.py
@@ -72,6 +72,7 @@ from superset.mcp_service.utils.serialization import (
)
from superset.sql.parse import has_aggregate
from superset.utils import json
+from superset.utils.number_format import is_valid_d3_format
class DatasetFilter(ColumnOperator):
@@ -475,6 +476,16 @@ class CreateDatasetMetric(BaseModel):
)
return value
+ @field_validator("d3format")
+ @classmethod
+ def d3format_must_be_valid(cls, value: str | None) -> str | None:
+ if not is_valid_d3_format(value):
+ raise ValueError(
+ "Invalid number format: must be a d3-format specifier or a "
+ "supported named format"
+ )
+ return value
+
class CreateDatasetCalculatedColumn(BaseModel):
"""Calculated column definition for dataset creation."""
@@ -718,6 +729,16 @@ class DatasetMetricProperties(OmittedMeansUnchanged):
None, description="JSON-encoded string with extra metric metadata."
)
+ @field_validator("d3format")
+ @classmethod
+ def d3format_must_be_valid(cls, value: str | None) -> str | None:
+ if not is_valid_d3_format(value):
+ raise ValueError(
+ "Invalid number format: must be a d3-format specifier or a "
+ "supported named format"
+ )
+ return value
+
def updates(self) -> Dict[str, Any]:
"""Return only the metric properties explicitly provided by the caller.
diff --git a/superset/translations/messages.pot
b/superset/translations/messages.pot
index fad4710ba14..da1b67008df 100644
--- a/superset/translations/messages.pot
+++ b/superset/translations/messages.pot
@@ -7868,6 +7868,9 @@ msgstr ""
msgid "Invalid metric object: %(metric)s"
msgstr ""
+msgid "Invalid number format: must be a d3-format specifier or a supported
named format"
+msgstr ""
+
#, python-format
msgid "Invalid numpy function: %(operator)s"
msgstr ""
diff --git a/superset/utils/number_format.py b/superset/utils/number_format.py
index 99fccfbefa2..b7050cdd83a 100644
--- a/superset/utils/number_format.py
+++ b/superset/utils/number_format.py
@@ -609,3 +609,97 @@ def raw_string(value: float) -> str:
if isinstance(value, float) and value.is_integer():
return str(int(value))
return str(value)
+
+
+# ---------------------------------------------------------------------------
+# Server-side validation of a user-supplied ``d3format`` string.
+#
+# Distinct from ``D3_FORMAT_RE`` above (which parses the subset the
report-table
+# path renders and rejects padding): this matches the full d3-format grammar
for
+# *acceptance* and also allows the named formats the frontend registers, so the
+# dataset API accepts exactly what the UI can render and nothing else.
+# ---------------------------------------------------------------------------
+
+OVER_MAX_HIDDEN: str = "OVER_MAX_HIDDEN"
+
+# Full d3-format specifier, matched against the whole string:
+# [[fill]align][sign][symbol][0][width][,][.precision][~][type]
+# A fill character is allowed only when followed by an align character; type is
+# case-sensitive. See https://github.com/d3/d3-format.
+D3_FORMAT_VALIDATION_RE: re.Pattern[str] = re.compile(
+ r"(?:.?[<>=^])?" # [[fill]align]
+ r"[-+ (]?" # [sign]
+ r"[$#]?" # [symbol]
+ r"0?" # [0] zero-pad flag
+ r"\d*" # [width]
+ r",?" # [,] group separator
+ r"(?:\.\d+)?" # [.precision]
+ r"~?" # [~] trim insignificant zeros
+ r"[efgrspbodxXcn%]?" # [type]
+)
+
+# Named number formats (and deprecated/typo shims) the frontend accepts as
+# ``d3format`` values but that are not plain d3 specifiers. Kept in one
constant
+# so the backend stays in sync with what the UI renders. Sources:
+# - superset-frontend/packages/superset-ui-core/src/number-format/
+# NumberFormats.ts (SMART_NUMBER, SMART_NUMBER_SIGNED, OVER_MAX_HIDDEN)
+# - superset-frontend/src/setup/setupFormatters.ts (the registered named
+# formatters -- already enumerated by UNSUPPORTED_FRONTEND_PRESETS, plus
the
+# throughput formatters -- and the "deprecated or common typo" shims)
+# - superset-frontend/src/explore/controls.tsx (D3_FORMAT_OPTIONS)
+NAMED_NUMBER_FORMATS: frozenset[str] = UNSUPPORTED_FRONTEND_PRESETS |
frozenset(
+ {
+ SMART_NUMBER,
+ SMART_NUMBER_SIGNED,
+ OVER_MAX_HIDDEN,
+ "NETWORK_THROUGHPUT",
+ "NETWORK_THROUGHPUT_FROM_BYTES",
+ # deprecated / common-typo shims (setupFormatters.ts)
+ ",0",
+ "null",
+ "%",
+ ".",
+ ",f",
+ ",r",
+ "0f",
+ ",#",
+ "$,f",
+ "0%",
+ "f",
+ ",.",
+ ".1%f",
+ "1%",
+ "3%",
+ ",%",
+ ".r",
+ "$,.0",
+ "$,.1",
+ ",0s",
+ "%%%",
+ ",0f",
+ "+,%",
+ "$f",
+ "+,",
+ ",2f",
+ ",g",
+ "int",
+ ".0%f",
+ "$,0",
+ "$,0f",
+ "$,.f",
+ }
+)
+
+
+def is_valid_d3_format(value: str | None) -> bool:
+ """Return whether ``value`` is an acceptable ``d3format`` string.
+
+ ``None`` and the empty string (unset) are accepted, as are the supported
+ named formats and any well-formed d3-format specifier. Length is enforced
+ separately by the schema.
+ """
+ if not value:
+ return True
+ if value in NAMED_NUMBER_FORMATS:
+ return True
+ return D3_FORMAT_VALIDATION_RE.fullmatch(value) is not None
diff --git a/tests/integration_tests/datasets/api_tests.py
b/tests/integration_tests/datasets/api_tests.py
index 6a6ff7e8a23..4844193cb29 100644
--- a/tests/integration_tests/datasets/api_tests.py
+++ b/tests/integration_tests/datasets/api_tests.py
@@ -1477,6 +1477,51 @@ class TestDatasetApi(SupersetTestCase):
self.items_to_delete = [dataset]
+ def test_update_dataset_metric_d3format_validation(self):
+ """
+ Dataset API: a metric d3format must be a valid number format
+ """
+ dataset = self.insert_default_dataset()
+ uri = f"api/v1/dataset/{dataset.id}"
+ self.login(ADMIN_USERNAME)
+ rv = self.get_assert_metric(uri, "get")
+ existing = json.loads(rv.data.decode("utf-8"))["result"]["metrics"]
+ strip_read_only_fields(existing)
+
+ base_metric = {
+ "description": None,
+ "expression": "COUNT(*)",
+ "metric_name": "fmt_metric",
+ "metric_type": None,
+ "verbose_name": None,
+ "warning_text": None,
+ }
+
+ # An invalid number format is rejected and nothing is stored.
+ # DatasetPutSchema reports schema errors as a SIP-40 422.
+ invalid = {**base_metric, "d3format": "<img src=x onerror=alert(1)>"}
+ rv = self.client.put(uri, json={"metrics": [*existing, invalid]})
+ assert rv.status_code == 422
+ assert (
+ db.session.query(SqlMetric)
+ .filter_by(table_id=dataset.id, metric_name="fmt_metric")
+ .one_or_none()
+ is None
+ )
+
+ # A valid specifier saves.
+ valid = {**base_metric, "d3format": ",.2f"}
+ rv = self.client.put(uri, json={"metrics": [*existing, valid]})
+ assert rv.status_code == 200
+ stored = (
+ db.session.query(SqlMetric)
+ .filter_by(table_id=dataset.id, metric_name="fmt_metric")
+ .one()
+ )
+ assert stored.d3format == ",.2f"
+
+ self.items_to_delete = [dataset]
+
def test_update_dataset_delete_metric(self):
"""
Dataset API: Test update dataset delete metric
diff --git a/tests/unit_tests/datasets/schema_tests.py
b/tests/unit_tests/datasets/schema_tests.py
index 684c1ad765c..a2cf4b7962b 100644
--- a/tests/unit_tests/datasets/schema_tests.py
+++ b/tests/unit_tests/datasets/schema_tests.py
@@ -369,3 +369,83 @@ def test_dataset_put_schema_resolves_is_certified(
result = DatasetColumnsPutSchema().load({"column_name": "a", **item})
assert result == {"column_name": "a", **expected}
+
+
[email protected]("fmt", [None, ",.2f", ".3s", "SMART_NUMBER",
"DURATION"])
+def test_metric_put_schema_accepts_valid_d3format(fmt: Any) -> None:
+ from superset.datasets.schemas import DatasetMetricsPutSchema
+
+ result = DatasetMetricsPutSchema().load(
+ {"metric_name": "m", "expression": "COUNT(*)", "d3format": fmt}
+ )
+ assert result["metric_name"] == "m"
+
+
[email protected](
+ "fmt",
+ [
+ "<script>alert(1)</script>",
+ "<img src=x onerror=alert(1)>",
+ "nulld2fxk)",
+ ",.2f<b>",
+ "SMART_NUMBER<",
+ ],
+)
+def test_metric_put_schema_rejects_invalid_d3format(fmt: str) -> None:
+ from superset.datasets.schemas import DatasetMetricsPutSchema
+
+ with pytest.raises(ValidationError) as exc:
+ DatasetMetricsPutSchema().load(
+ {"metric_name": "m", "expression": "COUNT(*)", "d3format": fmt}
+ )
+ assert "d3format" in exc.value.messages
+
+
+def test_metric_put_schema_rejects_over_max_length_d3format() -> None:
+ from superset.datasets.schemas import DatasetMetricsPutSchema
+
+ # A well-formed specifier that exceeds the 128-char column limit.
+ too_long = "," + "." + "1" * 130 + "f"
+ with pytest.raises(ValidationError) as exc:
+ DatasetMetricsPutSchema().load(
+ {"metric_name": "m", "expression": "COUNT(*)", "d3format":
too_long}
+ )
+ assert "d3format" in exc.value.messages
+
+
+def test_dataset_put_schema_reports_offending_metric_index() -> None:
+ from superset.datasets.schemas import DatasetPutSchema
+ from superset.exceptions import SupersetMarshmallowValidationError
+
+ # DatasetPutSchema.handle_error wraps marshmallow errors in a SIP-40 error
+ # (HTTP 422); the nested payload still points at the offending metric
index.
+ with pytest.raises(SupersetMarshmallowValidationError) as exc:
+ DatasetPutSchema().load(
+ {
+ "metrics": [
+ {"metric_name": "ok", "expression": "COUNT(*)",
"d3format": ",.2f"},
+ {
+ "metric_name": "bad",
+ "expression": "COUNT(*)",
+ "d3format": "<script>alert(1)</script>",
+ },
+ ]
+ }
+ )
+ messages = exc.value.error.extra["messages"]
+ assert 1 in messages["metrics"]
+ assert "d3format" in messages["metrics"][1]
+
+
+def test_import_metric_schema_rejects_invalid_d3format() -> None:
+ from superset.datasets.schemas import ImportV1MetricSchema
+
+ with pytest.raises(ValidationError) as exc:
+ ImportV1MetricSchema().load(
+ {
+ "metric_name": "m",
+ "expression": "COUNT(*)",
+ "d3format": "<img src=x onerror=alert(1)>",
+ }
+ )
+ assert "d3format" in exc.value.messages
diff --git a/tests/unit_tests/utils/number_format_test.py
b/tests/unit_tests/utils/number_format_test.py
index 30369e8ee9a..48985af2363 100644
--- a/tests/unit_tests/utils/number_format_test.py
+++ b/tests/unit_tests/utils/number_format_test.py
@@ -28,6 +28,8 @@ from superset.utils.number_format import (
format_number_with_config,
format_numeric,
get_currency_locale,
+ is_valid_d3_format,
+ NAMED_NUMBER_FORMATS,
resolve_auto_currency,
resolve_symbol_position,
)
@@ -586,3 +588,32 @@ def
test_unsupported_d3_padding_is_explicitly_rejected(d3_format: str) -> None:
with pytest.raises(ValueError, match="padding"):
format_d3(d3_format, 1234.5)
assert format_number_with_config(d3_format, None, 1234.5) == "1234.5"
+
+
[email protected](
+ "value",
+ [None, "", ",d", ",.2f", ".3s", "+,.1%", "$,.2f", "~g", "0>8d", "(,.2f",
".4r"],
+)
+def test_is_valid_d3_format_accepts_valid_specifiers(value: str | None) ->
None:
+ assert is_valid_d3_format(value) is True
+
+
[email protected]("value", sorted(NAMED_NUMBER_FORMATS))
+def test_is_valid_d3_format_accepts_named_formats(value: str) -> None:
+ assert is_valid_d3_format(value) is True
+
+
[email protected](
+ "value",
+ [
+ "<script>alert(1)</script>",
+ "<img src=x onerror=alert(1)>",
+ "nulld2fxk)",
+ ",.2f<b>",
+ "SMART_NUMBER<",
+ "not a format",
+ "abc",
+ ],
+)
+def test_is_valid_d3_format_rejects_invalid_values(value: str) -> None:
+ assert is_valid_d3_format(value) is False
