fix(dashboard): escape emoji in position_json before saving to prevent truncation#39737
Conversation
Code Review Agent Run #4ccbd8Actionable Suggestions - 0Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #39737 +/- ##
=======================================
Coverage 64.49% 64.49%
=======================================
Files 2566 2566
Lines 133960 133962 +2
Branches 31116 31117 +1
=======================================
+ Hits 86394 86396 +2
Misses 46071 46071
Partials 1495 1495
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:
|
|
The flagged issue is correct: the PR only adds emoji escaping for position_json in UpdateDashboardCommand.run, but the create path via POST /api/v1/dashboard lacks this normalization, risking MySQL utf8 truncation and broken JSON. To resolve, apply the same json.loads → json.dumps normalization in CreateDashboardCommand.run before calling DashboardDAO.create. superset/commands/dashboard/create.py |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
For creation, the positions value is already nested inside json_metadata and goes through the same dump process, so it's not needed |
Code Review Agent Run #929ad1Actionable 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 |
|
@justinpark PR #36332 is a red herring in the bug report. It introduced the specific frontend behavior that triggered the code path more visibly (saving a chart to a dashboard tab), but the raw-setattr path in |
|
Bito Automatic Review Skipped – PR Already Merged |
…t truncation (apache#39737) Co-authored-by: Michael S. Molina <michael.s.molina@gmail.com>
…t truncation (apache#39737) Co-authored-by: Michael S. Molina <michael.s.molina@gmail.com>
SUMMARY
MySQL's utf8 charset only supports up to 3-byte UTF-8 characters. Emoji (e.g. 📈) are 4-byte Unicode characters, so when position_json containing raw emoji bytes is written directly to the DB column, MySQL silently truncates the string at the emoji character — breaking the stored JSON and causing parse errors on reload.
Root cause(introduced from https://github.com/apache/superset/pull/36332/changes#diff-a1256efcca3d8954c815c26f94d13eb86abd3f52c67258071fed1b3ece4a5b00R440-R446) : The PUT /api/v1/dashboard/ endpoint stores position_json via DashboardDAO.update() → setattr(model, "position_json", value), passing the raw string directly without re-serialization. This means any 4-byte emoji sent by the client reaches the DB as-is.
By contrast, the json_metadata.positions path (used when saving via set_dash_metadata) was unaffected because it always passes through json.dumps(), which uses simplejson with ensure_ascii=True by default — escaping emoji to \uXXXX (ASCII-safe).
Fix: In UpdateDashboardCommand.run(), parse and re-serialize position_json through json.loads → json.dumps before passing it to DashboardDAO.update(). This ensures emoji are escaped to \uXXXX and safe for MySQL utf8 columns.
if position_json := self._properties.get("position_json"):
self._properties["position_json"] = json.dumps(json.loads(position_json))
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
or
pytest tests/integration_tests/dashboards/test_update_emoji.py -v
ADDITIONAL INFORMATION