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

Documentation issue and question for custom elements #854

Closed
Judekeyser opened this issue Aug 27, 2023 · 4 comments
Closed

Documentation issue and question for custom elements #854

Judekeyser opened this issue Aug 27, 2023 · 4 comments

Comments

@Judekeyser
Copy link

Judekeyser commented Aug 27, 2023

I was trying to use the examples written in https://github.com/cure53/DOMPurify#control-behavior-relating-to-custom-elements and there is something I think is wrong documentation. The two last configuration look roughly the same, up to regex/predicate difference, yet they do not seem to return the same result (the is attribute is removed in the last snippet).

I was wondering if it shouldn't be instead

const clean = DOMPurify.sanitize(
    '<foo-bar baz="foobar" forbidden="true"></foo-bar><div is="foo-baz"></div>',
    {
        CUSTOM_ELEMENT_HANDLING: {
            tagNameCheck: /^foo-/, // allow all tags starting with "foo-"
            attributeNameCheck: /baz/, // allow all attributes containing "baz"
            allowCustomizedBuiltInElements: false, // customized built-ins are *not* allowed <<<<< HERE
        },
    }
); // <foo-bar baz="foobar"></foo-bar><div is=""></div>

const clean = DOMPurify.sanitize(
    '<foo-bar baz="foobar" forbidden="true"></foo-bar><div is="foo-baz"></div>',
    {
        CUSTOM_ELEMENT_HANDLING: {
            tagNameCheck: (tagName) => tagName.match(/^foo-/), // allow all tags starting with "foo-"
            attributeNameCheck: (attr) => attr.match(/baz/), // allow all containing "baz"
            allowCustomizedBuiltInElements: true, // allow customized built-ins
        },
    }
); // <foo-bar baz="foobar"></foo-bar><div is="foo-baz"></div>

I also notice that if the element foo-bar actually writes something (for example, it does this.textContent = "Hello" when an attribute changes), even in strict default mode, then the value of the text content is still in the DOM, but not the custom element. Is this behavior expected, or is it simply undefined behavior ?

@Judekeyser Judekeyser changed the title Documentation issue for custom elements Documentation issue and question for custom elements Aug 27, 2023
@cure53
Copy link
Owner

cure53 commented Aug 29, 2023

Hey there, thanks for filing this. I think you are right, either the result be different or the value should be set to false. From what I can see, this line needs changing, correct?

https://github.com/cure53/DOMPurify/blob/main/README.md?plain=1#L243

It should be this, correct?

<foo-bar baz="foobar"></foo-bar><div is="foo-baz"></div>

@Judekeyser
Copy link
Author

This is my guess too, based on what I observed.

For the second point, I couldn't find anywhere in the documentation if it is expected that the DOM content of the custom element will be removed, or only the custom element markup. From what I test, it feels like only the markup is gone and the content is still there (might it be a content generated by the element itself! for example, after getting triggered by an attribute change), but I do not know if this is a specified behavior, or if I cannot count on it.

@cure53
Copy link
Owner

cure53 commented Aug 29, 2023

This is my guess too, based on what I observed.

Cool, will fix then, thanks.

but I do not know if this is a specified behavior, or if I cannot count on it.

I think this is currently expected behavior: https://github.com/cure53/DOMPurify/blob/main/src/purify.js#L291

@Judekeyser
Copy link
Author

Excellent, thank you for the reference! I missed it previously.

@cure53 cure53 closed this as completed Aug 31, 2023
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

No branches or pull requests

2 participants