rusackas commented on code in PR #40660:
URL: https://github.com/apache/superset/pull/40660#discussion_r3338260515
##########
superset/views/core.py:
##########
@@ -864,12 +864,11 @@ def dashboard_permalink(
)
if url_params := state.get("urlParams"):
for param_key, param_val in url_params:
- if param_key == "native_filters":
- # native_filters doesnt need to be encoded here
- url = f"{url}&native_filters={param_val}"
- else:
- params = parse.urlencode([(param_key, param_val)])
- url = f"{url}&{params}"
+ # URL-encode every param value (including native_filters) so a
+ # value containing '&'/'#'/'=' cannot inject extra parameters
+ # into the redirect target. Flask decodes the value back on
read.
+ params = parse.urlencode([(param_key, param_val)])
+ url = f"{url}&{params}"
Review Comment:
Good call on the test coverage gap. The existing integration test suite for
permalink redirects is in `tests/integration_tests/core_tests.py`. Adding a
parametrized test covering reserved characters in `native_filters` values is a
worthwhile follow-up, but out of scope for this focused fix. Will track
separately.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
