fix(sanitization): scope allowlist to the rich content path#31245
Open
thetaPC wants to merge 2 commits into
Open
fix(sanitization): scope allowlist to the rich content path#31245thetaPC wants to merge 2 commits into
thetaPC wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue number: N/A
What is the current behavior?
The attribute allowlist in
core/src/utils/sanitizationis shared betweensanitizeDOMStringandsanitizeDOMTree. The rich content work broadened it from 6 attributes to the full component and SVG set plusaria-*anddata-*. BecausesanitizeDOMStringis also used byion-toast,ion-loading, theion-alertmessage,ion-refresher-content, andion-infinite-scroll-content, those components keep attributes that were previously stripped wheninnerHTMLTemplatesEnabledis on.renderClonedContentis the only place that sanitizes content for callers that pass anHTMLElementstraight torenderOptionLabel, but the surroundingcloneToVNodecomment implies sanitization always happens upstream, so the call looks redundant and could be removed.The
ion-select-optiondisabledJSDoc states the property does not apply tointerface="action-sheet", which is no longer accurate.What is the new behavior?
sanitizeDOMTree(the select rich content path).sanitizeDOMStringreverts to its original narrow allowlist, so sanitization output is unchanged for toast, loading, the alert message, refresher content, and infinite scroll content. The shared dangerous vector scrubbing (style removal, script scheme checks,data:URI filtering) still applies to both paths.sanitizeDOMStringstrips attributes that are only allowed on the rich content path.sanitizeDOMTreecall inrenderClonedContentexplaining it is the only sanitization pass for directHTMLElementcallers and must not be removed.ion-select-optiondisabledprop JSDoc, since action sheet options now honordisabled.Does this introduce a breaking change?
Other information
Follow-up to the select rich content work (FW-7599) addressing review feedback. The allowlist change restores the prior sanitization behavior for the non-select consumers rather than changing it, so there is no user facing behavior change for those components.