From 79824cadd329f4493aed7dfa775e69be81eb5cf3 Mon Sep 17 00:00:00 2001 From: weeklies <80141759+weeklies@users.noreply.github.com> Date: Sun, 24 Mar 2024 19:53:38 +0000 Subject: [PATCH] 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 | 2 +- .../feedback_reporters/abuse_reporter.rb | 4 +- .../feedback_reporters/support_reporter.rb | 4 +- app/views/admin/banners/_banner.html.erb | 2 +- .../bookmarks/_bookmark_blurb_short.html.erb | 4 +- .../bookmarks/_bookmark_user_module.html.erb | 4 +- app/views/comments/_single_comment.html.erb | 4 +- app/views/layouts/_banner.html.erb | 37 +++++++--------- app/views/pseuds/_pseud_module.html.erb | 4 +- app/views/share/_embed_meta_bookmark.erb | 2 +- features/bookmarks/bookmark_create.feature | 12 ++++++ .../comments_and_kudos/add_comment.feature | 15 ++++++- .../comments_adminposts.feature | 11 +++++ features/other_a/pseuds.feature | 12 ++++++ lib/html_cleaner.rb | 20 +++++---- spec/mailers/admin_mailer_spec.rb | 42 +++++++++++++++++++ spec/mailers/comment_mailer_spec.rb | 40 +++++++++++++++++- .../feedback_reporters/abuse_reporter_spec.rb | 8 ++++ .../support_reporter_spec.rb | 8 ++++ 19 files changed, 191 insertions(+), 44 deletions(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index 250a4c0df12..23fb2b02879 100644 --- a/app/models/comment.rb +++ b/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 diff --git a/app/models/feedback_reporters/abuse_reporter.rb b/app/models/feedback_reporters/abuse_reporter.rb index 2d1159ebfb7..628258102e5 100644 --- a/app/models/feedback_reporters/abuse_reporter.rb +++ b/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 diff --git a/app/models/feedback_reporters/support_reporter.rb b/app/models/feedback_reporters/support_reporter.rb index 70c8a45d090..e4f3074985f 100644 --- a/app/models/feedback_reporters/support_reporter.rb +++ b/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 diff --git a/app/views/admin/banners/_banner.html.erb b/app/views/admin/banners/_banner.html.erb index 3e171555f60..1f8f8444dce 100644 --- a/app/views/admin/banners/_banner.html.erb +++ b/app/views/admin/banners/_banner.html.erb @@ -2,6 +2,6 @@ <% # don't forget to update layouts/banner! %>
- <%=raw sanitize_field(admin_banner, :content) %> + <%= raw sanitize_field(admin_banner, :content, strip_images: true) %>
diff --git a/app/views/bookmarks/_bookmark_blurb_short.html.erb b/app/views/bookmarks/_bookmark_blurb_short.html.erb index cc177827d5c..c85c935707b 100755 --- a/app/views/bookmarks/_bookmark_blurb_short.html.erb +++ b/app/views/bookmarks/_bookmark_blurb_short.html.erb @@ -28,10 +28,10 @@ <% end %> - <% unless bookmark.bookmarker_notes.blank? %> + <% if bookmark.bookmarker_notes.present? %>
<%= ts("Bookmark Notes:") %>
- <%=raw sanitize_field(bookmark, :bookmarker_notes) %> + <%= raw sanitize_field(bookmark, :bookmarker_notes, strip_images: true) %>
<% end %> diff --git a/app/views/bookmarks/_bookmark_user_module.html.erb b/app/views/bookmarks/_bookmark_user_module.html.erb index 45faac2fbca..60330591694 100644 --- a/app/views/bookmarks/_bookmark_user_module.html.erb +++ b/app/views/bookmarks/_bookmark_user_module.html.erb @@ -45,10 +45,10 @@ <% end %> - <% unless bookmark.bookmarker_notes.blank? %> + <% if bookmark.bookmarker_notes.present? %>
<%= ts('Bookmarker\'s Notes') %>
- <%=raw sanitize_field(bookmark, :bookmarker_notes) %> + <%= raw sanitize_field(bookmark, :bookmarker_notes, strip_images: true) %>
<% end %> <% # end of information added by the bookmark owner %> diff --git a/app/views/comments/_single_comment.html.erb b/app/views/comments/_single_comment.html.erb index 19d8cb027f6..72085c022c2 100644 --- a/app/views/comments/_single_comment.html.erb +++ b/app/views/comments/_single_comment.html.erb @@ -55,7 +55,9 @@ <% if single_comment.hidden_by_admin? %>

<%= ts("This comment has been hidden by an admin.") %>

<% end %> -
<%=raw sanitize_field(single_comment, :comment_content) %>
+
+ <%= raw sanitize_field(single_comment, :comment_content, strip_images: single_comment.ultimate_parent.is_a?(AdminPost)) %> +
<% end %> <% if single_comment.edited_at.present? %>

diff --git a/app/views/layouts/_banner.html.erb b/app/views/layouts/_banner.html.erb index 745779d2186..25115ff1f6b 100644 --- a/app/views/layouts/_banner.html.erb +++ b/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] %> -

-
- <%=raw sanitize_field(@admin_banner, :content) %> -
- <% if current_user.nil? %> -

- <%= link_to "×".html_safe, current_path_with(hide_banner: true), :class => 'showme action', :title => ts("hide banner") %> -

- <% 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 %> +
+
+ <%= raw sanitize_field(@admin_banner, :content, strip_images: true) %> +
+ <% if current_user.nil? %>

- <%= submit_tag "×".html_safe, :title => ts("hide banner") %> + <%= link_to "×".html_safe, current_path_with(hide_banner: true), class: "showme action", title: ts("hide banner") %>

+ <% else %> + <%= form_tag end_banner_user_path(current_user), method: :post, remote: true do %> +

+ <%= submit_tag "×".html_safe, title: ts("hide banner") %> +

+ <% end %> <% end %> - <% end %> -
-<% end %> -<% end %> +
+ <% end %> <% end %> diff --git a/app/views/pseuds/_pseud_module.html.erb b/app/views/pseuds/_pseud_module.html.erb index 3c727397998..32258ea5c8e 100644 --- a/app/views/pseuds/_pseud_module.html.erb +++ b/app/views/pseuds/_pseud_module.html.erb @@ -15,8 +15,8 @@

<%= date %>

<% end %> -<% unless pseud.description.blank? %> +<% if pseud.description.present? %>
- <%=raw sanitize_field(pseud, :description) %> + <%= raw sanitize_field(pseud, :description, strip_images: true) %>
<% end %> diff --git a/app/views/share/_embed_meta_bookmark.erb b/app/views/share/_embed_meta_bookmark.erb index 757688ab8dc..b5f79af897c 100644 --- a/app/views/share/_embed_meta_bookmark.erb +++ b/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 %> diff --git a/features/bookmarks/bookmark_create.feature b/features/bookmarks/bookmark_create.feature index 72d1d8efe72..e56da0f057c 100644 --- a/features/bookmarks/bookmark_create.feature +++ b/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!" + 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 | diff --git a/features/comments_and_kudos/add_comment.feature b/features/comments_and_kudos/add_comment.feature index 782700c82f0..c4613c550a8 100644 --- a/features/comments_and_kudos/add_comment.feature +++ b/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!" + 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" diff --git a/features/comments_and_kudos/comments_adminposts.feature b/features/comments_and_kudos/comments_adminposts.feature index da130cc1d0e..a5060948daa 100644 --- a/features/comments_and_kudos/comments_adminposts.feature +++ b/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!" + 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!" diff --git a/features/other_a/pseuds.feature b/features/other_a/pseuds.feature index 50379fcfc78..077c1407e00 100644 --- a/features/other_a/pseuds.feature +++ b/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!" + 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" diff --git a/lib/html_cleaner.rb b/lib/html_cleaner.rb index f2636d00a16..a70e19517ec 100644 --- a/lib/html_cleaner.rb +++ b/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 diff --git a/spec/mailers/admin_mailer_spec.rb b/spec/mailers/admin_mailer_spec.rb index 9074b1b51d2..2ab35574f50 100644 --- a/spec/mailers/admin_mailer_spec.rb +++ b/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) { "" } + + 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) { "" } + + 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) } diff --git a/spec/mailers/comment_mailer_spec.rb b/spec/mailers/comment_mailer_spec.rb index 1946bcdd561..c60f4647e8d 100644 --- a/spec/mailers/comment_mailer_spec.rb +++ b/spec/mailers/comment_mailer_spec.rb @@ -71,7 +71,21 @@ end end - describe "comment_notification" do + shared_examples "strips image tags" do + let(:image_tag) { "" } + + 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" @@ -79,6 +93,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)) } @@ -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) } diff --git a/spec/models/feedback_reporters/abuse_reporter_spec.rb b/spec/models/feedback_reporters/abuse_reporter_spec.rb index 9bafc6a8aea..0cbd4c06f44 100644 --- a/spec/models/feedback_reporters/abuse_reporter_spec.rb +++ b/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!Bye!") + + expect(subject.report_attributes.fetch("description")).to eq("Hi!Bye!") + end + end end end diff --git a/spec/models/feedback_reporters/support_reporter_spec.rb b/spec/models/feedback_reporters/support_reporter_spec.rb index c14fce8555e..33065485836 100644 --- a/spec/models/feedback_reporters/support_reporter_spec.rb +++ b/spec/models/feedback_reporters/support_reporter_spec.rb @@ -89,5 +89,13 @@ expect(subject.report_attributes.fetch("cf").fetch("cf_user_agent")).to eq("Unknown user agent") 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!Bye!") + + expect(subject.report_attributes.fetch("description")).to eq("Hi!Bye!") + end + end end end