rusackas commented on PR #43504: URL: https://github.com/apache/superset/pull/43504#issuecomment-5593982779
Thanks for the refactor to quiet ruff, @n4uu. The two things flagged earlier are still open though. `_svg_to_data_uri` only guards against falsy values, not non-string ones, so a malformed `brandSpinnerSvg` (say, a bool or int from a bad theme record) would still blow up with an AttributeError on `.encode()`. Worth the isinstance check codeant/sadpandajoe suggested. Also still no test for the actual security fix, something that feeds a malicious `brandSpinnerSvg` through and asserts it comes out as a data-URI `<img>` rather than inline SVG. For an XSS-containment change I'd really like that regression coverage before this merges. Not sure if I can push fixes to your fork, but I'll see if I can... -- 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]
