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

Behaviour changed for numbers given to sanitizeHtml #593

Closed alexander-schranz closed 1 year ago

alexander-schranz commented 1 year ago

Why I could fix my previous issue #592 I still found a maybe breaking change in the update from 2.7.3 -> 2.8.0 which I think is related to the htmlparser2 update.

To Reproduce

We are using the following code:

const sanitizedHtml = sanitizeHtml(value, {
    allowedTags: ['b', 'em', 'i', 's', 'small', 'strong', 'sub', 'sup', 'time', 'u'],
    allowedAttributes: {},
    disallowedTagsMode: 'recursiveEscape',
});

return sanitizedHtml;

If the value is a integer / number in 2.7.3 the result was '5' but now the result is '' so it seems the new htmlparser2 version seems to strip away all none strings given into it.

Expected behavior

Still return '5'.

Describe the bug

Given a number into it fails now.

Details

Version of Node.js: 14

Server Operating System: MacOS

Additional context:

Build with webpack 4.

Screenshots

Bildschirmfoto 2022-12-13 um 15 01 32

Workaound

Do a value.toString() before given the value to sanitizeHtml function.

boutell commented 1 year ago

Numbers aren't strings and HTML clearly should be a string, so I wouldn't call this a bug, more of an undefined behavior. However I'd accept a PR to correct it in the case of numbers.

alexander-schranz commented 1 year ago

@boutell https://github.com/apostrophecms/sanitize-html/pull/594 :)