This is an automated email from the ASF dual-hosted git repository.
eschutho 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 5cfcee95b4e fix(import): return 400 instead of 500 on malformed JSON
in import form fields (#45579)
5cfcee95b4e is described below
commit 5cfcee95b4e06dacb1f23d489d841c29c8893bce
Author: Elizabeth Thompson <[email protected]>
AuthorDate: Thu Oct 22 14:59:27 2026 -0700
fix(import): return 400 instead of 500 on malformed JSON in import form
fields (#45579)
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
---
superset/charts/api.py | 25 +++-------
superset/dashboards/api.py | 26 +++--------
superset/databases/api.py | 32 +++----------
superset/datasets/api.py | 26 +++--------
superset/importexport/api.py | 38 +++++----------
superset/queries/saved_queries/api.py | 26 +++--------
superset/views/base_api.py | 24 ++++++++++
tests/unit_tests/databases/api_test.py | 44 ++++++++++++++++++
tests/unit_tests/importexport/api_test.py | 30 ++++++++++++
.../unit_tests/views/test_parse_json_form_field.py | 54 ++++++++++++++++++++++
10 files changed, 195 insertions(+), 130 deletions(-)
diff --git a/superset/charts/api.py b/superset/charts/api.py
index e2b3488e04c..fd83e07cc93 100644
--- a/superset/charts/api.py
+++ b/superset/charts/api.py
@@ -150,6 +150,7 @@ from superset.versioning.etag import set_version_etag
from superset.versioning.schemas import VersionListItemSchema
from superset.views.base_api import (
BaseSupersetModelRestApi,
+ parse_json_form_field,
RelatedFieldFilter,
requires_form_data,
requires_json,
@@ -1754,26 +1755,12 @@ class ChartRestApi(SoftDeleteApiMixin,
BaseSupersetModelRestApi):
if not contents:
raise NoValidFilesFoundError()
- passwords = (
- json.loads(request.form["passwords"])
- if "passwords" in request.form
- else None
- )
+ passwords = parse_json_form_field("passwords")
overwrite = request.form.get("overwrite") == "true"
- ssh_tunnel_passwords = (
- json.loads(request.form["ssh_tunnel_passwords"])
- if "ssh_tunnel_passwords" in request.form
- else None
- )
- ssh_tunnel_private_keys = (
- json.loads(request.form["ssh_tunnel_private_keys"])
- if "ssh_tunnel_private_keys" in request.form
- else None
- )
- ssh_tunnel_priv_key_passwords = (
- json.loads(request.form["ssh_tunnel_private_key_passwords"])
- if "ssh_tunnel_private_key_passwords" in request.form
- else None
+ ssh_tunnel_passwords = parse_json_form_field("ssh_tunnel_passwords")
+ ssh_tunnel_private_keys =
parse_json_form_field("ssh_tunnel_private_keys")
+ ssh_tunnel_priv_key_passwords = parse_json_form_field(
+ "ssh_tunnel_private_key_passwords"
)
command = ImportChartsCommand(
diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py
index 1860d8d4682..bba2857eb92 100644
--- a/superset/dashboards/api.py
+++ b/superset/dashboards/api.py
@@ -210,7 +210,6 @@ from superset.tasks.thumbnails import (
cache_dashboard_thumbnail,
)
from superset.tasks.utils import get_current_user
-from superset.utils import json
from superset.utils.core import (
get_user_id,
parse_boolean_string,
@@ -237,6 +236,7 @@ from superset.versioning.etag import set_version_etag
from superset.versioning.schemas import VersionListItemSchema
from superset.views.base_api import (
BaseSupersetModelRestApi,
+ parse_json_form_field,
RelatedFieldFilter,
requires_form_data,
requires_json,
@@ -3185,28 +3185,14 @@ class DashboardRestApi(
if not contents:
raise NoValidFilesFoundError()
- passwords = (
- json.loads(request.form["passwords"])
- if "passwords" in request.form
- else None
- )
+ passwords = parse_json_form_field("passwords")
overwrite = request.form.get("overwrite") == "true"
overwrite_all = parse_boolean_string(request.form.get("overwrite_all",
"false"))
- ssh_tunnel_passwords = (
- json.loads(request.form["ssh_tunnel_passwords"])
- if "ssh_tunnel_passwords" in request.form
- else None
- )
- ssh_tunnel_private_keys = (
- json.loads(request.form["ssh_tunnel_private_keys"])
- if "ssh_tunnel_private_keys" in request.form
- else None
- )
- ssh_tunnel_priv_key_passwords = (
- json.loads(request.form["ssh_tunnel_private_key_passwords"])
- if "ssh_tunnel_private_key_passwords" in request.form
- else None
+ ssh_tunnel_passwords = parse_json_form_field("ssh_tunnel_passwords")
+ ssh_tunnel_private_keys =
parse_json_form_field("ssh_tunnel_private_keys")
+ ssh_tunnel_priv_key_passwords = parse_json_form_field(
+ "ssh_tunnel_private_key_passwords"
)
command = ImportDashboardsCommand(
diff --git a/superset/databases/api.py b/superset/databases/api.py
index a1ac73a8514..d38f44c497b 100644
--- a/superset/databases/api.py
+++ b/superset/databases/api.py
@@ -128,7 +128,6 @@ from superset.extensions import security_manager
from superset.models.core import Database
from superset.sql.parse import Partition, Table
from superset.superset_typing import FlaskResponse
-from superset.utils import json
from superset.utils.core import (
error_msg_from_exception,
get_username,
@@ -141,6 +140,7 @@ from superset.utils.oauth2 import decode_oauth2_state
from superset.utils.ssh_tunnel import mask_password_info
from superset.views.base_api import (
BaseSupersetModelRestApi,
+ parse_json_form_field,
RelatedFieldFilter,
requires_form_data,
requires_json,
@@ -1710,32 +1710,14 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
if not contents:
raise NoValidFilesFoundError()
- passwords = (
- json.loads(request.form["passwords"])
- if "passwords" in request.form
- else None
- )
+ passwords = parse_json_form_field("passwords")
overwrite = request.form.get("overwrite") == "true"
- ssh_tunnel_passwords = (
- json.loads(request.form["ssh_tunnel_passwords"])
- if "ssh_tunnel_passwords" in request.form
- else None
- )
- ssh_tunnel_private_keys = (
- json.loads(request.form["ssh_tunnel_private_keys"])
- if "ssh_tunnel_private_keys" in request.form
- else None
- )
- ssh_tunnel_priv_key_passwords = (
- json.loads(request.form["ssh_tunnel_private_key_passwords"])
- if "ssh_tunnel_private_key_passwords" in request.form
- else None
- )
- encrypted_extra_secrets = (
- json.loads(request.form["encrypted_extra_secrets"])
- if "encrypted_extra_secrets" in request.form
- else None
+ ssh_tunnel_passwords = parse_json_form_field("ssh_tunnel_passwords")
+ ssh_tunnel_private_keys =
parse_json_form_field("ssh_tunnel_private_keys")
+ ssh_tunnel_priv_key_passwords = parse_json_form_field(
+ "ssh_tunnel_private_key_passwords"
)
+ encrypted_extra_secrets =
parse_json_form_field("encrypted_extra_secrets")
command = ImportDatabasesCommand(
contents,
diff --git a/superset/datasets/api.py b/superset/datasets/api.py
index f41fce02140..4fb8c333ca4 100644
--- a/superset/datasets/api.py
+++ b/superset/datasets/api.py
@@ -110,7 +110,6 @@ from superset.exceptions import (
)
from superset.jinja_context import BaseTemplateProcessor,
get_template_processor
from superset.subjects.filters import FilterRelatedSubjects,
subject_type_filter
-from superset.utils import json
from superset.utils.core import (
parse_boolean_string,
send_export_zip,
@@ -137,6 +136,7 @@ from superset.versioning.schemas import
VersionListItemSchema
from superset.views.base import DatasourceFilter
from superset.views.base_api import (
BaseSupersetModelRestApi,
+ parse_json_form_field,
RelatedFieldFilter,
requires_form_data,
requires_json,
@@ -1754,28 +1754,14 @@ class DatasetRestApi(SoftDeleteApiMixin,
BaseSupersetModelRestApi):
if not contents:
raise NoValidFilesFoundError()
- passwords = (
- json.loads(request.form["passwords"])
- if "passwords" in request.form
- else None
- )
+ passwords = parse_json_form_field("passwords")
overwrite = request.form.get("overwrite") == "true"
sync_columns = request.form.get("sync_columns") == "true"
sync_metrics = request.form.get("sync_metrics") == "true"
- ssh_tunnel_passwords = (
- json.loads(request.form["ssh_tunnel_passwords"])
- if "ssh_tunnel_passwords" in request.form
- else None
- )
- ssh_tunnel_private_keys = (
- json.loads(request.form["ssh_tunnel_private_keys"])
- if "ssh_tunnel_private_keys" in request.form
- else None
- )
- ssh_tunnel_priv_key_passwords = (
- json.loads(request.form["ssh_tunnel_private_key_passwords"])
- if "ssh_tunnel_private_key_passwords" in request.form
- else None
+ ssh_tunnel_passwords = parse_json_form_field("ssh_tunnel_passwords")
+ ssh_tunnel_private_keys =
parse_json_form_field("ssh_tunnel_private_keys")
+ ssh_tunnel_priv_key_passwords = parse_json_form_field(
+ "ssh_tunnel_private_key_passwords"
)
command = ImportDatasetsCommand(
diff --git a/superset/importexport/api.py b/superset/importexport/api.py
index 3f912f5cd3a..15a6e27fce4 100644
--- a/superset/importexport/api.py
+++ b/superset/importexport/api.py
@@ -32,13 +32,17 @@ from superset.commands.importers.v1.utils import (
open_import_bundle,
)
from superset.extensions import event_logger
-from superset.utils import json
from superset.utils.core import (
parse_boolean_string,
send_export_zip,
write_zip_entry,
)
-from superset.views.base_api import BaseSupersetApi, requires_form_data,
statsd_metrics
+from superset.views.base_api import (
+ BaseSupersetApi,
+ parse_json_form_field,
+ requires_form_data,
+ statsd_metrics,
+)
class ImportExportRestApi(BaseSupersetApi):
@@ -199,31 +203,13 @@ class ImportExportRestApi(BaseSupersetApi):
# endpoint always overwrote existing assets.
overwrite = parse_boolean_string(request.form.get("overwrite", "true"))
- passwords = (
- json.loads(request.form["passwords"])
- if "passwords" in request.form
- else None
- )
- ssh_tunnel_passwords = (
- json.loads(request.form["ssh_tunnel_passwords"])
- if "ssh_tunnel_passwords" in request.form
- else None
- )
- ssh_tunnel_private_keys = (
- json.loads(request.form["ssh_tunnel_private_keys"])
- if "ssh_tunnel_private_keys" in request.form
- else None
- )
- ssh_tunnel_priv_key_passwords = (
- json.loads(request.form["ssh_tunnel_private_key_passwords"])
- if "ssh_tunnel_private_key_passwords" in request.form
- else None
- )
- encrypted_extra_secrets = (
- json.loads(request.form["encrypted_extra_secrets"])
- if "encrypted_extra_secrets" in request.form
- else None
+ passwords = parse_json_form_field("passwords")
+ ssh_tunnel_passwords = parse_json_form_field("ssh_tunnel_passwords")
+ ssh_tunnel_private_keys =
parse_json_form_field("ssh_tunnel_private_keys")
+ ssh_tunnel_priv_key_passwords = parse_json_form_field(
+ "ssh_tunnel_private_key_passwords"
)
+ encrypted_extra_secrets =
parse_json_form_field("encrypted_extra_secrets")
command = ImportAssetsCommand(
contents,
diff --git a/superset/queries/saved_queries/api.py
b/superset/queries/saved_queries/api.py
index 1662a3f97d0..37d794d6009 100644
--- a/superset/queries/saved_queries/api.py
+++ b/superset/queries/saved_queries/api.py
@@ -57,10 +57,10 @@ from superset.queries.saved_queries.schemas import (
openapi_spec_methods_override,
validate_label,
)
-from superset.utils import json
from superset.utils.core import send_export_zip, write_zip_entry
from superset.views.base_api import (
BaseSupersetModelRestApi,
+ parse_json_form_field,
RelatedFieldFilter,
requires_form_data,
statsd_metrics,
@@ -402,26 +402,12 @@ class SavedQueryRestApi(BaseSupersetModelRestApi):
if not contents:
raise NoValidFilesFoundError()
- passwords = (
- json.loads(request.form["passwords"])
- if "passwords" in request.form
- else None
- )
+ passwords = parse_json_form_field("passwords")
overwrite = request.form.get("overwrite") == "true"
- ssh_tunnel_passwords = (
- json.loads(request.form["ssh_tunnel_passwords"])
- if "ssh_tunnel_passwords" in request.form
- else None
- )
- ssh_tunnel_private_keys = (
- json.loads(request.form["ssh_tunnel_private_keys"])
- if "ssh_tunnel_private_keys" in request.form
- else None
- )
- ssh_tunnel_priv_key_passwords = (
- json.loads(request.form["ssh_tunnel_private_key_passwords"])
- if "ssh_tunnel_private_key_passwords" in request.form
- else None
+ ssh_tunnel_passwords = parse_json_form_field("ssh_tunnel_passwords")
+ ssh_tunnel_private_keys =
parse_json_form_field("ssh_tunnel_private_keys")
+ ssh_tunnel_priv_key_passwords = parse_json_form_field(
+ "ssh_tunnel_private_key_passwords"
)
command = ImportSavedQueriesCommand(
diff --git a/superset/views/base_api.py b/superset/views/base_api.py
index 8c4398725e7..aca0caee27b 100644
--- a/superset/views/base_api.py
+++ b/superset/views/base_api.py
@@ -50,6 +50,7 @@ from superset.models.slice import Slice
from superset.schemas import error_payload_content
from superset.sql_lab import Query as SqllabQuery
from superset.superset_typing import FlaskResponse
+from superset.utils import json
from superset.utils.core import get_user_id, time_function
from superset.views.error_handling import handle_api_exception,
json_error_response
@@ -191,6 +192,29 @@ def requires_form_data(f: Callable[..., Any]) ->
Callable[..., Any]:
return functools.update_wrapper(wraps, f)
+def parse_json_form_field(name: str) -> dict[str, Any] | None:
+ """
+ Parse an optional JSON object from a multipart form field.
+
+ :param name: the form field name
+ :returns: the parsed object, or None if the field is absent
+ :raises InvalidPayloadFormatError: if the value is not a valid JSON object
+ """
+ if name not in request.form:
+ return None
+ try:
+ value = json.loads(request.form[name])
+ except json.JSONDecodeError as ex:
+ raise InvalidPayloadFormatError(
+ message=f"Form field '{name}' is not valid JSON"
+ ) from ex
+ if not isinstance(value, dict):
+ raise InvalidPayloadFormatError(
+ message=f"Form field '{name}' must be a JSON object"
+ )
+ return value
+
+
@overload
def statsd_metrics(
f: Callable[..., Any],
diff --git a/tests/unit_tests/databases/api_test.py
b/tests/unit_tests/databases/api_test.py
index efc371aa34d..119474e69e6 100644
--- a/tests/unit_tests/databases/api_test.py
+++ b/tests/unit_tests/databases/api_test.py
@@ -709,6 +709,50 @@ def test_import_with_encrypted_extra_secrets(
)
[email protected](
+ "field, value, message",
+ [
+ ("passwords", "{not json", "Form field 'passwords' is not valid JSON"),
+ ("passwords", "123", "Form field 'passwords' must be a JSON object"),
+ (
+ "encrypted_extra_secrets",
+ "{not json",
+ "Form field 'encrypted_extra_secrets' is not valid JSON",
+ ),
+ ],
+)
+def test_import_invalid_json_form_field(
+ mocker: MockerFixture,
+ client: Any,
+ full_api_access: None,
+ field: str,
+ value: str,
+ message: str,
+) -> None:
+ """
+ Test that a malformed JSON form field returns a 400 instead of a 500.
+ """
+ mocker.patch("superset.databases.api.is_zipfile", return_value=True)
+ mocker.patch("superset.databases.api.open_import_bundle")
+ mocker.patch(
+ "superset.databases.api.get_contents_from_bundle",
+ return_value={"metadata.yaml": "version: 1.0.0\n"},
+ )
+ command = mocker.patch("superset.databases.api.ImportDatabasesCommand")
+
+ form_data = {"formData": (BytesIO(b"test"), "test.zip"), field: value}
+ response = client.post(
+ "/api/v1/database/import/",
+ data=form_data,
+ content_type="multipart/form-data",
+ )
+
+ assert response.status_code == 400
+ assert response.json["errors"][0]["message"] == message
+ assert response.json["errors"][0]["error_type"] ==
"INVALID_PAYLOAD_FORMAT_ERROR"
+ command.assert_not_called()
+
+
def test_non_zip_import(client: Any, full_api_access: None) -> None:
"""
Test that non-ZIP imports are not allowed.
diff --git a/tests/unit_tests/importexport/api_test.py
b/tests/unit_tests/importexport/api_test.py
index 4855c2fdfa4..ebf249716a8 100644
--- a/tests/unit_tests/importexport/api_test.py
+++ b/tests/unit_tests/importexport/api_test.py
@@ -420,6 +420,36 @@ def test_import_assets_incorrect_form_data(
assert response.json == {"message": "Arguments are not correct"}
+def test_import_assets_malformed_json_form_field(
+ mocker: MockerFixture,
+ client: Any,
+ full_api_access: None,
+) -> None:
+ """
+ Test that a malformed JSON form field returns a 400 instead of a 500.
+ """
+ ImportAssetsCommand =
mocker.patch("superset.importexport.api.ImportAssetsCommand") # noqa: N806
+
+ buf = BytesIO()
+ with ZipFile(buf, "w") as bundle:
+ with bundle.open("assets_export/metadata.yaml", "w") as fp:
+ fp.write(b"version: 1.0.0\ntype: assets\n")
+ buf.seek(0)
+
+ form_data = {
+ "bundle": (buf, "assets_export.zip"),
+ "encrypted_extra_secrets": "{not json",
+ }
+ response = client.post(
+ "/api/v1/assets/import/", data=form_data,
content_type="multipart/form-data"
+ )
+ assert response.status_code == 400
+ assert response.json["errors"][0]["message"] == (
+ "Form field 'encrypted_extra_secrets' is not valid JSON"
+ )
+ ImportAssetsCommand.assert_not_called()
+
+
def test_import_assets_no_contents(
mocker: MockerFixture,
client: Any,
diff --git a/tests/unit_tests/views/test_parse_json_form_field.py
b/tests/unit_tests/views/test_parse_json_form_field.py
new file mode 100644
index 00000000000..bacff693657
--- /dev/null
+++ b/tests/unit_tests/views/test_parse_json_form_field.py
@@ -0,0 +1,54 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from typing import TYPE_CHECKING
+
+import pytest
+
+from superset.exceptions import InvalidPayloadFormatError
+from superset.views.base_api import parse_json_form_field
+
+if TYPE_CHECKING:
+ from superset.app import SupersetApp
+
+
+def test_parse_json_form_field_absent(app: "SupersetApp") -> None:
+ with app.test_request_context(method="POST", data={"other": "{}"}):
+ assert parse_json_form_field("passwords") is None
+
+
+def test_parse_json_form_field_valid(app: "SupersetApp") -> None:
+ with app.test_request_context(
+ method="POST", data={"passwords": '{"databases/db.yaml": "secret"}'}
+ ):
+ assert parse_json_form_field("passwords") == {"databases/db.yaml":
"secret"}
+
+
+def test_parse_json_form_field_malformed(app: "SupersetApp") -> None:
+ with app.test_request_context(method="POST", data={"passwords": "{not
json"}):
+ with pytest.raises(InvalidPayloadFormatError) as excinfo:
+ parse_json_form_field("passwords")
+ assert excinfo.value.status == 400
+ assert excinfo.value.error.message == "Form field 'passwords' is not valid
JSON"
+
+
[email protected]("value", ["[]", "[1]", "123", '"secret"', "null"])
+def test_parse_json_form_field_not_object(app: "SupersetApp", value: str) ->
None:
+ with app.test_request_context(method="POST", data={"passwords": value}):
+ with pytest.raises(InvalidPayloadFormatError) as excinfo:
+ parse_json_form_field("passwords")
+ assert excinfo.value.status == 400
+ assert excinfo.value.error.message == "Form field 'passwords' must be a
JSON object"
