rails/rails-html-sanitizer

Stripping of comments

fschwahn opened this issue · 3 comments

fschwahn commented

I just ran into an issue where something was removed which was unexpected. I also found a test, but this test is rather confusing: test_strip_comments. It contains this assertion:

assert_equal "This is ", full_sanitize("This is <-- not\n a comment here.")

This assertion was different, but was changed 3 years ago, without explanation why this behaviour changed.

assert_equal "This is <-- not\n a comment here.", full_sanitize("This is <-- not\n a comment here.")

I think the old assertion is what is expected, not that everything after <-- is stripped.

This came up because I ran simple_format on user input, and everything after the user entered <- was removed.

rafaelfranca commented

Nokogiri interpret any non closed tag as a tag. The behavior changed because we are using Nokogiri and libxml to sanitize the input now. We can't change this behavior without opening the application to security vulnerabilities.

fschwahn commented

Thanks, that makes sense. Would you consider a PR where I rename that test to test_remove_unclosed_tag? I think that would be a better description of what is tested.

rafaelfranca commented

Great idea. Sure, could you send the PR. Thanks