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 useAutoClosingTag option #469

Conversation

dev-boku
Copy link

@dev-boku dev-boku commented Apr 7, 2021

Summary

  • I suggest the useAutoClosingTag option to fix the git issue below.

Reference

Copy link
Contributor

@abea abea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@potter7050 Thanks for the contribution. On a style note, I would suggest calling this option autoCloseTags. I would also ask for documentation of the new feature in the README file.

The bigger issue is that setting useAutoCloseTag: false does not only disable closing unclosed tags. It also fails to close tags that are already closed in the markup. See this test:

    const wackyText = sanitizeHtml('<p>here\'s a string with a <wacky> tag.</p>', {
      useAutoClosingTag: false,
      disallowedTagsMode: 'escape'
    });
    assert.equal(wackyText, '<p>here\'s a string with a &lt;wacky&gt; tag.</p>');

This fails with output <p>here's a string with a &lt;wacky&gt; tag..

Here we have a closed paragraph tag with an unclosed fake tag. The fake tag isn't closed, but the paragraph tag isn't closed either. If we were to call this closeTags: false, this would no longer be serving to sanitize HTML.

A fix for the original problem would need to

  1. Recognize that a tag is not recognized (disallowed). If recognized, there should be no change.
  2. Identify if the unrecognized tag is already closed in the input. If it is already closed, there should be no change.
  3. Finally, if the tag is not recognized and it is not already close, then it can skip the closing tag.

As I mentioned in #464, I think the difficulty here is that htmlparser2 automatically tries to close these tags. It doesn't provide a signal (that I'm aware of) that the tag wasn't already closed.

@stale
Copy link

stale bot commented Jun 20, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 20, 2021
@abea abea self-assigned this Jun 21, 2021
@stale stale bot removed the stale label Jun 21, 2021
@stale
Copy link

stale bot commented Aug 21, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added stale and removed stale labels Aug 21, 2021
@abea abea closed this Aug 23, 2021
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.

None yet

2 participants