From 5169bcdb6e22d9df1db179c17e360cb146b444a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Thu, 30 Jun 2016 16:55:01 +0200 Subject: [PATCH] FIX: httpshttps ultra secure URLs --- app/models/topic_link_click.rb | 7 +++---- app/models/upload.rb | 4 ++-- lib/cooked_post_processor.rb | 6 +++--- lib/discourse.rb | 32 +++++++------------------------- lib/file_store/base_store.rb | 1 - lib/file_store/local_store.rb | 26 +++++++++++++++----------- lib/file_store/s3_store.rb | 27 +++++++++++++++++---------- lib/pretty_text.rb | 24 ++++++------------------ lib/url_helper.rb | 2 +- 9 files changed, 54 insertions(+), 75 deletions(-) diff --git a/app/models/topic_link_click.rb b/app/models/topic_link_click.rb index 1b31196d95f..e6f4d59ea07 100644 --- a/app/models/topic_link_click.rb +++ b/app/models/topic_link_click.rb @@ -35,7 +35,7 @@ class TopicLinkClick < ActiveRecord::Base cdn_uri = URI.parse(Discourse.asset_host) rescue nil if cdn_uri && cdn_uri.hostname == uri.hostname && uri.path.starts_with?(cdn_uri.path) is_cdn_link = true - urls << uri.path[(cdn_uri.path.length)..-1] + urls << uri.path[cdn_uri.path.length..-1] end end @@ -43,8 +43,7 @@ class TopicLinkClick < ActiveRecord::Base cdn_uri = URI.parse(SiteSetting.s3_cdn_url) rescue nil if cdn_uri && cdn_uri.hostname == uri.hostname && uri.path.starts_with?(cdn_uri.path) is_cdn_link = true - - path = uri.path[(cdn_uri.path.length)..-1] + path = uri.path[cdn_uri.path.length..-1] urls << path urls << "#{Discourse.store.absolute_base_url}#{path}" end @@ -66,7 +65,7 @@ class TopicLinkClick < ActiveRecord::Base # If no link is found... unless link.present? # ... return the url for relative links or when using the same host - return url if url =~ /^\// || uri.try(:host) == Discourse.current_hostname + return url if url =~ /^\/[^\/]/ || uri.try(:host) == Discourse.current_hostname # If we have it somewhere else on the site, just allow the redirect. # This is likely due to a onebox of another topic. diff --git a/app/models/upload.rb b/app/models/upload.rb index 9295a3613a1..149176c29d9 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -223,9 +223,9 @@ class Upload < ActiveRecord::Base def self.get_from_url(url) return if url.blank? # we store relative urls, so we need to remove any host/cdn - url = url.sub(/^#{Discourse.asset_host}/i, "") if Discourse.asset_host.present? + url = url.sub(Discourse.asset_host, "") if Discourse.asset_host.present? # when using s3, we need to replace with the absolute base url - url = url.sub(/^#{SiteSetting.s3_cdn_url}/i, Discourse.store.absolute_base_url) if SiteSetting.s3_cdn_url.present? + url = url.sub(SiteSetting.s3_cdn_url, Discourse.store.absolute_base_url) if SiteSetting.s3_cdn_url.present? Upload.find_by(url: url) end diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index 4e344c5f5c2..427914fcf79 100644 --- a/lib/cooked_post_processor.rb +++ b/lib/cooked_post_processor.rb @@ -325,20 +325,20 @@ class CookedPostProcessor end end - use_s3_cdn = SiteSetting.s3_cdn_url.present? && SiteSetting.enable_s3_uploads + use_s3_cdn = SiteSetting.enable_s3_uploads && SiteSetting.s3_cdn_url.present? %w{href data-download-href}.each do |selector| @doc.css("a[#{selector}]").each do |a| href = a[selector].to_s a[selector] = UrlHelper.schemaless UrlHelper.absolute(href) if UrlHelper.is_local(href) - a[selector] = a[selector].sub(Discourse.store.absolute_base_url, SiteSetting.s3_cdn_url) if use_s3_cdn + a[selector] = Discourse.store.cnd_url(a[selector]) if use_s3_cdn end end @doc.css("img[src]").each do |img| src = img["src"].to_s img["src"] = UrlHelper.schemaless UrlHelper.absolute(src) if UrlHelper.is_local(src) - img["src"] = img["src"].sub(Discourse.store.absolute_base_url, SiteSetting.s3_cdn_url) if use_s3_cdn + img["src"] = Discourse.store.cnd_url(img["src"]) if use_s3_cdn end end diff --git a/lib/discourse.rb b/lib/discourse.rb index 5e9524fae20..61d29889213 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -131,7 +131,7 @@ module Discourse end def self.disabled_plugin_names - plugins.select {|p| !p.enabled?}.map(&:name) + plugins.select { |p| !p.enabled? }.map(&:name) end def self.plugins @@ -179,36 +179,18 @@ module Discourse # Get the current base URL for the current site def self.current_hostname - if SiteSetting.force_hostname.present? - SiteSetting.force_hostname - else - RailsMultisite::ConnectionManagement.current_hostname - end + SiteSetting.force_hostname.presence || RailsMultisite::ConnectionManagement.current_hostname end def self.base_uri(default_value = "") - if !ActionController::Base.config.relative_url_root.blank? - ActionController::Base.config.relative_url_root - else - default_value - end + ActionController::Base.config.relative_url_root.presence || default_value end def self.base_url_no_prefix - default_port = 80 - protocol = "http" - - if SiteSetting.force_https? - protocol = "https" - default_port = 443 - end - - result = "#{protocol}://#{current_hostname}" - - port = SiteSetting.port.present? && SiteSetting.port.to_i > 0 ? SiteSetting.port.to_i : default_port - - result << ":#{SiteSetting.port}" if port != default_port - result + protocol, default_port = SiteSetting.force_https? ? ["https", 443] : ["http", 80] + url = "#{protocol}://#{current_hostname}" + url << ":#{SiteSetting.port}" if SiteSetting.port.to_i > 0 && SiteSetting.port.to_i != default_port + url end def self.base_url diff --git a/lib/file_store/base_store.rb b/lib/file_store/base_store.rb index 275e78dff73..e88269f7f9d 100644 --- a/lib/file_store/base_store.rb +++ b/lib/file_store/base_store.rb @@ -33,7 +33,6 @@ module FileStore end def cdn_url(url) - url end def absolute_base_url diff --git a/lib/file_store/local_store.rb b/lib/file_store/local_store.rb index 5054c7cc810..3891656b084 100644 --- a/lib/file_store/local_store.rb +++ b/lib/file_store/local_store.rb @@ -11,18 +11,16 @@ module FileStore def remove_file(url) return unless is_relative?(url) - path = public_dir + url - return if !File.exists?(path) - tombstone = public_dir + url.sub("/uploads/", "/uploads/tombstone/") - FileUtils.mkdir_p(tombstone_dir) - FileUtils.move(path, tombstone, force: true) + source = "#{public_dir}#{url}" + return unless File.exists?(source) + destination = "#{public_dir}#{url.sub("/uploads/", "/uploads/tombstone/")}" + dir = Pathname.new(destination).dirname + FileUtils.mkdir_p(dir) unless Dir.exists?(dir) + FileUtils.move(source, destination, force: true) end def has_been_uploaded?(url) - return false if url.blank? - return true if is_relative?(url) - return true if is_local?(url) - false + is_relative?(url) || is_local?(url) end def absolute_base_url @@ -50,6 +48,11 @@ module FileStore "#{relative_base_url}/#{upload.sha1}" end + def cdn_url(url) + return url if Discourse.asset_host.blank? + url.sub(Discourse.base_url_no_prefix, Discourse.asset_host) + end + def path_for(upload) url = upload.try(:url) "#{public_dir}#{upload.url}" if url && url[0] == "/" && url[1] != "/" @@ -64,7 +67,8 @@ module FileStore end def copy_file(file, path) - FileUtils.mkdir_p(Pathname.new(path).dirname) + dir = Pathname.new(path).dirname + FileUtils.mkdir_p(dir) unless Dir.exists?(dir) # move the file to the right location # not using mv, cause permissions are no good on move File.open(path, "wb") { |f| f.write(file.read) } @@ -85,7 +89,7 @@ module FileStore end def tombstone_dir - public_dir + relative_base_url.sub("/uploads/", "/uploads/tombstone/") + "#{public_dir}#{relative_base_url.sub("/uploads/", "/uploads/tombstone/")}" end end diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb index aa9dc1601ec..8b7bb1ae3e7 100644 --- a/lib/file_store/s3_store.rb +++ b/lib/file_store/s3_store.rb @@ -1,3 +1,4 @@ +require "uri" require_dependency "file_store/base_store" require_dependency "file_store/local_store" require_dependency "s3_helper" @@ -23,8 +24,8 @@ module FileStore # - content_type # - cache_locally def store_file(file, path, opts={}) - filename = opts[:filename].presence - content_type = opts[:content_type].presence + filename = opts[:filename].presence + content_type = opts[:content_type].presence # cache file locally when needed cache_file(file, File.basename(path)) if opts[:cache_locally] # stored uploaded are public by default @@ -48,9 +49,13 @@ module FileStore def has_been_uploaded?(url) return false if url.blank? - return true if url.start_with?(absolute_base_url) - return true if SiteSetting.s3_cdn_url.present? && url.start_with?(SiteSetting.s3_cdn_url) - false + + base_hostname = URI.parse(absolute_base_url).hostname + return true if url[base_hostname] + + return false if SiteSetting.s3_cdn_url.blank? + cdn_hostname = URI.parse(SiteSetting.s3_cdn_url || "").hostname + cdn_hostname.presence && url[cdn_hostname] end def absolute_base_url @@ -72,12 +77,13 @@ module FileStore def path_for(upload) url = upload.try(:url) - FileStore::LocalStore.new.path_for(upload) if url && url[0] == "/" && url[1] != "/" + FileStore::LocalStore.new.path_for(upload) if url && url[/^\/[^\/]/] end def cdn_url(url) return url if SiteSetting.s3_cdn_url.blank? - url.sub(absolute_base_url, SiteSetting.s3_cdn_url) + schema = url[/^(https?:)?\/\//, 1] + url.sub("#{schema}#{absolute_base_url}", SiteSetting.s3_cdn_url) end def cache_avatar(avatar, user_id) @@ -91,9 +97,10 @@ module FileStore end def s3_bucket - return @s3_bucket if @s3_bucket - raise Discourse::SiteSettingMissing.new("s3_upload_bucket") if SiteSetting.s3_upload_bucket.blank? - @s3_bucket = SiteSetting.s3_upload_bucket.downcase + @s3_bucket ||= begin + raise Discourse::SiteSettingMissing.new("s3_upload_bucket") if SiteSetting.s3_upload_bucket.blank? + SiteSetting.s3_upload_bucket.downcase + end end end diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 75174a8a826..ab7565f692d 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -37,18 +37,11 @@ module PrettyText UrlHelper.schemaless UrlHelper.absolute avatar_template end - def mention_lookup(username) - return false unless username - if Group.exec_sql('SELECT 1 FROM groups WHERE name = ?', username).values.length == 1 - "group" - else - username = username.downcase - if User.exec_sql('SELECT 1 FROM users WHERE username_lower = ?', username).values.length == 1 - "user" - else - nil - end - end + def mention_lookup(name) + return false if name.blank? + return "group" if Group.where(name: name).exists? + return "user" if User.where(username_lower: name.downcase).exists? + nil end def category_hashtag_lookup(category_slug) @@ -306,12 +299,7 @@ JS def self.add_s3_cdn(doc) doc.css("img").each do |img| next unless img["src"] - if img["src"].include? Discourse.store.absolute_base_url - src = img["src"].sub(Discourse.store.absolute_base_url, SiteSetting.s3_cdn_url) - # absolute is // style so we may have added an extra https:// here - src = src.sub(/https?:h/, "h") - img["src"] = src - end + img["src"] = Discourse.store.cdn_url(img["src"]) end end diff --git a/lib/url_helper.rb b/lib/url_helper.rb index 24f6192ac71..1a8382ecda5 100644 --- a/lib/url_helper.rb +++ b/lib/url_helper.rb @@ -18,7 +18,7 @@ class UrlHelper end def self.schemaless(url) - url.sub(/^http:/, "") + url.sub(/^http:/i, "") end end