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

Prevent converting greater than sign (>) to > within liquid templates #465

Closed amit777 closed 3 years ago

amit777 commented 3 years ago

The problem to solve

We allow customers to save liquid templates within our system. After the save the template, we run it through sanitize-html. However, liquid comparison operators like greater than and less-than get converted to HTML entities. Here is an example before and after snippet where I want to see if a string length is greater than 5: Before Save

<p> if name.length &gt; 5 = {% if name.length > 5 %} TRUE {% endif %} </p>

After Save

<p> if name.length &gt; 5 = {% if name.length &gt; 5 %} TRUE {% endif %} </p>

Is there a built in way to prevent the conversion of the > to > in this context?

abea commented 3 years ago

The one thought I had would be to use the parser option to set decodeEntities: false. that parser passes options down into htmlparser2.

But we might change these anyway. @boutell Do you know why we convert those angle brackets here even if decodeEntities: false? https://github.com/apostrophecms/sanitize-html/blob/main/index.js#L551

boutell commented 3 years ago

We really should either deprecate "decodeEntities: false" or replicate our entire test suite for exhaustive testing that it doesn't break the sanitization. This module passes "decodeEntities: true" and was validated with that in mind.

On Tue, Mar 9, 2021 at 1:05 PM Alex Bea @.***> wrote:

The one thought I had would be to use the parser option to set decodeEntities: false. that parser passes options down into htmlparser2.

But we might change these anyway. @boutell https://github.com/boutell Do you know why we convert those angle brackets here even if decodeEntities: false? https://github.com/apostrophecms/sanitize-html/blob/main/index.js#L551

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/sanitize-html/issues/465#issuecomment-794244969, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27LM57TEYXMJTBHOXADTCZIMJANCNFSM4YDRHUQQ .

--

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

boutell commented 3 years ago

(Despite my use of the word "exhaustive" here, which probably sounds intimidating, it wouldn't be a huge deal to copy the tests and give it a try to see how wise or unwise it is to attempt to claim we can safely handle decodeEntities: false.)

On Mon, Mar 15, 2021 at 8:16 AM Tom Boutell @.***> wrote:

We really should either deprecate "decodeEntities: false" or replicate our entire test suite for exhaustive testing that it doesn't break the sanitization. This module passes "decodeEntities: true" and was validated with that in mind.

On Tue, Mar 9, 2021 at 1:05 PM Alex Bea @.***> wrote:

The one thought I had would be to use the parser option to set decodeEntities: false. that parser passes options down into htmlparser2.

But we might change these anyway. @boutell https://github.com/boutell Do you know why we convert those angle brackets here even if decodeEntities: false? https://github.com/apostrophecms/sanitize-html/blob/main/index.js#L551

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/sanitize-html/issues/465#issuecomment-794244969, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27LM57TEYXMJTBHOXADTCZIMJANCNFSM4YDRHUQQ .

--

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

--

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

stale[bot] commented 3 years 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.