(superset) 01/01: fix(sqllab): wrap cost-estimate JSON parse to prevent raw JSONDecodeError leak

[el...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

eschutho pushed a commit to branch fix-presto-estimate-cost-jsondecodeerror
in repository https://gitbox.apache.org/repos/asf/superset.git

commit cd109e09f67f76f4111b3677aee625a86ba1236f
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Fri Sep 4 16:42:31 2026 +0000

    fix(sqllab): wrap cost-estimate JSON parse to prevent raw JSONDecodeError leak
    
    QueryEstimationCommand.run() calls estimate_query_cost(), which for
    Presto/Trino parses the EXPLAIN (TYPE IO, FORMAT JSON) output via
    simplejson-backed superset.utils.json (allow_nan=False). When a table
    lacks computed statistics, the engine emits a literal NaN token, so
    json.loads() raises a raw simplejson JSONDecodeError that propagates
    uncaught through the command and the /api/v1/sqllab/estimate/ endpoint,
    surfacing to the user as an opaque 500 GENERIC_BACKEND_ERROR crash.
    
    Catch json.JSONDecodeError as a sibling of the existing
    SupersetTimeoutException handler and convert it to a typed
    SupersetErrorException (500, GENERIC_BACKEND_ERROR) -- a backend/driver
    response-parsing failure, not user input error -- mirroring the sibling
    TemplateError conversion in the same function (#42757).
    
    Co-Authored-By: Claude Opus 4.8 <no...@anthropic.com>
---
 superset/commands/sql_lab/estimate.py              | 14 +++++++-
 tests/unit_tests/commands/sql_lab/test_estimate.py | 42 ++++++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/superset/commands/sql_lab/estimate.py b/superset/commands/sql_lab/estimate.py
index c73996f18e3..cf0be98ff0c 100644
--- a/superset/commands/sql_lab/estimate.py
+++ b/superset/commands/sql_lab/estimate.py
@@ -37,7 +37,7 @@ from superset.exceptions import (
 from superset.jinja_context import get_template_processor
 from superset.models.core import Database
 from superset.sql.parse import SQLScript
-from superset.utils import core as utils
+from superset.utils import core as utils, json
 from superset.utils.rls import apply_rls
 
 logger = logging.getLogger(__name__)
@@ -209,6 +209,18 @@ class QueryEstimationCommand(BaseCommand):
                 ),
                 status=500,
             ) from ex
+        except json.JSONDecodeError as ex:
+            logger.exception(ex)
+            raise SupersetErrorException(
+                SupersetError(
+                    message=__(
+                        "Unable to parse the cost estimate returned by the database."
+                    ),
+                    error_type=SupersetErrorType.GENERIC_BACKEND_ERROR,
+                    level=ErrorLevel.ERROR,
+                ),
+                status=500,
+            ) from ex
 
         spec = self._database.db_engine_spec
         query_cost_formatters: dict[str, Any] = app.config[
diff --git a/tests/unit_tests/commands/sql_lab/test_estimate.py b/tests/unit_tests/commands/sql_lab/test_estimate.py
index a34123af850..e8a1c62820a 100644
--- a/tests/unit_tests/commands/sql_lab/test_estimate.py
+++ b/tests/unit_tests/commands/sql_lab/test_estimate.py
@@ -445,3 +445,45 @@ def test_run_wraps_raw_jinja_undefined_error(
 
     assert exc_info.value.status == 400
     assert exc_info.value.error.error_type == SupersetErrorType.GENERIC_COMMAND_ERROR
+
+
+# ---------------------------------------------------------------------------
+# estimate_query_cost() error handling: a raw simplejson JSONDecodeError from
+# parsing the engine's cost-estimate response must not leak from run()
+# ---------------------------------------------------------------------------
+
+
+@patch("superset.commands.sql_lab.estimate.security_manager", new_callable=MagicMock)
+@patch("superset.commands.sql_lab.estimate.DatabaseDAO")
+def test_run_wraps_raw_jsondecodeerror_from_cost_estimation(
+    mock_dao: MagicMock,
+    mock_security_manager: MagicMock,
+) -> None:
+    """A raw ``simplejson.JSONDecodeError`` raised while parsing the engine's
+    cost-estimate response (e.g. Presto/Trino ``EXPLAIN (TYPE IO, FORMAT JSON)``
+    emitting a bare ``NaN`` token for tables lacking computed statistics) must
+    not leak past ``run()`` -- it should surface as a typed
+    ``SupersetErrorException`` with a 500 ``GENERIC_BACKEND_ERROR``, mirroring
+    the sibling ``TemplateError`` conversion in the same function."""
+    # ``superset.utils.json`` is simplejson-backed, so ``json.JSONDecodeError``
+    # here *is* ``simplejson.errors.JSONDecodeError`` -- the concrete class the
+    # Presto/Trino cost-estimate parse raises, which is distinct from the stdlib
+    # ``json.JSONDecodeError``. Using the util keeps us off the banned direct
+    # ``simplejson`` import while exercising the exact raised type.
+    from superset.utils import json
+
+    mock_database = MagicMock()
+    mock_dao.find_by_id.return_value = mock_database
+    mock_security_manager.raise_for_access.return_value = None
+    # The raw class actually raised by superset.utils.json (simplejson-backed)
+    # when the driver response contains a literal NaN token.
+    mock_database.db_engine_spec.estimate_query_cost.side_effect = json.JSONDecodeError(
+        "Expecting value", "{}", 0
+    )
+
+    command = QueryEstimationCommand(_make_params())
+    with pytest.raises(SupersetErrorException) as exc_info:
+        command.run()
+
+    assert exc_info.value.status == 500
+    assert exc_info.value.error.error_type == SupersetErrorType.GENERIC_BACKEND_ERROR