diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index 3c348a12870..9039e738a1d 100644 --- a/lib/cooked_post_processor.rb +++ b/lib/cooked_post_processor.rb @@ -12,52 +12,34 @@ class CookedPostProcessor @post = post @doc = Nokogiri::HTML::fragment(post.cooked) @size_cache = {} + @has_been_uploaded_cache = {} end - def dirty? - @dirty + def post_process + return unless @doc.present? + post_process_images + post_process_oneboxes end - # Bake onebox content into the post - def post_process_oneboxes - args = {post_id: @post.id} - args[:invalidate_oneboxes] = true if @opts[:invalidate_oneboxes] - - result = Oneboxer.apply(@doc) do |url, element| - Oneboxer.onebox(url, args) - end - @dirty ||= result.changed? - end - - # First let's consider the images def post_process_images images = @doc.search("img") return unless images.present? images.each do |img| + # keep track of the src src = img['src'] - src = Discourse.base_url_no_prefix + src if src =~ /^\/[^\/]/ + # make sure the src is absolute (when working with locally uploaded files) + img['src'] = Discourse.base_url_no_prefix + img['src'] if img['src'] =~ /^\/[^\/]/ if src.present? - - if img['width'].blank? || img['height'].blank? - w, h = get_size_from_image_sizes(src, @opts[:image_sizes]) || image_dimensions(src) - - if w && h - img['width'] = w.to_s - img['height'] = h.to_s - @dirty = true - end - end - - if src != img['src'] - img['src'] = src - @dirty = true - end - - convert_to_link!(img) + # update img dimensions if at least one is missing + update_dimensions!(img) + # optimize image img['src'] = optimize_image(img) - + # lightbox treatment + convert_to_link!(img) + # mark the post as dirty whenever the src has changed + @dirty |= src != img['src'] end end @@ -69,35 +51,32 @@ class CookedPostProcessor end + def post_process_oneboxes + args = { post_id: @post.id } + args[:invalidate_oneboxes] = true if @opts[:invalidate_oneboxes] + # bake onebox content into the post + result = Oneboxer.apply(@doc) do |url, element| + Oneboxer.onebox(url, args) + end + # mark the post as dirty whenever a onebox as been baked + @dirty |= result.changed? + end + + def update_dimensions!(img) + return if img['width'].present? && img['height'].present? + + w, h = get_size_from_image_sizes(img['src'], @opts[:image_sizes]) || image_dimensions(img['src']) + + if w && h + img['width'] = w.to_s + img['height'] = h.to_s + @dirty = true + end + end + def optimize_image(img) - src = img["src"] - return src - - # implementation notes: Sam - # - # I have disabled this for now, would like the following addressed. - # - # 1. We need a db record pointing the files on the file system to the post they are on, - # if we do not do that we have no way of purging any local optimised copies - # - # 2. We should be storing images in /uploads/site-name/_optimised ... it simplifies configuration - # - # 3. I don't want to have a folder with 10 million images, let split it so /uploads/site-name/_optimised/ABC/DEF/AAAAAAAA.jpg - # - # 4. We shoul confirm that that we test both saving as jpg and png and pick the more efficient format ... tricky to get right - # - # 5. All images should also be optimised using image_optim, it ensures that best compression is used - # - # 6. Admin screen should alert users of any missing dependencies (image magick, etc, and explain what it is for) - # - # 7. Optimise images should be a seperate site setting. - - # supports only local uploads - return src if SiteSetting.enable_s3_uploads? - - width, height = img["width"].to_i, img["height"].to_i - - ImageOptimizer.new(src).optimized_image_url(width, height) + return img["src"] + # TODO: needs some <3 end def convert_to_link!(img) @@ -135,52 +114,44 @@ class CookedPostProcessor end end - def post_process - return unless @doc.present? - post_process_images - post_process_oneboxes + # Retrieve the image dimensions for a url + def image_dimensions(url) + uri = get_image_uri(url) + return unless uri + w, h = get_size(url) + ImageSizer.resize(w, h) if w && h + end + + def get_size(url) + # we can always crawl our own images + return unless SiteSetting.crawl_images? || has_been_uploaded?(url) + @size_cache[url] ||= FastImage.size(url) + rescue Zlib::BufError # FastImage.size raises BufError for some gifs + end + + def get_image_uri(url) + uri = URI.parse(url) + uri if %w(http https).include?(uri.scheme) + end + + def has_been_uploaded?(url) + @has_been_uploaded_cache[url] ||= url.start_with?(base_url) + end + + def base_url + asset_host.present? ? asset_host : Discourse.base_url_no_prefix + end + + def asset_host + ActionController::Base.asset_host + end + + def dirty? + @dirty end def html @doc.try(:to_html) end - def doc - @doc - end - - def get_size(url) - # we need to find out whether it's an external image or an uploaded one - # an external image would be: http://google.com/logo.png - # an uploaded image would be: http://my.discourse.com/uploads/default/12345.png or http://my.cdn.com/uploads/default/12345.png - uri = url - # this will transform `http://my.discourse.com/uploads/default/12345.png` into a local uri - uri = "#{Rails.root}/public#{url[Discourse.base_url.length..-1]}" if url.start_with?(Discourse.base_url) - # this will do the same but when CDN has been defined in the configuration - uri = "#{Rails.root}/public#{url[ActionController::Base.asset_host.length..-1]}" if ActionController::Base.asset_host && url.start_with?(ActionController::Base.asset_host) - # return nil when it's an external image *and* crawling is disabled - return nil unless SiteSetting.crawl_images? || uri[0] == "/" - @size_cache[uri] ||= FastImage.size(uri) - rescue Zlib::BufError - # FastImage.size raises BufError for some gifs - return nil - end - - def get_image_uri(url) - uri = URI.parse(url) - if %w(http https).include?(uri.scheme) - uri - else - nil - end - end - - # Retrieve the image dimensions for a url - def image_dimensions(url) - uri = get_image_uri(url) - return nil unless uri - w, h = get_size(url) - ImageSizer.resize(w, h) if w && h - end - end diff --git a/lib/image_sizer.rb b/lib/image_sizer.rb index 18e8d16d82f..3af9f679b88 100644 --- a/lib/image_sizer.rb +++ b/lib/image_sizer.rb @@ -3,7 +3,7 @@ module ImageSizer # Resize an image to the aspect ratio we want def self.resize(width, height) max_width = SiteSetting.max_image_width.to_f - return nil if width.blank? || height.blank? + return if width.blank? || height.blank? w = width.to_f h = height.to_f diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 47960893a75..bcb6d790ba8 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -151,20 +151,18 @@ module PrettyText def self.apply_cdn(html, url) return html unless url - image = /\.(jpg|jpeg|gif|png|tiff|tif)$/ + image = /\.(jpg|jpeg|gif|png|tiff|tif|bmp)$/ doc = Nokogiri::HTML.fragment(html) + doc.css("a").each do |l| - href = l.attributes["href"].to_s - if href[0] == '/' && href =~ image - l["href"] = url + href - end + href = l["href"].to_s + l["href"] = url + href if href[0] == '/' && href =~ image end + doc.css("img").each do |l| - src = l.attributes["src"].to_s - if src[0] == '/' - l["src"] = url + src - end + src = l["src"].to_s + l["src"] = url + src if src[0] == '/' end doc.to_s diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index fc1b2d1f3a4..1e38644fc1b 100644 --- a/spec/components/cooked_post_processor_spec.rb +++ b/spec/components/cooked_post_processor_spec.rb @@ -41,7 +41,6 @@ describe CookedPostProcessor do before do @topic = Fabricate(:topic) @post = Fabricate.build(:post_with_image_url, topic: @topic, user: @topic.user) - ImageSorcery.any_instance.stubs(:convert).returns(false) @cpp = CookedPostProcessor.new(@post, image_sizes: {'http://www.forumwarz.com/images/header/logo.png' => {'width' => 111, 'height' => 222}}) @cpp.expects(:get_size).returns([111,222]) end @@ -65,7 +64,6 @@ describe CookedPostProcessor do before do FastImage.stubs(:size).returns([123, 456]) ImageSorcery.any_instance.stubs(:convert).returns(false) - CookedPostProcessor.any_instance.expects(:image_dimensions).returns([123, 456]) creator = PostCreator.new(user, raw: Fabricate.build(:post_with_images).raw, topic_id: topic.id) @post = creator.create end @@ -89,7 +87,6 @@ describe CookedPostProcessor do let(:processor) { CookedPostProcessor.new(post) } before do - ImageSorcery.any_instance.stubs(:convert).returns(false) processor.post_process_images end @@ -151,4 +148,35 @@ describe CookedPostProcessor do end end + context 'get_image_uri' do + + it "returns nil unless the scheme is either http or https" do + cpp.get_image_uri("http://domain.com").should == URI.parse("http://domain.com") + cpp.get_image_uri("https://domain.com").should == URI.parse("https://domain.com") + cpp.get_image_uri("ftp://domain.com").should == nil + cpp.get_image_uri("ftps://domain.com").should == nil + cpp.get_image_uri("//domain.com").should == nil + cpp.get_image_uri("/tmp/image.png").should == nil + end + + end + + context 'has_been_uploaded?' do + + it "identifies internal urls" do + Discourse.expects(:base_url_no_prefix).returns("http://my.discourse.com") + cpp.has_been_uploaded?("http://my.discourse.com").should == true + end + + it "identifies internal urls when using a CDN" do + ActionController::Base.expects(:asset_host).returns("http://my.cdn.com").twice + cpp.has_been_uploaded?("http://my.cdn.com").should == true + end + + it "identifies external urls" do + cpp.has_been_uploaded?("http://domain.com").should == false + end + + end + end diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb index 66da90ba8cc..bac455e97c1 100644 --- a/spec/models/post_action_spec.rb +++ b/spec/models/post_action_spec.rb @@ -2,11 +2,6 @@ require 'spec_helper' require_dependency 'post_destroyer' describe PostAction do - - before do - ImageSorcery.any_instance.stubs(:convert).returns(false) - end - it { should belong_to :user } it { should belong_to :post } it { should belong_to :post_action_type } diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 4aa8f41231a..70521715896 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -2,11 +2,6 @@ require 'spec_helper' require_dependency 'post_destroyer' describe Post do - - before do - ImageSorcery.any_instance.stubs(:convert).returns(false) - end - # Help us build a post with a raw body def post_with_body(body, user=nil) args = post_args.merge(raw: body)