From 0fb783174991fff7723c46b74ed9520081aede2c Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Fri, 1 Sep 2017 19:56:13 +0530 Subject: [PATCH] FEATURE: Add placeholders to broken and large image files (#5113) --- .../stylesheets/common/base/topic-post.scss | 6 ++ app/jobs/regular/pull_hotlinked_images.rb | 25 ++++++ spec/jobs/pull_hotlinked_images_spec.rb | 77 ++++++++++++++++--- 3 files changed, 96 insertions(+), 12 deletions(-) diff --git a/app/assets/stylesheets/common/base/topic-post.scss b/app/assets/stylesheets/common/base/topic-post.scss index 3f34809d20a..1485257753c 100644 --- a/app/assets/stylesheets/common/base/topic-post.scss +++ b/app/assets/stylesheets/common/base/topic-post.scss @@ -420,3 +420,9 @@ a.mention, a.mention-group { padding-bottom: 15px; } } + +.broken-image, .large-image { + border: 1px solid $primary-low; + font-size: 32px; + padding: 16px; +} diff --git a/app/jobs/regular/pull_hotlinked_images.rb b/app/jobs/regular/pull_hotlinked_images.rb index c38d9a55e24..ccd9b205430 100644 --- a/app/jobs/regular/pull_hotlinked_images.rb +++ b/app/jobs/regular/pull_hotlinked_images.rb @@ -25,6 +25,7 @@ module Jobs raw = post.raw.dup start_raw = raw.dup downloaded_urls = {} + broken_images, large_images = [], [] extract_images_from(post.cooked).each do |image| src = original_src = image['src'] @@ -56,9 +57,11 @@ module Jobs end else log(:info, "Failed to pull hotlinked image for post: #{post_id}: #{src} - Image is bigger than #{@max_size}") + large_images << original_src end else log(:info, "There was an error while downloading '#{src}' locally for post: #{post_id}") + broken_images << original_src end end # have we successfully downloaded that file? @@ -98,6 +101,28 @@ module Jobs post.revise(Discourse.system_user, changes, options) elsif downloaded_urls.present? post.trigger_post_process(true) + elsif broken_images.present? || large_images.present? + start_html = post.cooked + doc = Nokogiri::HTML::fragment(start_html) + images = doc.css("img[src]") - doc.css("img.avatar") + images.each do |tag| + src = tag['src'] + if broken_images.include?(src) + tag.name = 'span' + tag.set_attribute('class', 'broken-image fa fa-chain-broken') + tag.remove_attribute('src') + elsif large_images.include?(src) + tag.name = 'a' + tag.set_attribute('href', src) + tag.set_attribute('target', '_blank') + tag.remove_attribute('src') + tag.inner_html = '' + end + end + if start_html == post.cooked && doc.to_html != post.cooked + post.update_column(:cooked, doc.to_html) + post.publish_change_to_clients! :revised + end end end diff --git a/spec/jobs/pull_hotlinked_images_spec.rb b/spec/jobs/pull_hotlinked_images_spec.rb index 19e0049d059..566b37f3549 100644 --- a/spec/jobs/pull_hotlinked_images_spec.rb +++ b/spec/jobs/pull_hotlinked_images_spec.rb @@ -3,19 +3,29 @@ require 'jobs/regular/pull_hotlinked_images' describe Jobs::PullHotlinkedImages do - describe '#execute' do - let(:image_url) { "http://wiki.mozilla.org/images/2/2e/Longcat1.png" } - let(:png) { Base64.decode64("R0lGODlhAQABALMAAAAAAIAAAACAAICAAAAAgIAAgACAgMDAwICAgP8AAAD/AP//AAAA//8A/wD//wBiZCH5BAEAAA8ALAAAAAABAAEAAAQC8EUAOw==") } + let(:image_url) { "http://wiki.mozilla.org/images/2/2e/Longcat1.png" } + let(:broken_image_url) { "http://wiki.mozilla.org/images/2/2e/Longcat2.png" } + let(:large_image_url) { "http://wiki.mozilla.org/images/2/2e/Longcat3.png" } + let(:png) { Base64.decode64("R0lGODlhAQABALMAAAAAAIAAAACAAICAAAAAgIAAgACAgMDAwICAgP8AAAD/AP//AAAA//8A/wD//wBiZCH5BAEAAA8ALAAAAAABAAEAAAQC8EUAOw==") } + let(:large_png) { Base64.decode64("iVBORw0KGgoAAAANSUhEUgAAAEAAAABACAIAAAAlC+aJAAAK10lEQVR42r3aeVRTVx4H8Oc2atWO7Sw9OnM6HWvrOON0aFlcAZ3RopZWOyqgoCACKqPWBUVQi4gIqAVllciiKPu+JOyGnQQSNgkIIQgoKljAYVARCZnf4yXhkeXlJmDP+f4hOUF+n3fvffe++y5W0i4qJqWoDU8hKQUPxWFKcq9VnHxJ8gTi5EqS0yJOtiRZfHEyJWE0i0MnJaMJTzopaQ/wpJKS0ogneTQYABANTDlDvpxBCsiu72eUP0zPq8Fzr45e8TircRDFQAAy5ABpcgDCgJV2iCbRQM+rinU/E26ie9NgfrDO1GBtTBy96SH/WhBhaxwfGEjndmfKGeiaGsYAJXIANQyCkfR05u3dhuOKVhLamnmRzocyKp9mNo9QG9IRDDiAiMaG3Nqfo45aoJROzk3DDxNCbjGahBM0yAKoDfIDOpNZE/bNYrVKJyfylB2D91pdA3lAjwE0MDAyS+BCalw9kdu2xvT6AY0NWBkJoNaAzsrj4CN1YtUTidi/hdH4BvGmJGPAAYgGMuMery/U6ONJqZ5I1PlTjNExre7kgJU/EqEbJC0gjDpiiv9hnSkJ2z+t9dzxwNcSUudlUuuxnXP+W/bZTWWO64uO6hccWQ0pPm4IP1a6GFe5bYXvNF7f0xxg3XrzgCDYjn1m4+218/D/SndaYnSqBpMDDlDXkHYnMlh7Srj+HLanxfOsyyOVN0ScYI0zkOeVZvYZGEI2/DFDMkWgTw7jAGWUA5owMOt7QtcvDF09qybA/mGC6zA7aCLVExkq9U3895/wm9LpgyonBxmDGKDQoHBySPQ8B5e/zM2kJdalN/fqxKsn8oLhFr5mdvDyX6UVNqqcpMmDAWNJACjtUMDrDVn7m6SdS/kxPwrizg+zAycLAKm5tA0a4a7DPpSFhmIAxWAgDKm0IJrutBr/g3D5n9E9J7F6oiNFGf2WtnI2vboH3YADEA0AuG2ml2i2BC4/AAYKr00uAHL/ihk0QnxQMPqKFWM/FiEamFWPYMHD8tgF1UMmZfjKZLDIJ1z/vQibzTKrbop2wAGIhoxbt8IN5zZHnoHqO5LdJr16IkXHDG4afJDJG0B8chADUAxxTnbp1trE5Z/0ASDN09hTcJdLy+EoawQZgyyAwhCxcznr0k4C0JNz5R0BYFqM3PBhQugtxKdQrEICUGFoE4ZtWPAg4jQBeJHv/Y4AkBKHdTHuZ8lP0hSDAQdQGwhAUUNv4s6/EvcfSD/T590B2u8cj3SwltkNUGaQBSgbDAXc9pxTW4jqIf8ruAa37efJLg/DfuBd21ftYU7OA387+QXSk2gHWMmRw/M2F9D2d8WffsW8Sv5+X/mtyBN7s+V2NBQasMpOEYqhuLG3MimMqL4h/GTu4fW01b/z05qrMKEGC96W+8sA8g/qKX281JuWafX350lniG++rIpOTcknb8lQGHAAoqG+pgqqr7hqE2K4kCg0bO3CJDMthvVKInTrlUmm/4j+9vO7mxYNlfrJAJiHVsYaL0g1XZy194scmy+JMCyXxWz+CAD4anTFjLrLpiMVQW+4t1G2lQiDGIBiuF/NLbmwM1B3PpQe892SFtqh4fIAhZ14mBUo34WE7ECFC29hRdDz5LO5dtrwdAGM0pP/HKoMzWsZRtwakwVQGPJjo/2/ej9Q74N8xy19o+tQYcWNzjT3mJNmR/W/uPi9fobr3ifpl6hXeG9Zge1JF5LPWvz4zYoTa7VSzu0mniggMEigNcBQ7GjE5A9Kt/eoOxLGkQBUGkoyGeEbPqnys2+OPlcbdir80PdOX+usmDFdG8OIwCc3bI0vm657WeSrsPouhuelbQZh/9nqY7FB+lsGc2ad27w86oTJo5SLrwu9s/dpVXuYFPEHELcocQC1QXpjhS4EpcMwiPhh2/U9XzfedYYFhe7UKdJSqkNOIt4oMy/uIwP68n6C3/WzMmIFHIUeJawMLm7ul9lmVdYOYgCKob6aK72NEo8yQ+UBtl99BkXoTMFcv1sF3UNaIpd24vCqvykDvCr2PbJ6GQFwNtKFrjhuCHFCCvmvcuW2ihUaMO4TWYCyAU0GSJcSsCblRTjDSJAZoFnuNiafLqReMrQlukKTylQvBZC3iikMOIDCQGaQAT9nq1gLqQRQBABFLa9U7tcTBjEApR3IALh1/DIAlQZZAIWBDOjO9HrXAMT3JliVBKCyHciALsYvAUAx4IAqOYDCmxKPBFD5QDNBQHHLS2XvfmQMYgCKgQx4muGhFmCw1B8dIOTQyvj9FO+vyDclrPqpLECZgVczBoAlA3URMCubLv6D9I657ZOP0lws1QJQv4OTGnAAogEdAF+A+TXHw3b0R5qoszLLyx4+gc8RAeUt/SrfIxIGMYDCoBDwONVdaQ9mB+3XWeK87kvJ1EYTDfYLn9XDgsdO+3NYKSACUN6FQsYAKg2IgIqgY6tnzmi6bP8y2X2EmGUbkkWCPJitV82cURfuqPq5nhPM4vchvpDGauQAygxkAMW+ULCdsfWSj/tCTr8IdeqPdBnK94FnFCEr8DXd68CyRXeObkfpRWx+D+JLdRxANlC0QwMaINHZfP37c4oczQkDnjDnvlCnMuc9RvPnxp/ehQKokAAoOlIeGUDdDvKAtsQLyv72mzJ/P6uN+rNnHtf5S7GjRVeQQ6nTbge9pdB/vEzWDso9aqoEUBuw2mciZY0gY0AEEBHEuZzZqAdFG743c/n0aQ7rtBruOKO/y+HwnyMebsABiIbG2jFAa7wryh4bPDaUXD+swWuoKv5TxMMNYgCFgQSoIgHOv7uNLbgLcfldiAc0xgAqDbVtLwTJXgQAeojmLzLKAzjBxyl257vqcgsfChUeDJA3YHUkgEpDQz2vJU7cCDJTEnQSWOHBDK0wMACgL0U7mLptXWO/fGmCk7myGW2gOra09Q36aSUcoIahc4Rfmi59JBi3H5j3k5fJOs8dhgoTYL0Jqi/1PfyMTrUKHOKGcwS9Kg9okA1iALqh+tGggBFIGJRtn2gWWEHwmlsRD5lIDdj9LpG8gXpyuN/yRJBwEQCwRYWytkEcuB28iuK2EXVPXOEAqaEW2dBUzZI+HE/wTT2RnjpGSZtQg1NjYoDa7dA50sKMIgywyTPB6l9VRbPaXmt28m0MQNEOCgdDbXu/IM17tCO5TaQjveWG1Qi6NT75htWTAOoaeA/4gnhXlF0Wiq7f3NSk1okrGQMO0NzQOdLMziU60usSPw2q7+SVlnWMlE3g1BjG6xZNxFDe1s2OO0Z0JHhxBuMBJlroUSgju682ldUxTH24QaVhDFAvB1Bp4HS+PRO/5ZDP7xtjnaXLJGKlBMtVeGqDuRk2If97z/tl0XVYZg+T3nF0F3tcjN1W2vFWrdNK8gYcgGiQvykFFl7a7oFBvG5o5UfvVRQrRuQu+mjgH5lRu7JjLPISLAtTrJ1pf94dj4U0+mhw4opsEAPU6kiEIZ1XYnZlFgFQKzu8MYtYzKYUs63E7Lnz0ls5iKeVFBrGAGq1A6uj1zZw0XZPzPwuZhqE7biiqm4vzNQP/7JVFmZbgdlxxnKienFBe4/G7YA1kADI7TDilmQJZVlE41cRirBlYdZMzIqB7UnGdseRkohZZmDW+ZhNmfibEHvuzAOcaWTD5XpLuBepdfKtiAxQ1xDPTdnhOdXUH7Nlj7uWKDnAme7bvPlI1a/Hfz4ljp+BfnqPPKD/DzQWIVWNoUiJAAAAAElFTkSuQmCC") } + before do + stub_request(:get, image_url).to_return(body: png, headers: { "Content-Type" => "image/png" }) + stub_request(:head, image_url) + stub_request(:head, broken_image_url).to_return(status: 404) + stub_request(:get, large_image_url).to_return(body: large_png, headers: { "Content-Type" => "image/png" }) + stub_request(:head, large_image_url) + SiteSetting.download_remote_images_to_local = true + SiteSetting.max_image_size_kb = 2 + end + + describe '#execute' do before do - stub_request(:get, image_url).to_return(body: png, headers: { "Content-Type" => "image/png" }) - stub_request(:head, image_url) - SiteSetting.download_remote_images_to_local = true FastImage.expects(:size).returns([100, 100]).at_least_once end it 'replaces images' do - post = Fabricate(:post, raw: "") + post = Fabricate(:post, raw: "") Jobs::PullHotlinkedImages.new.execute(post_id: post.id) post.reload @@ -24,7 +34,8 @@ describe Jobs::PullHotlinkedImages do end it 'replaces images without protocol' do - post = Fabricate(:post, raw: "") + url = image_url.sub(/^https?\:/, '') + post = Fabricate(:post, raw: "") Jobs::PullHotlinkedImages.new.execute(post_id: post.id) post.reload @@ -33,10 +44,10 @@ describe Jobs::PullHotlinkedImages do end it 'replaces images without extension' do - extensionless_url = "http://wiki.mozilla.org/images/2/2e/Longcat1" - stub_request(:get, extensionless_url).to_return(body: png, headers: { "Content-Type" => "image/png" }) - stub_request(:head, extensionless_url) - post = Fabricate(:post, raw: "") + url = image_url.sub(/\.[a-zA-Z0-9]+$/, '') + stub_request(:get, url).to_return(body: png, headers: { "Content-Type" => "image/png" }) + stub_request(:head, url) + post = Fabricate(:post, raw: "") Jobs::PullHotlinkedImages.new.execute(post_id: post.id) post.reload @@ -80,6 +91,48 @@ describe Jobs::PullHotlinkedImages do expect(post.cooked).to match(/ +#{url} + + + ") + + Jobs::ProcessPost.new.execute(post_id: post.id) + Jobs::PullHotlinkedImages.new.execute(post_id: post.id) + Jobs::ProcessPost.new.execute(post_id: post.id) + Jobs::PullHotlinkedImages.new.execute(post_id: post.id) + post.reload + + expect(post.cooked).to match(/

<\/span><\/a>/) + end + end + end + + describe 'replace' do + it 'broken image with placeholder' do + post = Fabricate(:post, raw: "") + + Jobs::ProcessPost.new.execute(post_id: post.id) + Jobs::PullHotlinkedImages.new.execute(post_id: post.id) + post.reload + + expect(post.cooked).to match(/") + + Jobs::ProcessPost.new.execute(post_id: post.id) + Jobs::PullHotlinkedImages.new.execute(post_id: post.id) + post.reload + + expect(post.cooked).to match(/<\/span><\/a>/) end end