apostrophecms / sanitize-html

Clean up user-submitted HTML, preserving whitelisted elements and whitelisted attributes on a per-element basis. Built on htmlparser2 for speed and tolerance
MIT License
3.68k stars 349 forks source link

Unclosed tag in escape mode is removed #603

Closed kiprobinsonknack closed 1 year ago

kiprobinsonknack commented 1 year ago

To Reproduce

const sanitizeHtml = require('sanitize-html');

// example when tag is an allowed, non-self-closing tag
const sanitized1 = sanitizeHtml('a<b', { disallowedTagsMode: 'escape' });
console.log(sanitized1); // prints "a", i expect "a&lt;b"

// example when tag is a non-allowed tag
const sanitized2 = sanitizeHtml('b<bogus', { disallowedTagsMode: 'escape' });
console.log(sanitized2); // prints "b", i expect "b&lt;bogus"

// example when tag is an allowed, self-closing tag
const sanitized3 = sanitizeHtml('c<br', { disallowedTagsMode: 'escape' });
console.log(sanitized3); // prints "c", i expect "a&lt;b"

Expected behavior

Mentioned in comments above, I expect output to be:

a&lt;b
b&lt;bogus
c&lt;br

But in fact I see:

a
b
c

Describe the bug

Basically, I'm expecting malformed tags to be escaped in escape mode.

Details

Version of Node.js: 18.12.0

Server Operating System: MacOSX Ventura 13.0.1

Additional context: n/a

Screenshots n/a

boutell commented 1 year ago

It's not a tag, it's just junk that wishes it were a tag 😄 So I don't think this is a bug. I appreciate why you would prefer that it happen this way. I think that could make a good PR if you're able to make that happen with the stream of information we're getting from htmlparser2.

stale[bot] commented 1 year ago

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.

batrudinych commented 1 year ago

@boutell

I would appreciate if you could reconsider.

>  sanitizeHtml('<p><3 Linux</p>', { allowedTags: [], allowedAttributes: {} })
''

to me it seems strange, doesn't it?

upd: worked on the latest lib version. My apologies

stale[bot] commented 1 year ago

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.