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.84k stars 353 forks source link

<sup> tag doesn't work with sanitizeHtml #595

Closed samilieberman closed 1 year ago

samilieberman commented 1 year ago

PLEASE NOTE: make sure the bug exists in the latest patch level of the project. For instance, if you are running a 2.x version of Apostrophe, you should use the latest in that major version to confirm the bug.

To Reproduce

  const formattedTitle = 'My Company<sup>&#174;</sup>';
  console.log(sanitizeHtml(formattedTitle));

The superscript tag doesn't work, even though it is in the default tags

Expected behavior

A clear and concise description of what you expected to happen.

The symbol should be a superscript

Describe the bug

A clear and concise description of what the bug is.

The superscript tag doesn't work, even though it is in the default tags

Details

The superscript tag doesn't work, even though it is in the default tags

Version of Node.js: PLEASE NOTE: Only stable LTS versions (10.x and 12.x) are fully supported but we will do our best with newer versions. Node 16

Server Operating System: The server (which might be your dev laptop) on which Apostrophe is running. Linux? MacOS X? Windows? Is Docker involved? Mac OS X

Additional context:

Add any other context about the problem here. If the problem is specific to a browser, OS or mobile device, specify which.

Screenshots If applicable, add screenshots to help explain your problem.

Screen Shot 2022-12-16 at 4 10 27 PM
boutell commented 1 year ago

Please provide a unit test that fails (see the tests folder). If this does not fail when written as a unit test then something is happening in your larger application that is unrelated to sanitize-html itself.

On Fri, Dec 16, 2022 at 4:11 PM Sami Lieberman @.***> wrote:

PLEASE NOTE: make sure the bug exists in the latest patch level of the project. For instance, if you are running a 2.x version of Apostrophe, you should use the latest in that major version to confirm the bug. To Reproduce

const formattedTitle = 'My Company®'; console.log(sanitizeHtml(formattedTitle));

[image: Screen Shot 2022-12-16 at 4 10 27 PM] https://user-images.githubusercontent.com/15946916/208189541-baf52788-da32-4989-848c-823950d6b46a.png

The superscript tag doesn't work, even though it is in the default tags Expected behavior

A clear and concise description of what you expected to happen. The symbol should be a superscript

Describe the bug

A clear and concise description of what the bug is.

Details

Version of Node.js: PLEASE NOTE: Only stable LTS versions (10.x and 12.x) are fully supported but we will do our best with newer versions.

Server Operating System: The server (which might be your dev laptop) on which Apostrophe is running. Linux? MacOS X? Windows? Is Docker involved?

Additional context:

Add any other context about the problem here. If the problem is specific to a browser, OS or mobile device, specify which.

Screenshots If applicable, add screenshots to help explain your problem.

— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/sanitize-html/issues/595, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27JYD6EUDDKOF4MVNN3WNTLIRANCNFSM6AAAAAATBQIKOQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

joetjengerdes commented 1 year ago

I attempted to reproduce the reported issue but couldn't confirm it. The provided test, which checks for the removal of <sup> tags, did not yield the expected result. The <sup> tags were retained, including the registered trademark symbol. Based on my findings, I believe this issue can be closed.

it('should not remove sup tags as they are in default allowed list', function() {
  assert.equal(sanitizeHtml('My Company<sup>&#174;</sup>'), 'My Company<sup>®</sup>');
});

Please consider reviewing the test and closing the issue accordingly.

boutell commented 1 year ago

Closing as the issue could not be reproduced. @samilieberman if you think we're missing something feel free to provide a unit test (see test/test.js).

samilieberman commented 1 year ago

@boutell - We are using another library as a workaround because this is still not working for us. I don't have the bandwidth to look into this at the moment, but my colleague Ganesh is going to pick it up. Thanks!