fix: set charset via content_type to avoid malformed Content-Type headers#40658
fix: set charset via content_type to avoid malformed Content-Type headers#40658rusackas wants to merge 1 commit into
Conversation
Code Review Agent Run #8ca439Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
This question isn’t related to the pull request. I can only help with questions about the PR’s code or comments. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40658 +/- ##
=======================================
Coverage 64.08% 64.08%
=======================================
Files 2663 2663
Lines 143289 143289
Branches 32952 32952
=======================================
Hits 91832 91832
Misses 49871 49871
Partials 1586 1586
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rusackas
requested review from
Antonio-RiveroMartnez,
amaannawab923,
dpgaspar and
sha174n
There was a problem hiding this comment.
Pull request overview
This PR standardizes and corrects Content-Type header construction in Superset responses to avoid malformed/doubled charset parameters and to consistently include charset=utf-8 for JSON responses.
Changes:
- Switch streaming CSV export to use
Response(content_type=...)instead ofmimetype=...to avoid Werkzeug producing doubledcharsetparameters. - Update
json_successandBaseSupersetView.json_responseto returnapplication/json; charset=utf-8. - Update an integration test assertion to match the new JSON
Content-Type.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
superset/charts/data/api.py |
Uses content_type for streaming CSV export to prevent malformed Content-Type headers. |
superset/views/base.py |
Sets JSON responses to application/json; charset=utf-8 via content_type. |
tests/integration_tests/charts/api_tests.py |
Updates expected JSON Content-Type in an integration test. |
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #d9035e
Actionable Suggestions - 1
-
superset/charts/data/api.py - 1
- CWE-73: Filename Sanitization Correct · Line 620-620
Additional Suggestions - 1
-
superset/charts/data/api.py - 1
-
DATA_MANIPULATION: Unused parameter · Line 620-621`expected_rows` is extracted at line 624-629 and logged at line 734, but never passed to or used by `StreamingCSVExportCommand`. Verify if this is intentional (informational logging only) or if it should be plumbed through to the command for future features like progress estimation.
Code suggestion
--- a/superset/charts/data/api.py +++ b/superset/charts/data/api.py @@ -623,8 +623,11 @@ class ChartDataRestApi(BaseChartDataRestApi): expected_rows = None if expected_rows_str := request.form.get("expected_rows"): try: expected_rows = int(expected_rows_str) - logger.info("FRONTEND PROVIDED EXPECTED ROWS: %d", expected_rows) + if expected_rows > 0: + logger.info("FRONTEND PROVIDED EXPECTED ROWS: %d", expected_rows) + else: + expected_rows = None # Ignore non-positive values except (ValueError, TypeError): logger.warning("Invalid expected_rows value: %s", expected_rows_str)
-
Review Details
-
Files reviewed - 1 · Commit Range:
49e590d..2b32dc7- superset/charts/data/api.py
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
…ders
Two Content-Type construction issues:
- The streaming CSV export passed mimetype=f"text/csv; charset={encoding}" to
Flask's Response. Werkzeug appends a default charset to the mimetype, so a
charset embedded there produces a doubled, malformed header
(text/csv; charset=utf-8; charset=utf-8). Use content_type= instead, which is
taken verbatim.
- json_success and BaseSupersetView.json_response returned application/json
without a charset, inconsistent with _send_chart_response. Set
content_type="application/json; charset=utf-8".
Update the form_data content-type assertion accordingly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
rusackas
force-pushed
the
fix/response-content-type-charset
branch
from
2b32dc7 to
be4d2ae
Compare
Code Review Agent Run #6168d8Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
SUMMARY
Two Content-Type construction issues (CWE-116 / ASVS 4.1.1):
superset/charts/data/api.py) passedmimetype=f"text/csv; charset={encoding}"to Flask'sResponse. Werkzeug appends its own default charset to themimetypevalue, producing a doubled, malformed header —Content-Type: text/csv; charset=utf-8; charset=utf-8. Switched tocontent_type=, which is used verbatim. (Reproduced on the pinned Werkzeug 3.x.)json_successandBaseSupersetView.json_response(superset/views/base.py) returnedapplication/jsonwith no charset, inconsistent with_send_chart_response. Setcontent_type="application/json; charset=utf-8".Updated the one test that asserted the exact
application/jsoncontent type.(Note: the related FINDING-024 —
X-Content-Type-Options: nosniffon chart screenshots — is not included: Flask-Talisman already sets that header globally, so it's a false positive.)TESTING INSTRUCTIONS
ADDITIONAL INFORMATION
🤖 Generated with Claude Code