fix(jinja): handle non-JSON body with JSON content-type in get_form_data#42196
Conversation
request.is_json only inspects the Content-Type header, not whether the
body is actually parseable JSON. When a request context has that shape
(as happens when an MCP tool call renders a filter_values() template),
request.get_json() raises an uncaught werkzeug.exceptions.BadRequest
that propagates through Jinja rendering. Fall back to {} instead, the
same way loads_request_json() already does for the request.form and
request.args paths.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #42196 +/- ##
==========================================
- Coverage 65.19% 65.19% -0.01%
==========================================
Files 2767 2767
Lines 156062 156064 +2
Branches 35717 35715 -2
==========================================
+ Hits 101741 101742 +1
- Misses 52361 52362 +1
Partials 1960 1960
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Code Review Agent Run #63474fActionable 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 |
1 participant
SUMMARY
get_form_data()crashed with an unhandledwerkzeug.exceptions.BadRequestwhenever it ran inside a request context where theContent-Typeheader claimed JSON but the body wasn't actually parseable JSON. This happens for any caller that reachesget_form_data()outside a normal chart-data POST — for example thefilter_values()/get_filters()Jinja macros used in chart and dataset SQL templates, when rendered from a request context that isn't a real chart-data request.request.is_jsononly inspects theContent-Typeheader; it does not guarantee the body is valid, non-empty JSON.get_form_data()already has a helper,loads_request_json(), that gracefully falls back to{}on a JSON parse failure — but that guard was only applied to therequest.form/request.argsfallback paths, not to the primaryrequest.get_json(cache=True)call, which is the one that was actually raising.This PR extracts the JSON-body parsing into a small
get_request_json_body()helper that catchesBadRequestand falls back to{}, matching the existingloads_request_json()behavior.Internal tracking: SC-114072
TESTING INSTRUCTIONS
tests/unit_tests/views/test_utils.py::test_get_form_data_handles_non_json_body_with_json_content_type, which builds a request context withContent-Type: application/jsonand a non-JSON body, and assertsget_form_data()returns({}, None)instead of raising.pytest tests/unit_tests/views/test_utils.py tests/unit_tests/jinja_context_test.py -q— 104 passed.master(request.get_json(cache=True)raisingBadRequest) and confirmed the fix resolves it viaapp.test_request_context(data="not-json-at-all", content_type="application/json").ADDITIONAL INFORMATION