fix(jinja): ensure consistent escaping in url_param helper

[l3tchupkt]

SUMMARY

Ensure consistent handling of parameters in the url_param() helper by applying escaping logic uniformly regardless of input source.

Previously, values obtained from request arguments and form data were processed through different code paths, leading to inconsistent behavior. This change consolidates the logic so that all inputs follow the same processing and escaping flow when enabled, while preserving the existing precedence of request arguments over form data.

---

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not applicable.

---

TESTING INSTRUCTIONS

1. Enable template processing in configuration:
   FEATURE_FLAGS = {
   "ENABLE_TEMPLATE_PROCESSING": True,
   }

2. Create or use a dataset with a query like:
   SELECT * FROM logs WHERE source = '{{ url_param("source") }}'

3. Send a request with a parameter via query string:
   /api/v1/chart/data?source=test_value

4. Verify that the parameter is processed correctly and queries execute as expected.

5. Repeat the same using form_data["url_params"] and confirm consistent behavior.

6. Test edge cases:

   • Empty value (?source=)
   • Numeric values (?source=0)
   • Missing parameter (default fallback)

7. Ensure no regressions in existing functionality using url_param().

---

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags: ENABLE_TEMPLATE_PROCESSING
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • 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

---

Author: Lakshmikanthan K (letchupkt)
LinkedIn: linkedin.com/in/lakshmikanthank

[bito-code-review]

Code Review Agent Run #b807a0

Actionable Suggestions - 0

Review Details

• Files reviewed - 1 · Commit Range: 1ca8d02..7d05e68

  • superset/jinja_context.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

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[hainenber]

@l3tchupkt thanks for opening out, can you help adding a unit test?

Edit: also, can you link to an existing issue?

[l3tchupkt]

@hainenber Thanks for the feedback!

I’ll add a unit test to cover the behavior of url_param() for both request arguments and form data, including edge cases.

Regarding the issue reference , this change addresses a reported issue currently being tracked by the security team, so I’ll avoid linking a public issue for now.

Please let me know if you'd prefer a placeholder reference or any specific test coverage.

[l3tchupkt]

@hainenber Hi, I’ve added additional test coverage to validate the fix more thoroughly.

The new tests focus on edge cases around url_param, especially ensuring consistent behavior between query_string and form_data, proper precedence handling, and correct escaping/unescaping of values containing quotes.

I also included a specific test to confirm that query parameters overriding form_data still respect the escape_result=False flag, ensuring there’s no inconsistency in how inputs are processed.

All tests are passing locally, and the changes do not impact existing functionality.

Please let me know if you’d like me to extend coverage further or adjust anything.

[l3tchupkt]

@hainenber could you please check this ??

[hainenber]

I'm cross-checking this issue with other maintainers.

Re: your fix, it's logically sound but lets wait for the cross-check result.

[l3tchupkt]

@hainenber okey thanks for the update : )

[Copilot]

Pull request overview

This PR fixes inconsistent escaping behavior in the Jinja url_param() helper by ensuring values pulled from the URL query string go through the same escaping flow as values coming from form_data["url_params"] (while preserving query-string precedence).

Changes:

• Consolidate url_param() value resolution so query-string values no longer bypass escape_result.
• Apply escaping uniformly for both query-string and form_data["url_params"] sources when escape_result=True and a dialect is available.
• Add unit tests covering escaped/unescaped query-string behavior, falsy/empty query values, and query precedence over form_data.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
superset/jinja_context.py	Unifies url_param() value resolution and ensures consistent escaping for query-string and form-data sourced params.
tests/unit_tests/jinja_context_test.py	Adds targeted unit tests for query-string escaping, precedence, and edge cases (empty and falsy values).

[l3tchupkt]

@dpgaspar Hi, could you please review this when you have a moment?