sadpandajoe commented on code in PR #45575:
URL: https://github.com/apache/superset/pull/45575#discussion_r4363273424
##########
superset/mcp_service/utils/sanitization.py:
##########
@@ -305,6 +339,19 @@ def sanitize_user_input(
value = _strip_html_tags(value)
value = _remove_dangerous_unicode(value)
+ # Reject rather than store tag-shaped text: a later sanitizer pass would
+ # decode and strip it, so the stored value would not be stable.
+ _check_stable_html(value, field_name)
+
+ # Conservative bracket escaping can expand text even after tags are
removed.
+ # Enforce the storage cap for callers without a Pydantic Field length
limit.
+ if len(value) > max_length:
Review Comment:
The cap is checked on the tag-stripped value before the callers' final
`.strip()`, so whitespace that only exists because markup was removed still
counts toward the limit. For example, `chart_name = "<b> " + "A" * 250 + "
</b>"` strips to `" " + "A"*250 + " "` (252 chars), is rejected with
`string_too_long`, and yet would store as a valid 250-character name after
trimming; `duplicate_dashboard` with `"<b> " + "x" * 500 + " </b>"` fails the
same way at 500. Could the length check run after the stripped value is trimmed
so it matches the stored name? The docs sentence that says limits apply after
trimming would then be accurate too.
--
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]
