Skip to content

Commit

Permalink
Use Loofah to scrub HTML attachments
Browse files Browse the repository at this point in the history
`:prune` removes unknown/unsafe tags and their contents (including their
subtrees):

    unsafe_html = "ohai! <div>div is safe</div> <foo>but foo is
    <b>not</b></foo>"

    Loofah.fragment(unsafe_html).scrub!(:prune)
    # => "ohai! <div>div is safe</div> "

* Adds a `DOCTYPE` to the fixture file so that Nokogiri doesn't insert a
  HTML 4 `DOCTYPE` automatically, making comparison in the spec uglier
* Nokogiri also adds a `meta` tag to the output. Not much we can do
  about this: sparklemotion/nokogiri#1008
  • Loading branch information
garethrees committed Aug 3, 2017
1 parent f0665ba commit d20a309
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 4 deletions.
5 changes: 4 additions & 1 deletion app/controllers/request_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,10 @@ def get_attachment
apply_masks(@attachment.default_body, @attachment.content_type)

if content_type == 'text/html'
body = ActionController::Base.helpers.sanitize(body)
body =
Loofah.scrub_document(body, :prune).
to_html(encoding: 'UTF-8').
try(:html_safe)
end

render :body => body, :content_type => content_type
Expand Down
18 changes: 15 additions & 3 deletions spec/controllers/request_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -604,9 +604,21 @@ def get_attachment(params = {})
:part => 2,
:file_name => 'interesting.html',
:skip_cache => 1
expected =
"<html>\n <head>\n </head>\n <body>dull\n \n </body>\n</html>\n"
expect(response.body).to eq(expected)

# Nokogiri adds the meta tag; see
# https://github.com/sparklemotion/nokogiri/issues/1008
expected = <<-EOF.squish
<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>dull
</body>
</html>
EOF

expect(response.body.squish).to eq(expected)
end

it "censors attachments downloaded directly" do
Expand Down
1 change: 1 addition & 0 deletions spec/fixtures/files/interesting.html
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
<!DOCTYPE html>
<html>
<head>
</head>
Expand Down

0 comments on commit d20a309

Please sign in to comment.