Skip to content

Add ADD_NAMESPACES option to support strict XML documents#729

Merged
cure53 merged 4 commits intocure53:mainfrom
kevin-deyoungster:kdeyoungster/xml
Nov 10, 2022
Merged

Add ADD_NAMESPACES option to support strict XML documents#729
cure53 merged 4 commits intocure53:mainfrom
kevin-deyoungster:kdeyoungster/xml

Conversation

@kevin-deyoungster
Copy link
Copy Markdown
Contributor

@kevin-deyoungster kevin-deyoungster commented Nov 4, 2022 •
edited
Loading

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
Copy Markdown
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
cosmincatrinescu
cosmincatrinescu reviewed Nov 9, 2022
View reviewed changes
@kevin-deyoungster
Copy link
Copy Markdown
Contributor Author

kevin-deyoungster commented Nov 9, 2022

@cure53, could you kindly review, thanks!

@kevin-deyoungster
Copy link
Copy Markdown
Contributor Author

kevin-deyoungster commented Nov 9, 2022 •
edited
Loading

@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
Copy Markdown
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

Reviewers

@cure53 cure53 cure53 approved these changes

+1 more reviewer

@cosmincatrinescu cosmincatrinescu cosmincatrinescu left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kevin-deyoungster @cure53 @cosmincatrinescu @tosmolka