From 08aa23f0ca23711717aeb0febed7721c690fc2e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Sat, 22 Jun 2013 13:38:42 +0200 Subject: [PATCH] FIX: lightbox wasn't working when using s3 upload --- app/models/upload.rb | 18 ++++++++++++++---- ...20130622110348_add_url_index_to_uploads.rb | 5 +++++ lib/cooked_post_processor.rb | 19 ++++++++++++------- lib/local_store.rb | 4 ++++ lib/s3.rb | 6 +++++- spec/components/cooked_post_processor_spec.rb | 16 ++++++++-------- spec/models/upload_spec.rb | 7 +++++++ 7 files changed, 55 insertions(+), 20 deletions(-) create mode 100644 db/migrate/20130622110348_add_url_index_to_uploads.rb diff --git a/app/models/upload.rb b/app/models/upload.rb index 6257f0653..20edfa37e 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -30,6 +30,7 @@ class Upload < ActiveRecord::Base def create_thumbnail! return unless SiteSetting.create_thumbnails? + return if SiteSetting.enable_s3_uploads? return unless width > SiteSetting.auto_link_images_wider_than return if has_thumbnail? thumbnail = OptimizedImage.create_for(self, width, height) @@ -86,12 +87,20 @@ class Upload < ActiveRecord::Base LocalStore.remove_file(url) end - def self.uploaded_regex - /\/uploads\/#{RailsMultisite::ConnectionManagement.current_db}\/(?\d+)\/[0-9a-f]{16}\.(png|jpg|jpeg|gif|tif|tiff|bmp)/ + def self.has_been_uploaded?(url) + is_relative?(url) || is_local?(url) || is_on_s3?(url) end - def self.has_been_uploaded?(url) - (url =~ /^\/[^\/]/) == 0 || url.start_with?(base_url) + def self.is_relative?(url) + (url =~ /^\/[^\/]/) == 0 + end + + def self.is_local?(url) + url.start_with?(base_url) + end + + def self.is_on_s3?(url) + SiteSetting.enable_s3_uploads? && url.start_with?(S3.base_url) end def self.base_url @@ -122,6 +131,7 @@ end # Indexes # # index_uploads_on_sha1 (sha1) UNIQUE +# index_uploads_on_url (url) # index_uploads_on_user_id (user_id) # diff --git a/db/migrate/20130622110348_add_url_index_to_uploads.rb b/db/migrate/20130622110348_add_url_index_to_uploads.rb new file mode 100644 index 000000000..6775c7c11 --- /dev/null +++ b/db/migrate/20130622110348_add_url_index_to_uploads.rb @@ -0,0 +1,5 @@ +class AddUrlIndexToUploads < ActiveRecord::Migration + def change + add_index :uploads, :url + end +end diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index ab9c8c3fc..d9c75f0aa 100644 --- a/lib/cooked_post_processor.rb +++ b/lib/cooked_post_processor.rb @@ -85,8 +85,12 @@ class CookedPostProcessor end def get_upload_from_url(url) - if Upload.has_been_uploaded?(url) && m = Upload.uploaded_regex.match(url) - Upload.where("id = ?", m[:upload_id]).first + if Upload.has_been_uploaded?(url) + if m = LocalStore.uploaded_regex.match(url) + Upload.where(id: m[:upload_id]).first + elsif Upload.is_on_s3?(url) + Upload.where(url: url).first + end end end @@ -160,22 +164,23 @@ class CookedPostProcessor # 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 + # make sure s3 urls have a scheme (otherwise, FastImage will fail) + url = "http:" + url if Upload.is_on_s3? (url) + return unless is_valid_image_uri? url + # we can *always* crawl our own images return unless SiteSetting.crawl_images? || Upload.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) + def is_valid_image_uri?(url) uri = URI.parse(url) - uri if %w(http https).include?(uri.scheme) + %w(http https).include? uri.scheme end def dirty? diff --git a/lib/local_store.rb b/lib/local_store.rb index 37539b1f9..f893461da 100644 --- a/lib/local_store.rb +++ b/lib/local_store.rb @@ -20,4 +20,8 @@ module LocalStore rescue Errno::ENOENT end + def self.uploaded_regex + /\/uploads\/#{RailsMultisite::ConnectionManagement.current_db}\/(?\d+)\/[0-9a-f]{16}\.(png|jpg|jpeg|gif|tif|tiff|bmp)/ + end + end diff --git a/lib/s3.rb b/lib/s3.rb index c7c0c791e..a880cfd0e 100644 --- a/lib/s3.rb +++ b/lib/s3.rb @@ -9,7 +9,11 @@ module S3 # if this fails, it will throw an exception file = S3.upload(file, remote_filename, directory) - return "//#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/#{remote_filename}" + "#{S3.base_url}/#{remote_filename}" + end + + def self.base_url + "//#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com" end def self.remove_file(url) diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index 6ace798c3..1467bbd90 100644 --- a/spec/components/cooked_post_processor_spec.rb +++ b/spec/components/cooked_post_processor_spec.rb @@ -164,15 +164,15 @@ describe CookedPostProcessor do end end - context 'get_image_uri' do + context 'is_valid_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 + it "needs the scheme to be either http or https" do + cpp.is_valid_image_uri?("http://domain.com").should == true + cpp.is_valid_image_uri?("https://domain.com").should == true + cpp.is_valid_image_uri?("ftp://domain.com").should == false + cpp.is_valid_image_uri?("ftps://domain.com").should == false + cpp.is_valid_image_uri?("//domain.com").should == false + cpp.is_valid_image_uri?("/tmp/image.png").should == false end end diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 42e22f2fd..dc300dbac 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -71,8 +71,15 @@ describe Upload do Upload.has_been_uploaded?("http://my.cdn.com/upload/1234/42/0123456789ABCDEF.jpg").should == true end + it "identifies S3 uploads" do + SiteSetting.stubs(:enable_s3_uploads).returns(true) + SiteSetting.stubs(:s3_upload_bucket).returns("bucket") + Upload.has_been_uploaded?("//bucket.s3.amazonaws.com/1337.png").should == true + end + it "identifies external urls" do Upload.has_been_uploaded?("http://domain.com/upload/1234/42/0123456789ABCDEF.jpg").should == false + Upload.has_been_uploaded?("//bucket.s3.amazonaws.com/1337.png").should == false end end