From 31fd5b85bc8826eda791f3928da851bbc20b957c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Wed, 20 Nov 2013 13:10:08 +0100 Subject: [PATCH] FIX markdown hotlinked images were not properly pulled --- app/helpers/common_helper.rb | 2 +- app/helpers/composer_messages_helper.rb | 2 - app/jobs/regular/pull_hotlinked_images.rb | 7 ++- lib/cooked_post_processor.rb | 16 ++---- lib/url_helper.rb | 15 ++++++ spec/components/cooked_post_processor_spec.rb | 18 +++++++ spec/components/url_helper_spec.rb | 52 +++++++++++++++++++ 7 files changed, 96 insertions(+), 16 deletions(-) delete mode 100644 app/helpers/composer_messages_helper.rb create mode 100644 lib/url_helper.rb create mode 100644 spec/components/url_helper_spec.rb diff --git a/app/helpers/common_helper.rb b/app/helpers/common_helper.rb index 8a1efe42716..9946e9c555d 100644 --- a/app/helpers/common_helper.rb +++ b/app/helpers/common_helper.rb @@ -1,6 +1,6 @@ module CommonHelper def render_google_analytics_code - if Rails.env == "production" && SiteSetting.ga_tracking_code.present? + if Rails.env == "production" && SiteSetting.ga_tracking_code.present? render partial: "common/google_analytics" end end diff --git a/app/helpers/composer_messages_helper.rb b/app/helpers/composer_messages_helper.rb deleted file mode 100644 index dd95c751713..00000000000 --- a/app/helpers/composer_messages_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module ComposerMessagesHelper -end diff --git a/app/jobs/regular/pull_hotlinked_images.rb b/app/jobs/regular/pull_hotlinked_images.rb index 90f9b447146..4c06be58ca3 100644 --- a/app/jobs/regular/pull_hotlinked_images.rb +++ b/app/jobs/regular/pull_hotlinked_images.rb @@ -1,6 +1,9 @@ +require_dependency 'url_helper' + module Jobs class PullHotlinkedImages < Jobs::Base + include UrlHelper def initialize # maximum size of the file in bytes @@ -40,11 +43,13 @@ module Jobs if downloaded_urls[src].present? url = downloaded_urls[src] escaped_src = src.gsub("?", "\\?").gsub(".", "\\.").gsub("+", "\\+") - # there are 5 ways to insert an image in a post + # there are 6 ways to insert an image in a post # HTML tag - raw.gsub!(/src=["']#{escaped_src}["']/i, "src='#{url}'") # BBCode tag - [img]http://...[/img] raw.gsub!(/\[img\]#{escaped_src}\[\/img\]/i, "[img]#{url}[/img]") + # Markdown linked image - [![alt](http://...)](http://...) + raw.gsub!(/\[!\[([^\]]*)\]\(#{escaped_src}\)\]/) { "[#{$1}]" } # Markdown inline - ![alt](http://...) raw.gsub!(/!\[([^\]]*)\]\(#{escaped_src}\)/) { "![#{$1}](#{url})" } # Markdown reference - [x]: http:// diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index f1e8558a2a6..794c5eb60e2 100644 --- a/lib/cooked_post_processor.rb +++ b/lib/cooked_post_processor.rb @@ -2,9 +2,11 @@ # For example, inserting the onebox content, or image sizes/thumbnails. require_dependency "oneboxer" +require_dependency 'url_helper' class CookedPostProcessor include ActionView::Helpers::NumberHelper + include UrlHelper def initialize(post, opts={}) @dirty = false @@ -124,10 +126,11 @@ class CookedPostProcessor def is_a_hyperlink?(img) parent = img.parent while parent - return if parent.name == "a" + return true if parent.name == "a" break unless parent.respond_to? :parent parent = parent.parent end + false end def add_lightbox!(img, original_width, original_height, upload=nil) @@ -206,17 +209,6 @@ class CookedPostProcessor end end - def is_local(url) - Discourse.store.has_been_uploaded?(url) || url =~ /^\/assets\// - end - - def absolute(url) - url =~ /^\/[^\/]/ ? (Discourse.asset_host || Discourse.base_url_no_prefix) + url : url - end - - def schemaless(url) - url.gsub(/^https?:/, "") - end def pull_hotlinked_images # is the job enabled? diff --git a/lib/url_helper.rb b/lib/url_helper.rb new file mode 100644 index 00000000000..8e46e00ee6e --- /dev/null +++ b/lib/url_helper.rb @@ -0,0 +1,15 @@ +module UrlHelper + + def is_local(url) + Discourse.store.has_been_uploaded?(url) || url =~ /^\/assets\// + end + + def absolute(url) + url =~ /^\/[^\/]/ ? (Discourse.asset_host || Discourse.base_url_no_prefix) + url : url + end + + def schemaless(url) + url.gsub(/^https?:/, "") + end + +end diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index 3847dadfbe7..c5453ab42bd 100644 --- a/spec/components/cooked_post_processor_spec.rb +++ b/spec/components/cooked_post_processor_spec.rb @@ -340,4 +340,22 @@ describe CookedPostProcessor do end + context ".is_a_hyperlink?" do + + let(:post) { build(:post) } + let(:cpp) { CookedPostProcessor.new(post) } + let(:doc) { Nokogiri::HTML::fragment('

') } + + it "is true when the image is inside a link" do + img = doc.css("img#linked_image").first + cpp.is_a_hyperlink?(img).should be_true + end + + it "is false when the image is not inside a link" do + img = doc.css("img#standard_image").first + cpp.is_a_hyperlink?(img).should be_false + end + + end + end diff --git a/spec/components/url_helper_spec.rb b/spec/components/url_helper_spec.rb new file mode 100644 index 00000000000..a010af8e3bb --- /dev/null +++ b/spec/components/url_helper_spec.rb @@ -0,0 +1,52 @@ +require 'spec_helper' +require_dependency 'url_helper' + +describe UrlHelper do + + class DummyClass + include UrlHelper + end + + let(:helper) { DummyClass.new } + + describe "#is_local" do + + it "is true when the file has been uploaded" do + store = stub + store.expects(:has_been_uploaded?).returns(true) + Discourse.stubs(:store).returns(store) + helper.is_local("http://discuss.site.com/path/to/file.png").should be_true + end + + it "is true for relative assets" do + store = stub + store.expects(:has_been_uploaded?).returns(false) + Discourse.stubs(:store).returns(store) + helper.is_local("/assets/javascripts/all.js").should be_true + end + + end + + describe "#absolute" do + + it "does not change non-relative url" do + helper.absolute("http://www.discourse.org").should == "http://www.discourse.org" + end + + it "changes a relative url to an absolute one" do + helper.absolute("/path/to/file").should == "http://test.localhost/path/to/file" + end + + end + + describe "#schemaless" do + + it "removes http or https schemas only" do + helper.schemaless("http://www.discourse.org").should == "//www.discourse.org" + helper.schemaless("https://secure.discourse.org").should == "//secure.discourse.org" + helper.schemaless("ftp://ftp.discourse.org").should == "ftp://ftp.discourse.org" + end + + end + +end