Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ADD_NAMESPACES option to support strict XML documents #729

Merged
merged 4 commits into from
Nov 10, 2022

Conversation

kevin-deyoungster
Copy link
Contributor

@kevin-deyoungster kevin-deyoungster commented Nov 4, 2022

This pull request implements feature #728: Support additional namespaces in XML documents

Background & Context

DOMPurify cannot sanitize XML documents with custom namespaces. This PR introduces a CONFIG option ADD_NAMESPACES to enable users to allowlist custom namespaces to consider during sanitization.

Sample Usage

DOMPurify.sanitize(dirty, {
    PARSER_MEDIA_TYPE: "application/xhtml+xml",
    ADD_TAGS: [],
    ADD_ATTR: [],
    ADD_NAMESPACES: []
})

Tasks

  • Add ADD_NAMESPACES config parameter
  • Confirm from reviewers if feature can be supported
  • Refactor
  • Add to test suite: "Config-Flag tests: ADD_NAMESPACES"

@cure53
Copy link
Owner

cure53 commented Nov 6, 2022

Hey there, thanks for the PR, this looks generally good and I think supporting the feature as proposed makes sense!

@kevin-deyoungster kevin-deyoungster changed the title Add ADD_NAMESPACES option to support strict XHTML documents Add ADD_NAMESPACES option to support strict XML documents Nov 6, 2022
@kevin-deyoungster
Copy link
Contributor Author

@cure53, could you kindly review, thanks!

@kevin-deyoungster
Copy link
Contributor Author

kevin-deyoungster commented Nov 9, 2022

@cure53, awesome.

  1. Could you add me as contrib. so I can merge the PR in?
    I don't have write permissions to this repo to trigger the merge.

  2. Would I need to update type definitions in https://github.com/DefinitelyTyped/DefinitelyTyped/blob/15ecc0968b3e6eaa0bc9cd2377afa547a7f98653/types/dompurify/index.d.ts as well?

@cure53 cure53 merged commit 5f77429 into cure53:main Nov 10, 2022
@cure53
Copy link
Owner

cure53 commented Nov 10, 2022

@kevin-deyoungster Awesome, thanks :)

I just did the merge and yes, it's likely necessary to send a small PR to the DT folks, thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants