-
-
Notifications
You must be signed in to change notification settings - Fork 138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Option to not remove comments on scrub! #86
Comments
Just adding # do not escape COMMENT_NODE
require 'loofah/scrubber'
module Loofah
class Scrubber
private
def html5lib_sanitize node
case node.type
when Nokogiri::XML::Node::ELEMENT_NODE
if HTML5::Scrub.allowed_element? node.name
HTML5::Scrub.scrub_attributes node
return Scrubber::CONTINUE
end
when Nokogiri::XML::Node::TEXT_NODE, Nokogiri::XML::Node::CDATA_SECTION_NODE,Nokogiri::XML::Node::COMMENT_NODE
return Scrubber::CONTINUE
end
Scrubber::STOP
end
end
end |
Hi! Thanks for asking this question. Can you tell me a little bit more about your use case? What do you want to do that Loofah's not currently allowing you to do? Can you share the code snippet and test case that tells me what you'd like to do? |
Loofah is removing HTML comments, which I find useful and always safe to have, for example: <html>
<body>
<!-- my comment about something -->
</body>
</html> The above comment is removed by Loofah. |
Another usecase would be (cringes...) IE conditional comments. |
@Qqwy the reason that comments are scrubbed is precisely what you suggest; that there could be unescaped HTML and/or javascript in the comment that would be executed by the browser. Given that, I guess we could recursively escape the contents of the comment, similar to what we do with the contents of a Would love to hear what Loofah and Rails users think. |
It looks like there's not interest in addressing this concern, so I'm closing this issue. |
Is there a way to NOT remove comments on
scrub!
?The text was updated successfully, but these errors were encountered: