eschutho opened a new pull request, #45579:
URL: https://github.com/apache/superset/pull/45579
### SUMMARY
**Problem**
The multipart `/import/` endpoints (charts, dashboards, databases, datasets,
saved queries, and assets) parse optional form fields with an unguarded
`json.loads(request.form[...])`. The fields are `passwords`,
`ssh_tunnel_passwords`, `ssh_tunnel_private_keys`,
`ssh_tunnel_private_key_passwords`, and, for databases and assets,
`encrypted_extra_secrets`.
- **Malformed JSON** (e.g. `passwords={not json`) raises a raw
`JSONDecodeError`. Nothing handles it, so it falls through to the generic
`Exception` handler and the client gets an opaque 500, with an ERROR log entry
for what is a user-fixable input problem.
- **Valid JSON that is not an object** (e.g. `passwords=123`) is passed into
the import command. `load_configs` then does `file_name in passwords` and
crashes with `TypeError: argument of type 'int' is not iterable`, which is also
a 500.
This is the same class of bug as #45380 (corrupt ZIP uploads returning 500
instead of 422).
**Fix**
- Add a small shared helper, `parse_json_form_field(name)`, in
`superset/views/base_api.py` next to `requires_form_data`. It returns `None` if
the field is absent. Otherwise it parses the value and raises the existing
`InvalidPayloadFormatError` (400, `INVALID_PAYLOAD_FORMAT_ERROR`) with a
message naming the field if the value is not valid JSON or not a JSON object.
- Replace the 26 identical inline parse blocks across the six import
endpoints with calls to the helper. Behavior for valid input is unchanged.
None of these routes use FAB's `@safe`, so the raised
`InvalidPayloadFormatError` reaches the app-level `SupersetErrorException`
handler and is returned as a structured 400.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (API only).
Before: `POST /api/v1/database/import/` with `passwords={not json` returns
**500** with a generic `GENERIC_BACKEND_ERROR` carrying the raw decoder message.
After: **400** `{"errors": [{"message": "Form field 'passwords' is not valid
JSON", "error_type": "INVALID_PAYLOAD_FORMAT_ERROR", ...}]}`.
### TESTING INSTRUCTIONS
Unit tests:
```bash
pytest tests/unit_tests/views/test_parse_json_form_field.py \
tests/unit_tests/databases/api_test.py \
tests/unit_tests/importexport/api_test.py
```
- `test_parse_json_form_field.py`: covers the helper with an absent field,
valid JSON, malformed JSON, and non-object values (`[]`, `[1]`, `123`,
`"secret"`, `null`).
- `databases/api_test.py::test_import_invalid_json_form_field`: malformed
`passwords`, `passwords=123`, and malformed `encrypted_extra_secrets` all
return 400, and the command is never called.
- `importexport/api_test.py::test_import_assets_malformed_json_form_field`:
a malformed `encrypted_extra_secrets` returns 400.
On the pre-fix code the three malformed-JSON endpoint cases and the assets
case return 500. The `123` case returns 200 there only because the command is
mocked in that test. Without the mock it returns 500 with the `TypeError` above.
Manual check:
```bash
curl -X POST http://localhost:8088/api/v1/database/import/ \
-H "Authorization: Bearer $TOKEN" \
-F formData=@database_export.zip \
-F 'passwords={not json'
# expect HTTP 400 with "Form field 'passwords' is not valid JSON"
```
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
Related: #45380 (same bug class: user-fixable import input returning 500).
The helper validates only the top-level value. The other
`json.loads(request.form[...])` sites in `views/` and `charts/data/api.py` are
outside the import flow and are not changed here.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
