rusackas commented on PR #40855: URL: https://github.com/apache/superset/pull/40855#issuecomment-4652697388
I took a closer look at this from a security angle, since url_param feeding unescaped values into SQL is exactly the kind of thing worth double-checking. Conclusion: it's fully superseded by the already-merged #40633, with no remaining gap to harden. The only attacker-controllable source either PR changed is the `request.args` (query-string) path, where the old early `return` bypassed escaping. #40633 already routes that through the dialect `literal_processor` in `url_param` — identical to what this PR does. The `form_data["url_params"]` source, the `default` value, and the `add_to_cache_keys` path all flow through the same escape on master already. The added tests are name-variants of the ones #40633 shipped (`test_url_param_escaped_request_args` etc.). So there's no open injection vector left for this to close. Closing as redundant — thanks! -- 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]
