From f54e4b71b1c38b1018d83d6b21fc90349103be2b Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Mon, 27 May 2019 11:28:37 +0800 Subject: [PATCH] DEV: Make `CookedPostProcessor#post_process_images` method private. --- lib/cooked_post_processor.rb | 18 +- spec/components/cooked_post_processor_spec.rb | 745 +++++++++--------- 2 files changed, 378 insertions(+), 385 deletions(-) diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index 5ca50d50143..11828186bb4 100644 --- a/lib/cooked_post_processor.rb +++ b/lib/cooked_post_processor.rb @@ -65,15 +65,6 @@ class CookedPostProcessor BadgeGranter.grant(Badge.find(Badge::FirstReplyByEmail), @post.user, post_id: @post.id) if @post.is_reply_by_email? end - def post_process_images - extract_images.each do |img| - unless add_image_placeholder!(img) - limit_size!(img) - convert_to_link!(img) - end - end - end - def post_process_quotes @doc.css("aside.quote").each do |q| post_number = q['data-post'] @@ -688,6 +679,15 @@ class CookedPostProcessor private + def post_process_images + extract_images.each do |img| + unless add_image_placeholder!(img) + limit_size!(img) + convert_to_link!(img) + end + end + end + def process_inline_onebox(element) inline_onebox = InlineOneboxer.lookup( element.attributes["href"].value, diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index c6f3759b230..4c51589d48e 100644 --- a/spec/components/cooked_post_processor_spec.rb +++ b/spec/components/cooked_post_processor_spec.rb @@ -6,11 +6,7 @@ require "file_store/s3_store" describe CookedPostProcessor do context "#post_process" do - fab!(:upload) do - Fabricate(:upload, - url: '/uploads/default/original/1X/1/1234567890123456.jpg' - ) - end + fab!(:upload) { Fabricate(:upload) } fab!(:post) do Fabricate(:post, raw: <<~RAW) @@ -254,409 +250,408 @@ describe CookedPostProcessor do end end end - end - - context "#post_process_images" do - - before do - SiteSetting.responsive_post_image_sizes = "" - end - - context "responsive images" do - before { SiteSetting.responsive_post_image_sizes = "1|1.5|3" } - - it "includes responsive images on demand" do - upload = Fabricate(:upload, width: 2000, height: 1500, filesize: 10000) - post = Fabricate(:post, raw: "hello ") - - # fake some optimized images - OptimizedImage.create!( - url: '/uploads/default/666x500.jpg', - width: 666, - height: 500, - upload_id: upload.id, - sha1: SecureRandom.hex, - extension: '.jpg', - filesize: 500, - version: OptimizedImage::VERSION - ) - - # fake 3x optimized image, we lose 2 pixels here over original due to rounding on downsize - OptimizedImage.create!( - url: '/uploads/default/1998x1500.jpg', - width: 1998, - height: 1500, - upload_id: upload.id, - sha1: SecureRandom.hex, - extension: '.jpg', - filesize: 800 - ) - - # Fake a loading image - optimized_image = OptimizedImage.create!( - url: '/uploads/default/10x10.png', - width: CookedPostProcessor::LOADING_SIZE, - height: CookedPostProcessor::LOADING_SIZE, - upload_id: upload.id, - sha1: SecureRandom.hex, - extension: '.png', - filesize: 123 - ) - - cpp = CookedPostProcessor.new(post) - - cpp.add_to_size_cache(upload.url, 2000, 1500) - cpp.post_process_images - cpp.optimize_urls - - html = cpp.html - - expect(html).to include(%Q|data-small-upload="//test.localhost/uploads/default/10x10.png"|) - # 1.5x is skipped cause we have a missing thumb - expect(html).to include('srcset="//test.localhost/uploads/default/666x500.jpg, //test.localhost/uploads/default/1998x1500.jpg 3x"') - expect(html).to include('src="//test.localhost/uploads/default/666x500.jpg"') - - # works with CDN - set_cdn_url("http://cdn.localhost") - - cpp = CookedPostProcessor.new(post) - cpp.add_to_size_cache(upload.url, 2000, 1500) - cpp.post_process_images - cpp.optimize_urls - - html = cpp.html - - expect(html).to include(%Q|data-small-upload="//cdn.localhost/uploads/default/10x10.png"|) - expect(html).to include('srcset="//cdn.localhost/uploads/default/666x500.jpg, //cdn.localhost/uploads/default/1998x1500.jpg 3x"') - expect(html).to include('src="//cdn.localhost/uploads/default/666x500.jpg"') - end - - it "doesn't include response images for cropped images" do - upload = Fabricate(:upload, width: 200, height: 4000, filesize: 12345) - post = Fabricate(:post, raw: "hello ") - - # fake some optimized images - OptimizedImage.create!( - url: 'http://a.b.c/200x500.jpg', - width: 200, - height: 500, - upload_id: upload.id, - sha1: SecureRandom.hex, - extension: '.jpg', - filesize: 500 - ) - - cpp = CookedPostProcessor.new(post) - cpp.add_to_size_cache(upload.url, 200, 4000) - cpp.post_process_images - cpp.optimize_urls - - expect(cpp.html).to_not include('srcset="') - end - end - - shared_examples "leave dimensions alone" do - it "doesn't use them" do - expect(cpp.html).to match(/src="http:\/\/foo.bar\/image.png" width="" height=""/) - expect(cpp.html).to match(/src="http:\/\/domain.com\/picture.jpg" width="50" height="42"/) - expect(cpp).to be_dirty - end - end - - context "with image_sizes" do - fab!(:post) { Fabricate(:post_with_image_urls) } - let(:cpp) { CookedPostProcessor.new(post, image_sizes: image_sizes) } + context "processing images" do before do - cpp.post_process_images - cpp.optimize_urls + SiteSetting.responsive_post_image_sizes = "" end - context "valid" do - let(:image_sizes) { { "http://foo.bar/image.png" => { "width" => 111, "height" => 222 } } } + context "responsive images" do + before { SiteSetting.responsive_post_image_sizes = "1|1.5|3" } - it "uses them" do - expect(cpp.html).to match(/src="http:\/\/foo.bar\/image.png" width="111" height="222"/) + it "includes responsive images on demand" do + upload = Fabricate(:upload, width: 2000, height: 1500, filesize: 10000) + post = Fabricate(:post, raw: "hello ") + + # fake some optimized images + OptimizedImage.create!( + url: '/uploads/default/666x500.jpg', + width: 666, + height: 500, + upload_id: upload.id, + sha1: SecureRandom.hex, + extension: '.jpg', + filesize: 500, + version: OptimizedImage::VERSION + ) + + # fake 3x optimized image, we lose 2 pixels here over original due to rounding on downsize + OptimizedImage.create!( + url: '/uploads/default/1998x1500.jpg', + width: 1998, + height: 1500, + upload_id: upload.id, + sha1: SecureRandom.hex, + extension: '.jpg', + filesize: 800 + ) + + # Fake a loading image + optimized_image = OptimizedImage.create!( + url: '/uploads/default/10x10.png', + width: CookedPostProcessor::LOADING_SIZE, + height: CookedPostProcessor::LOADING_SIZE, + upload_id: upload.id, + sha1: SecureRandom.hex, + extension: '.png', + filesize: 123 + ) + + cpp = CookedPostProcessor.new(post) + + cpp.add_to_size_cache(upload.url, 2000, 1500) + cpp.post_process + + html = cpp.html + + expect(html).to include(%Q|data-small-upload="//test.localhost/uploads/default/10x10.png"|) + # 1.5x is skipped cause we have a missing thumb + expect(html).to include('srcset="//test.localhost/uploads/default/666x500.jpg, //test.localhost/uploads/default/1998x1500.jpg 3x"') + expect(html).to include('src="//test.localhost/uploads/default/666x500.jpg"') + + # works with CDN + set_cdn_url("http://cdn.localhost") + + cpp = CookedPostProcessor.new(post) + cpp.add_to_size_cache(upload.url, 2000, 1500) + cpp.post_process + + html = cpp.html + + expect(html).to include(%Q|data-small-upload="//cdn.localhost/uploads/default/10x10.png"|) + expect(html).to include('srcset="//cdn.localhost/uploads/default/666x500.jpg, //cdn.localhost/uploads/default/1998x1500.jpg 3x"') + expect(html).to include('src="//cdn.localhost/uploads/default/666x500.jpg"') + end + + it "doesn't include response images for cropped images" do + upload = Fabricate(:upload, width: 200, height: 4000, filesize: 12345) + post = Fabricate(:post, raw: "hello ") + + # fake some optimized images + OptimizedImage.create!( + url: 'http://a.b.c/200x500.jpg', + width: 200, + height: 500, + upload_id: upload.id, + sha1: SecureRandom.hex, + extension: '.jpg', + filesize: 500 + ) + + cpp = CookedPostProcessor.new(post) + cpp.add_to_size_cache(upload.url, 200, 4000) + cpp.post_process + + expect(cpp.html).to_not include('srcset="') + end + end + + shared_examples "leave dimensions alone" do + it "doesn't use them" do + expect(cpp.html).to match(/src="http:\/\/foo.bar\/image.png" width="" height=""/) expect(cpp.html).to match(/src="http:\/\/domain.com\/picture.jpg" width="50" height="42"/) expect(cpp).to be_dirty end end - context "invalid width" do - let(:image_sizes) { { "http://foo.bar/image.png" => { "width" => 0, "height" => 222 } } } - include_examples "leave dimensions alone" - end - - context "invalid height" do - let(:image_sizes) { { "http://foo.bar/image.png" => { "width" => 111, "height" => 0 } } } - include_examples "leave dimensions alone" - end - - context "invalid width & height" do - let(:image_sizes) { { "http://foo.bar/image.png" => { "width" => 0, "height" => 0 } } } - include_examples "leave dimensions alone" - end - - end - - context "with unsized images" do - - fab!(:post) { Fabricate(:post_with_unsized_images) } - let(:cpp) { CookedPostProcessor.new(post) } - - it "adds the width and height to images that don't have them" do - FastImage.expects(:size).returns([123, 456]) - cpp.post_process_images - expect(cpp.html).to match(/width="123" height="456"/) - expect(cpp).to be_dirty - end - - end - - context "with large images" do - - fab!(:upload) { Fabricate(:upload) } - fab!(:post) { Fabricate(:post_with_large_image) } - let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } - - before do - SiteSetting.max_image_height = 2000 - SiteSetting.create_thumbnails = true - FastImage.expects(:size).returns([1750, 2000]) - end - - it "generates overlay information" do - Upload.expects(:get_from_url).returns(upload) - OptimizedImage.expects(:resize).returns(true) - - FileStore::BaseStore.any_instance.expects(:get_depth_for).returns(0) - - cpp.post_process_images - cpp.optimize_urls - - expect(cpp.html).to match_html <<~HTML -

- logo.png1750×2000 1.21 KB -

- HTML - - expect(cpp).to be_dirty - end - - describe 'when image is inside onebox' do - let(:url) { 'https://image.com/my-avatar' } - let(:post) { Fabricate(:post, raw: url) } + context "with image_sizes" do + fab!(:post) { Fabricate(:post_with_image_urls) } + let(:cpp) { CookedPostProcessor.new(post, image_sizes: image_sizes) } before do - Oneboxer.stubs(:onebox).with(url, anything).returns("") + cpp.post_process end - it 'should not add lightbox' do - cpp.post_process_oneboxes - cpp.post_process_images - cpp.optimize_urls + context "valid" do + let(:image_sizes) { { "http://foo.bar/image.png" => { "width" => 111, "height" => 222 } } } - expect(cpp.html).to match_html <<~HTML -

- HTML + it "uses them" do + expect(cpp.html).to match(/src="http:\/\/foo.bar\/image.png" width="111" height="222"/) + expect(cpp.html).to match(/src="http:\/\/domain.com\/picture.jpg" width="50" height="42"/) + expect(cpp).to be_dirty + end end + + context "invalid width" do + let(:image_sizes) { { "http://foo.bar/image.png" => { "width" => 0, "height" => 222 } } } + include_examples "leave dimensions alone" + end + + context "invalid height" do + let(:image_sizes) { { "http://foo.bar/image.png" => { "width" => 111, "height" => 0 } } } + include_examples "leave dimensions alone" + end + + context "invalid width & height" do + let(:image_sizes) { { "http://foo.bar/image.png" => { "width" => 0, "height" => 0 } } } + include_examples "leave dimensions alone" + end + end - describe 'when image is an svg' do - fab!(:post) do - Fabricate(:post, raw: '') + context "with unsized images" do + + fab!(:post) { Fabricate(:post_with_unsized_images) } + let(:cpp) { CookedPostProcessor.new(post) } + + it "adds the width and height to images that don't have them" do + FastImage.expects(:size).returns([123, 456]) + cpp.post_process + expect(cpp.html).to match(/width="123" height="456"/) + expect(cpp).to be_dirty end - it 'should not add lightbox' do - cpp.post_process_images - cpp.optimize_urls + end - expect(cpp.html).to match_html <<~HTML -

+ context "with large images" do + fab!(:upload) { Fabricate(:upload) } + + fab!(:post) do + Fabricate(:post, raw: <<~HTML) + HTML end - describe 'when image src is an URL' do - let(:post) do - Fabricate(:post, raw: '') + let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } + + before do + SiteSetting.max_image_height = 2000 + SiteSetting.create_thumbnails = true + FastImage.expects(:size).returns([1750, 2000]) + end + + it "generates overlay information" do + OptimizedImage.expects(:resize).returns(true) + FileStore::BaseStore.any_instance.expects(:get_depth_for).returns(0) + + cpp.post_process + + expect(cpp.html).to match_html <<~HTML +

+ logo.png1750×2000 1.21 KB +

+ HTML + + expect(cpp).to be_dirty + end + + describe 'when image is inside onebox' do + let(:url) { 'https://image.com/my-avatar' } + let(:post) { Fabricate(:post, raw: url) } + + before do + Oneboxer.stubs(:onebox).with(url, anything).returns("") end it 'should not add lightbox' do - SiteSetting.crawl_images = true - cpp.post_process_images - cpp.optimize_urls + cpp.post_process - expect(cpp.html).to match_html("

") + expect(cpp.html).to match_html <<~HTML +

+ HTML end end + + describe 'when image is an svg' do + fab!(:post) do + Fabricate(:post, raw: '') + end + + it 'should not add lightbox' do + cpp.post_process + + expect(cpp.html).to match_html <<~HTML +

+ HTML + end + + describe 'when image src is an URL' do + let(:post) do + Fabricate(:post, raw: '') + end + + it 'should not add lightbox' do + SiteSetting.crawl_images = true + cpp.post_process + + expect(cpp.html).to match_html("

") + end + end + end + + end + + context "with tall images" do + fab!(:upload) { Fabricate(:upload) } + + fab!(:post) do + Fabricate(:post, raw: <<~HTML) + + HTML + end + + let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } + + before do + SiteSetting.create_thumbnails = true + FastImage.expects(:size).returns([860, 2000]) + OptimizedImage.expects(:resize).never + OptimizedImage.expects(:crop).returns(true) + FileStore::BaseStore.any_instance.expects(:get_depth_for).returns(0) + end + + it "crops the image" do + cpp.post_process + + expect(cpp.html).to match(/width="690" height="500">/) + expect(cpp).to be_dirty + end + + end + + context "with iPhone X screenshots" do + fab!(:upload) { Fabricate(:upload) } + + fab!(:post) do + Fabricate(:post, raw: <<~HTML) + + HTML + end + + let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } + + before do + SiteSetting.create_thumbnails = true + FastImage.expects(:size).returns([1125, 2436]) + OptimizedImage.expects(:resize).returns(true) + OptimizedImage.expects(:crop).never + FileStore::BaseStore.any_instance.expects(:get_depth_for).returns(0) + end + + it "crops the image" do + cpp.post_process + + expect(cpp.html).to match_html <<~HTML +

+ logo.png1125×2436 1.21 KB +

+ HTML + + expect(cpp).to be_dirty + end + + end + + context "with large images when using subfolders" do + fab!(:upload) { Fabricate(:upload) } + + fab!(:post) do + Fabricate(:post, raw: <<~HTML) + + HTML + end + + let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } + let(:base_url) { "http://test.localhost/subfolder" } + let(:base_uri) { "/subfolder" } + + before do + SiteSetting.max_image_height = 2000 + SiteSetting.create_thumbnails = true + Discourse.stubs(:base_url).returns(base_url) + Discourse.stubs(:base_uri).returns(base_uri) + FastImage.expects(:size).returns([1750, 2000]) + OptimizedImage.expects(:resize).returns(true) + + FileStore::BaseStore.any_instance.expects(:get_depth_for).returns(0) + end + + it "generates overlay information" do + cpp.post_process + + expect(cpp.html). to match_html <<~HTML +

+ logo.png1750×2000 1.21 KB +

+ HTML + + expect(cpp).to be_dirty + end + + it "should escape the filename" do + upload.update!(original_filename: ">.png") + cpp.post_process + + expect(cpp.html).to match_html <<~HTML +

+ &gt;&lt;img src=x onerror=alert(&#39;haha&#39;)&gt;.png1750×2000 1.21 KB +

+ HTML + end + + end + + context "with title" do + fab!(:upload) { Fabricate(:upload) } + + fab!(:post) do + Fabricate(:post, raw: <<~HTML) + + HTML + end + + let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } + + before do + SiteSetting.max_image_height = 2000 + SiteSetting.create_thumbnails = true + FastImage.expects(:size).returns([1750, 2000]) + OptimizedImage.expects(:resize).returns(true) + FileStore::BaseStore.any_instance.expects(:get_depth_for).returns(0) + end + + it "generates overlay information" do + cpp.post_process + + expect(cpp.html).to match_html <<~HTML +

+ WAT1750×2000 1.21 KB +

+ HTML + + expect(cpp).to be_dirty + end + + end + + context "topic image" do + let(:topic) { build(:topic, id: 1) } + let(:post) { Fabricate(:post_with_uploaded_image, topic: topic) } + let(:cpp) { CookedPostProcessor.new(post) } + + it "adds a topic image if there's one in the first post" do + FastImage.stubs(:size) + expect(post.topic.image_url).to eq(nil) + + cpp.post_process + post.topic.reload + expect(post.topic.image_url).to be_present + end end - end + context "post image" do + let(:reply) { Fabricate(:post_with_uploaded_image, post_number: 2) } + let(:cpp) { CookedPostProcessor.new(reply) } - context "with tall images" do - - let(:upload) { Fabricate(:upload) } - let(:post) { Fabricate(:post_with_large_image) } - let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } - - before do - SiteSetting.create_thumbnails = true - - Upload.expects(:get_from_url).returns(upload) - FastImage.expects(:size).returns([860, 2000]) - OptimizedImage.expects(:resize).never - OptimizedImage.expects(:crop).returns(true) - - FileStore::BaseStore.any_instance.expects(:get_depth_for).returns(0) - end - - it "crops the image" do - cpp.post_process_images - expect(cpp.html).to match(/width="690" height="500">/) - expect(cpp).to be_dirty - end - - end - - context "with iPhone X screenshots" do - - let(:upload) { Fabricate(:upload) } - let(:post) { Fabricate(:post_with_large_image) } - let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } - - before do - SiteSetting.create_thumbnails = true - - Upload.expects(:get_from_url).returns(upload) - FastImage.expects(:size).returns([1125, 2436]) - OptimizedImage.expects(:resize).returns(true) - OptimizedImage.expects(:crop).never - - FileStore::BaseStore.any_instance.expects(:get_depth_for).returns(0) - end - - it "crops the image" do - cpp.post_process_images - cpp.optimize_urls - - expect(cpp.html).to match_html <<~HTML -

- logo.png1125×2436 1.21 KB -

- HTML - - expect(cpp).to be_dirty - end - - end - - context "with large images when using subfolders" do - - fab!(:upload) { Fabricate(:upload) } - fab!(:post) { Fabricate(:post_with_large_image_on_subfolder) } - let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } - let(:base_url) { "http://test.localhost/subfolder" } - let(:base_uri) { "/subfolder" } - - before do - SiteSetting.max_image_height = 2000 - SiteSetting.create_thumbnails = true - Discourse.stubs(:base_url).returns(base_url) - Discourse.stubs(:base_uri).returns(base_uri) - - Upload.expects(:get_from_url).returns(upload) - FastImage.expects(:size).returns([1750, 2000]) - OptimizedImage.expects(:resize).returns(true) - - FileStore::BaseStore.any_instance.expects(:get_depth_for).returns(0) - end - - it "generates overlay information" do - cpp.post_process_images - cpp.optimize_urls - - expect(cpp.html). to match_html <<~HTML -

- logo.png1750×2000 1.21 KB -

- HTML - - expect(cpp).to be_dirty - end - - it "should escape the filename" do - upload.update!(original_filename: ">.png") - cpp.post_process_images - cpp.optimize_urls - - expect(cpp.html).to match_html <<~HTML -

- &gt;&lt;img src=x onerror=alert(&#39;haha&#39;)&gt;.png1750×2000 1.21 KB -

- HTML - end - - end - - context "with title" do - - let(:upload) { Fabricate(:upload) } - let(:post) { Fabricate(:post_with_large_image_and_title) } - let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } - - before do - SiteSetting.max_image_height = 2000 - SiteSetting.create_thumbnails = true - - Upload.expects(:get_from_url).returns(upload) - FastImage.expects(:size).returns([1750, 2000]) - OptimizedImage.expects(:resize).returns(true) - - FileStore::BaseStore.any_instance.expects(:get_depth_for).returns(0) - end - - it "generates overlay information" do - cpp.post_process_images - cpp.optimize_urls - - expect(cpp.html).to match_html <<~HTML -

- WAT1750×2000 1.21 KB -

- HTML - - expect(cpp).to be_dirty - end - - end - - context "topic image" do - let(:topic) { build(:topic, id: 1) } - let(:post) { Fabricate(:post_with_uploaded_image, topic: topic) } - let(:cpp) { CookedPostProcessor.new(post) } - - it "adds a topic image if there's one in the first post" do - FastImage.stubs(:size) - expect(post.topic.image_url).to eq(nil) - cpp.update_post_image - post.topic.reload - expect(post.topic.image_url).to be_present + it "adds a post image if there's one in the post" do + FastImage.stubs(:size) + expect(reply.image_url).to eq(nil) + cpp.post_process + reply.reload + expect(reply.image_url).to be_present + end end end - - context "post image" do - let(:reply) { Fabricate(:post_with_uploaded_image, post_number: 2) } - let(:cpp) { CookedPostProcessor.new(reply) } - - it "adds a post image if there's one in the post" do - FastImage.stubs(:size) - expect(reply.image_url).to eq(nil) - cpp.update_post_image - reply.reload - expect(reply.image_url).to be_present - end - end - end context "#extract_images" do @@ -867,9 +862,7 @@ describe CookedPostProcessor do post.save_custom_fields cpp = CookedPostProcessor.new(post, invalidate_oneboxes: true) - cpp.post_process_oneboxes - cpp.post_process_images - cpp.optimize_urls + cpp.post_process expect(cpp.doc.to_s).to match(/
/) end