AO3-6564 Reduce fields where images are shown (#4729)

* Add strip_images to banner

* AO3-6564 Bookmarker's notes

* AO3-6564 Pseud desc

* Abuse and support form description

* AO3-6564 Admin post comment

* AO3-6564 Add features (overkill?)

* AO3-6564 woof

* AO3-6564 doggo

* AO3-6564 Feedback

* AO3-6564 appease dog

* Strip images from emails (AdminPost only)

app/models/comment.rb

@@ -495,7 +495,7 @@ def mark_unhidden!
   end
 
   def sanitized_content
-    sanitize_field self, :comment_content
+    sanitize_field(self, :comment_content, strip_images: ultimate_parent.is_a?(AdminPost))
   end
   include Responder
 end

app/models/feedback_reporters/abuse_reporter.rb

@@ -30,6 +30,8 @@ def subject
   end
 
   def ticket_description
-    description.present? ? description.html_safe : "No comment submitted."
+    return "No comment submitted." if description.blank?
+
+    strip_images(description.html_safe)
   end
 end

app/models/feedback_reporters/support_reporter.rb

@@ -31,6 +31,8 @@ def subject
   end
 
   def ticket_description
-    description.present? ? description.html_safe : "No description submitted."
+    return "No description submitted." if description.blank?
+
+    strip_images(description.html_safe)
   end
 end

app/views/admin/banners/_banner.html.erb

@@ -2,6 +2,6 @@
 <% # don't forget to update layouts/banner! %>
 <div class="<%= admin_banner.banner_type %> announcement group">
   <blockquote class="userstuff">
-    <%=raw sanitize_field(admin_banner, :content) %>
+    <%= raw sanitize_field(admin_banner, :content, strip_images: true) %>
   </blockquote>
 </div>

app/views/bookmarks/_bookmark_blurb_short.html.erb

@@ -28,10 +28,10 @@
     </ul>
   <% end %>
   <!--notes-->
-  <% unless bookmark.bookmarker_notes.blank? %>
+  <% if bookmark.bookmarker_notes.present? %>
     <h6 class="landmark heading"><%= ts("Bookmark Notes:") %></h6>
     <blockquote class="userstuff summary">
-      <%=raw sanitize_field(bookmark, :bookmarker_notes) %>
+      <%= raw sanitize_field(bookmark, :bookmarker_notes, strip_images: true) %>
     </blockquote>
   <% end %>
   <!--actions-->

app/views/bookmarks/_bookmark_user_module.html.erb

@@ -45,10 +45,10 @@
     <% end %>
 
     <!--notes-->
-    <% unless bookmark.bookmarker_notes.blank? %>
+    <% if bookmark.bookmarker_notes.present? %>
       <h6 class="landmark heading"><%= ts('Bookmarker\'s Notes') %></h6>
       <blockquote class="userstuff notes">
-        <%=raw sanitize_field(bookmark, :bookmarker_notes) %>
+        <%= raw sanitize_field(bookmark, :bookmarker_notes, strip_images: true) %>
       </blockquote>
     <% end %>
     <% # end of information added by the bookmark owner %>

app/views/comments/_single_comment.html.erb

@@ -55,7 +55,9 @@
         <% if single_comment.hidden_by_admin? %>
           <p class="notice"><%= ts("This comment has been hidden by an admin.") %></p>
         <% end %>
-        <blockquote class="userstuff"><%=raw sanitize_field(single_comment, :comment_content) %></blockquote>
+        <blockquote class="userstuff">
+          <%= raw sanitize_field(single_comment, :comment_content, strip_images: single_comment.ultimate_parent.is_a?(AdminPost)) %>
+        </blockquote>
       <% end %>
       <% if single_comment.edited_at.present? %>
         <p class="edited datetime">

app/views/layouts/_banner.html.erb

@@ -1,25 +1,20 @@
-<% # BACK END this seems giant and messy and confusing, pls can we review? 
-   # FRONT END yes let us rewrite this
-%>
-<% unless current_user && current_user.try(:preference).try(:banner_seen) %>
-<% if @admin_banner && @admin_banner.active? %>
-<% unless current_user.nil? && session[:hide_banner] %>
-  <div class="<%= @admin_banner.banner_type %> announcement group" id="admin-banner">
-    <blockquote class="userstuff">
-      <%=raw sanitize_field(@admin_banner, :content) %>
-    </blockquote>
-    <% if current_user.nil? %>
-      <p class="submit">
-        <%= link_to "&times;".html_safe, current_path_with(hide_banner: true), :class => 'showme action', :title => ts("hide banner") %>
-      </p>
-    <% else %>
-      <%= form_tag end_banner_user_path(current_user), :method => :post, :remote => true do %>
+<% if @admin_banner&.active? %>
+  <% unless session[:hide_banner] || current_user&.preference&.banner_seen %>
+    <div class="<%= @admin_banner.banner_type %> announcement group" id="admin-banner">
+      <blockquote class="userstuff">
+        <%= raw sanitize_field(@admin_banner, :content, strip_images: true) %>
+      </blockquote>
+      <% if current_user.nil? %>
         <p class="submit">
-          <%= submit_tag "&times;".html_safe, :title => ts("hide banner") %>
+          <%= link_to "&times;".html_safe, current_path_with(hide_banner: true), class: "showme action", title: ts("hide banner") %>
         </p>
+      <% else %>
+        <%= form_tag end_banner_user_path(current_user), method: :post, remote: true do %>
+          <p class="submit">
+            <%= submit_tag "&times;".html_safe, title: ts("hide banner") %>
+          </p>
+        <% end %>
       <% end %>
-    <% end %>
-  </div>
-<% end %>
-<% end %>
+    </div>
+  <% end %>
 <% end %>

app/views/pseuds/_pseud_module.html.erb

@@ -15,8 +15,8 @@
     <p class="datetime"><%= date %></p>
   <% end %>
 </div>
-<% unless pseud.description.blank? %>
+<% if pseud.description.present? %>
   <blockquote class="userstuff">
-    <%=raw sanitize_field(pseud, :description) %>
+    <%= raw sanitize_field(pseud, :description, strip_images: true) %>
   </blockquote>
 <% end %>

app/views/share/_embed_meta_bookmark.erb

@@ -5,6 +5,6 @@
 <% end %>
 <% if bookmark.bookmarker_notes.present? %>
 <%= ts("Bookmarker's Notes: ").html_safe %>
-<%= raw(sanitize_field(bookmark, :bookmarker_notes)) %>
+<%= raw sanitize_field(bookmark, :bookmarker_notes, strip_images: true) %>
 <% end %>
 <% end %>

features/bookmarks/bookmark_create.feature

@@ -108,6 +108,18 @@ Scenario: extra commas in bookmark form (Issue 2284)
     And I press "Create"
   Then I should see "created"
 
+Scenario: Bookmark notes do not display images
+  Given I am logged in as "bookmarkuser"
+    And I post the work "Some Work"
+  When I follow "Bookmark"
+    And I fill in "Notes" with "Fantastic!<img src='http://example.com/icon.svg'>"
+    And I press "Create"
+    And all indexing jobs have been run
+  Then I should see "Bookmark was successfully created"
+  When I go to the bookmarks page
+  Then I should not see the image "src" text "http://example.com/icon.svg"
+    And I should see "Fantastic!"
+
 Scenario: bookmark added to moderated collection has flash notice only when not approved
   Given the following activated users exist
     | login      | password |

features/comments_and_kudos/add_comment.feature

@@ -133,7 +133,7 @@ Scenario: Comment threading, comment editing
       And I fill in "Comment" with "B's improved comment (edited)"
       And I press "Update"
     Then 0 emails should be delivered to "User_A"
-  
+
   Scenario: Try to post an invalid comment
 
     When I am logged in as "author"
@@ -180,6 +180,17 @@ Scenario: Set preference and receive comment notifications of your own comments
     And "commenter" should be emailed
     And 1 email should be delivered to "commenter"
 
+Scenario: Work comment displays images
+
+  Given the work "Generic Work"
+    And I am logged in as "commenter"
+    And I visit the new comment page for the work "Generic Work"
+  When I fill in "Comment" with "Fantastic!<img src='http://example.com/icon.svg'>"
+    And I press "Comment"
+  Then I should see "Comment created!"
+    And I should see "Fantastic!"
+    And I should see the image "src" text "http://example.com/icon.svg"
+
 Scenario: Try to post a comment with a < angle bracket before a linebreak, without a space before the bracket
 
     Given the work "Generic Work"
@@ -194,7 +205,7 @@ Scenario: Try to post a comment with a < angle bracket before a linebreak, witho
       And I press "Comment"
     Then I should see "Comment created!"
 
-Scenario: Try to post a comment with a < angle bracket before a linebreak, with a space before the bracket 
+Scenario: Try to post a comment with a < angle bracket before a linebreak, with a space before the bracket
 
     Given the work "Generic Work"
       And I am logged in as "commenter"

features/comments_and_kudos/comments_adminposts.feature

@@ -135,3 +135,14 @@ Feature: Commenting on admin posts
     When I follow "Edit Post"
     Then I should see "No one can comment"
     # TODO: Test that the other options aren't available/selected in a non-brittle way
+
+  Scenario: Admin post comment does not display images
+    Given I have posted an admin post
+      And I am logged in as "regular"
+      And I go to the admin-posts page
+      And I follow "Default Admin Post"
+    When I fill in "Comment" with "Hi!<img src='http://example.com/icon.svg'>"
+      And I press "Comment"
+    Then I should see "Comment created!"
+      And I should not see the image "src" text "http://example.com/icon.svg"
+      And I should see "Hi!"

features/other_a/pseuds.feature

@@ -118,6 +118,18 @@ Scenario: Manage pseuds - add, edit
     And I should see "I wanted to add another fancy name"
     And I should not see "My new name (editpseuds)"
 
+Scenario: Pseud descriptions do not display images
+
+  Given I am logged in as "myself"
+    And I go to my pseuds page
+    When I follow "Edit"
+    And I fill in "Description" with "Fantastic!<img src='http://example.com/icon.svg'>"
+    And I press "Update"
+  Then I should see "Pseud was successfully updated."
+  When I follow "Back To Pseuds"
+  Then I should not see the image "src" text "http://example.com/icon.svg"
+    And I should see "Fantastic!"
+
 Scenario: Comments reflect pseud changes immediately
 
   Given the work "Interesting"

lib/html_cleaner.rb

@@ -2,17 +2,21 @@
 module HtmlCleaner
   # If we aren't sure that this field hasn't been sanitized since the last sanitizer version,
   # we sanitize it before we allow it to pass through (and save it if possible).
-  def sanitize_field(object, fieldname)
+  def sanitize_field(object, fieldname, strip_images: false)
     return "" if object.send(fieldname).nil?
 
     sanitizer_version = object.try("#{fieldname}_sanitizer_version")
-    if sanitizer_version && sanitizer_version >= ArchiveConfig.SANITIZER_VERSION
-      # return the field without sanitizing
-      object.send(fieldname)
-    else
-      # no sanitizer version information, so re-sanitize
-      sanitize_value(fieldname, object.send(fieldname))
-    end
+    sanitized_field =
+      if sanitizer_version && sanitizer_version >= ArchiveConfig.SANITIZER_VERSION
+        # return the field without sanitizing
+        object.send(fieldname)
+      else
+        # no sanitizer version information, so re-sanitize
+        sanitize_value(fieldname, object.send(fieldname))
+      end
+
+    sanitized_field = strip_images(sanitized_field) if strip_images
+    sanitized_field
   end
 
   # yank out bad end-of-line characters and evil msword curly quotes

spec/mailers/admin_mailer_spec.rb

@@ -3,6 +3,48 @@
 require "spec_helper"
 
 describe AdminMailer do
+  describe "#comment_notification" do
+    let(:email) { described_class.comment_notification(comment.id) }
+
+    context "when the comment is on an admin post" do
+      let(:comment) { create(:comment, :on_admin_post) }
+
+      context "and the comment's contents contain an image" do
+        let(:image_tag) { "<img src=\"an_image.png\" />" }
+
+        before do
+          comment.comment_content += image_tag
+          comment.save!
+        end
+
+        it "strips the image from the email message" do
+          expect(email).not_to have_html_part_content(image_tag)
+        end
+      end
+    end
+  end
+
+  describe "#edited_comment_notification" do
+    let(:email) { described_class.edited_comment_notification(comment.id) }
+
+    context "when the comment is on an admin post" do
+      let(:comment) { create(:comment, :on_admin_post) }
+
+      context "and the comment's contents contain an image" do
+        let(:image_tag) { "<img src=\"an_image.png\" />" }
+
+        before do
+          comment.comment_content += image_tag
+          comment.save!
+        end
+
+        it "strips the image from the email message" do
+          expect(email).not_to have_html_part_content(image_tag)
+        end
+      end
+    end
+  end
+
   describe "send_spam_alert" do
     let(:spam_user) { create(:user) }
 

spec/mailers/comment_mailer_spec.rb

@@ -71,14 +71,34 @@
     end
   end
 
-  describe "comment_notification" do
+  shared_examples "strips image tags" do
+    let(:image_tag) { "<img src=\"an_image.png\" />" }
+
+    before do
+      comment.comment_content += image_tag
+      comment.save!
+    end
+
+    it "strips the image from the email message" do
+      expect(email).not_to have_html_part_content(image_tag)
+      expect(email).not_to have_text_part_content(image_tag)
+    end
+  end
+
+  describe "#comment_notification" do
     subject(:email) { CommentMailer.comment_notification(user, comment) }
 
     it_behaves_like "an email with a valid sender"
     it_behaves_like "it retries when the comment doesn't exist"
     it_behaves_like "a notification email with a link to the comment"
     it_behaves_like "a notification email with a link to reply to the comment"
 
+    context "when the comment is on an admin post" do
+      let(:comment) { create(:comment, :on_admin_post) }
+
+      it_behaves_like "strips image tags"
+    end
+
     context "when the comment is a reply to another comment" do
       let(:comment) { create(:comment, commentable: create(:comment)) }
 
@@ -111,6 +131,12 @@
     it_behaves_like "a notification email with a link to the comment"
     it_behaves_like "a notification email with a link to reply to the comment"
 
+    context "when the comment is on an admin post" do
+      let(:comment) { create(:comment, :on_admin_post) }
+
+      it_behaves_like "strips image tags"
+    end
+
     context "when the comment is a reply to another comment" do
       let(:comment) { create(:comment, commentable: create(:comment)) }
 
@@ -147,6 +173,12 @@
     it_behaves_like "a notification email with a link to reply to the comment"
     it_behaves_like "a notification email with a link to the comment's thread"
 
+    context "when the comment is on an admin post" do
+      let(:comment) { create(:comment, :on_admin_post) }
+
+      it_behaves_like "strips image tags"
+    end
+
     context "when the comment is on a tag" do
       let(:parent_comment) { create(:comment, :on_tag) }
 
@@ -183,6 +215,12 @@
     it_behaves_like "a notification email with a link to reply to the comment"
     it_behaves_like "a notification email with a link to the comment's thread"
 
+    context "when the comment is on an admin post" do
+      let(:comment) { create(:comment, :on_admin_post) }
+
+      it_behaves_like "strips image tags"
+    end
+
     context "when the comment is on a tag" do
       let(:parent_comment) { create(:comment, :on_tag) }
 

spec/models/feedback_reporters/abuse_reporter_spec.rb

@@ -79,5 +79,13 @@
         expect(subject.report_attributes.fetch("cf").fetch("cf_url")).to eq("Unknown URL")
       end
     end
+
+    context "if the report has an image in description" do
+      it "strips all img tags" do
+        allow(subject).to receive(:description).and_return("Hi!<img src='http://example.com/Camera-icon.svg'>Bye!")
+
+        expect(subject.report_attributes.fetch("description")).to eq("Hi!Bye!")
+      end
+    end
   end
 end