You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "eschutho (via GitHub)" <gi...@apache.org> on 2026/10/18 16:37:03 UTC
[PR] fix(import): return 422 instead of 500 for unsafe ZIP import bundles [superset]
eschutho opened a new pull request, #45440:
URL: https://github.com/apache/superset/pull/45440
### SUMMARY
#### Problem
`get_contents_from_bundle()` in `superset/commands/importers/v1/utils.py` calls `check_is_safe_zip()`. That function raises a bare `SupersetException` (default status 500) when an uploaded import bundle has a member larger than `ZIPPED_FILE_MAX_SIZE`, a total uncompressed size larger than `ZIP_FILE_MAX_TOTAL_SIZE`, or a compress ratio above `ZIP_FILE_MAX_COMPRESS_RATIO`.
This is bad input that the user can fix. Because the exception escapes unmapped, all 7 import endpoints (charts, dashboards, databases, datasets, assets, saved queries, themes) return an opaque 500 and log it at ERROR. The columnar upload reader (`superset/commands/database/uploaders/columnar_reader.py`) already maps the same `check_is_safe_zip()` error to a 4xx.
#### Fix
In `get_contents_from_bundle()`, catch `SupersetException` around the `check_is_safe_zip()` call only, and re-raise it as the existing `IncorrectFormatError` (422). The original message is kept and the original exception is chained. `open_import_bundle()` already maps other bundle problems to the same exception.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (API status code only). Posting a high-compress-ratio bundle to `POST /api/v1/assets/import/`:
- **Before:** `500 INTERNAL SERVER ERROR`. `get_contents_from_bundle()` raises `superset.exceptions.SupersetException` (status 500): "Zip compress ratio above allowed threshold".
- **After:** `422`, with `{"message": "Zip compress ratio above allowed threshold", "error_type": "GENERIC_COMMAND_ERROR", "level": "warning", ...}`.
### TESTING INSTRUCTIONS
New tests:
- `tests/unit_tests/commands/importers/v1/utils_test.py::test_get_contents_from_bundle_unsafe_zip` is parametrized over three cases:
- `member-size`: `ZIPPED_FILE_MAX_SIZE` lowered.
- `total-size`: `ZIP_FILE_MAX_TOTAL_SIZE` lowered.
- `compress-ratio`: a real 1 MB-of-zeros deflated bundle under the default config.
Each case asserts `IncorrectFormatError`, status 422, the preserved message, and a `SupersetException` as `__cause__`.
- `tests/unit_tests/importexport/api_test.py::test_import_assets_unsafe_zip` posts a high-compress-ratio bundle to `/api/v1/assets/import/` and asserts a 422 with the expected error payload.
```bash
pytest tests/unit_tests/commands/importers/v1/utils_test.py tests/unit_tests/importexport/api_test.py
```
With only `superset/commands/importers/v1/utils.py` reverted to `master`, the 4 new tests fail: the 3 unit cases raise a bare `SupersetException`, and the API test gets `assert 500 == 422`. With the fix, all 45 tests in both files pass. `tests/unit_tests/utils/test_core.py` and `tests/unit_tests/commands/databases/columnar_reader_test.py` (167 tests) still pass.
### ADDITIONAL INFORMATION
This is a follow-up to #45380 and #45424, in the same bundle-hardening vein. The only behavior change is the status code (500 → 422) and the corresponding log level for these rejected uploads. The size and ratio limits themselves are unchanged.
- [ ] 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
🤖 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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(import): return 422 instead of 500 for unsafe ZIP import bundles [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho merged PR #45440:
URL: https://github.com/apache/superset/pull/45440
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(import): return 422 instead of 500 for unsafe ZIP import bundles [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #45440:
URL: https://github.com/apache/superset/pull/45440#issuecomment-6252451444
## CodeAnt PR Risk: Low Risk
- The PR appears safe to merge; unsafe ZIP imports now surface as format errors instead of server errors.
- Tests cover oversized members, oversized total contents, high compression ratios, and the import API response.
**Assessed commit:** `3488f2d5c503`
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(import): return 422 instead of 500 for unsafe ZIP import bundles [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho closed pull request #45440: fix(import): return 422 instead of 500 for unsafe ZIP import bundles
URL: https://github.com/apache/superset/pull/45440
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(import): return 422 instead of 500 for unsafe ZIP import bundles [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #45440:
URL: https://github.com/apache/superset/pull/45440#issuecomment-6252608640
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/45440?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `60.00000%` with `2 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 65.04%. Comparing base ([`4f49d24`](https://app.codecov.io/gh/apache/superset/commit/4f49d24151d1583b225de5df0983e29d8375ca80?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`3488f2d`](https://app.codecov.io/gh/apache/superset/commit/3488f2d5c5036af54ed6c1d732de9b66173224e8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/45440?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/commands/importers/v1/utils.py](https://app.codecov.io/gh/apache/superset/pull/45440?src=pr&el=tree&filepath=superset%2Fcommands%2Fimporters%2Fv1%2Futils.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvY29tbWFuZHMvaW1wb3J0ZXJzL3YxL3V0aWxzLnB5) | 60.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/45440?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #45440 +/- ##
==========================================
- Coverage 65.17% 65.04% -0.13%
==========================================
Files 3048 3048
Lines 199705 199709 +4
Branches 46596 46596
==========================================
- Hits 130148 129909 -239
- Misses 66627 66862 +235
- Partials 2930 2938 +8
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/45440/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/45440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.97% <20.00%> (-0.01%)` | :arrow_down: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/45440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/45440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `52.98% <60.00%> (-0.01%)` | :arrow_down: |
| [presto](https://app.codecov.io/gh/apache/superset/pull/45440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `36.73% <20.00%> (-0.01%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/45440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.05% <60.00%> (-0.21%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/45440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/45440?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org