diff --git a/app/models/topic_embed.rb b/app/models/topic_embed.rb index d3d1572c5d2..18e154e9779 100644 --- a/app/models/topic_embed.rb +++ b/app/models/topic_embed.rb @@ -132,7 +132,7 @@ class TopicEmbed < ActiveRecord::Base response = FetchResponse.new begin - html = uri.read(allow_redirections: :safe) + html = uri.read rescue OpenURI::HTTPError, Net::OpenTimeout return end diff --git a/config/initializers/003-sql_builder.rb b/config/initializers/003-sql_builder.rb deleted file mode 100644 index a7be88c2942..00000000000 --- a/config/initializers/003-sql_builder.rb +++ /dev/null @@ -1,3 +0,0 @@ -# frozen_string_literal: true - -require 'sql_builder' diff --git a/lib/email/sender.rb b/lib/email/sender.rb index e91adecd793..b20c69a94b8 100644 --- a/lib/email/sender.rb +++ b/lib/email/sender.rb @@ -466,8 +466,7 @@ module Email # via group SMTP and if reply by email site settings are configured return if !user_id || !post_id || !header_value(Email::MessageBuilder::ALLOW_REPLY_BY_EMAIL_HEADER).present? - # use safe variant here cause we tend to see concurrency issue - reply_key = PostReplyKey.find_or_create_by_safe!( + PostReplyKey.create_or_find_by!( post_id: post_id, user_id: user_id ).reply_key diff --git a/lib/freedom_patches/active_record_attribute_methods.rb b/lib/freedom_patches/active_record_attribute_methods.rb index 8144cba19b4..fbf07b90ab7 100644 --- a/lib/freedom_patches/active_record_attribute_methods.rb +++ b/lib/freedom_patches/active_record_attribute_methods.rb @@ -12,14 +12,9 @@ module ActiveRecord module AttributeMethods module ClassMethods - # this is for Rails 5 - def enforce_raw_sql_whitelist(*args) - nil - end - BLANK_ARRAY = [].freeze - # this patch just allows everything in Rails 6 + # this patch just allows everything in Rails 6+ def disallow_raw_sql!(args, permit: nil) # we may consider moving to https://github.com/rails/rails/pull/33330 # once all frozen string hints are in place diff --git a/lib/freedom_patches/active_record_base.rb b/lib/freedom_patches/active_record_base.rb deleted file mode 100644 index f4909ae1f80..00000000000 --- a/lib/freedom_patches/active_record_base.rb +++ /dev/null @@ -1,57 +0,0 @@ -# frozen_string_literal: true - -class ActiveRecord::Base - - # Handle PG::UniqueViolation as well due to concurrency - # find_or_create does find_by(hash) || create!(hash) - # in some cases find will not find and multiple creates will be called - # - # note: Rails 6 has: https://github.com/rails/rails/blob/c83e30da27eafde79164ecb376e8a28ccc8d841f/activerecord/lib/active_record/relation.rb#L171-L201 - # This means that in Rails 6 we would either use: - # - # create_or_find_by! (if we are generally creating) - # - # OR - # - # find_by(hash) || create_or_find_by(hash) (if we are generally finding) - def self.find_or_create_by_safe!(hash) - begin - find_or_create_by!(hash) - rescue PG::UniqueViolation, ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid - # try again cause another transaction could have passed by now - find_or_create_by!(hash) - end - end - - # Execute SQL manually - def self.exec_sql(*args) - - Discourse.deprecate("exec_sql should not be used anymore, please use DB.exec or DB.query instead!", drop_from: '2.9.0') - - conn = ActiveRecord::Base.connection - sql = ActiveRecord::Base.public_send(:sanitize_sql_array, args) - conn.raw_connection.async_exec(sql) - end - - def exec_sql(*args) - ActiveRecord::Base.exec_sql(*args) - end - - # Executes the given block +retries+ times (or forever, if explicitly given nil), - # catching and retrying SQL Deadlock errors. - # - # Thanks to: http://stackoverflow.com/a/7427186/165668 - # - def self.retry_lock_error(retries = 5, &block) - begin - yield - rescue ActiveRecord::StatementInvalid => e - if e.message =~ /deadlock detected/ && (retries.nil? || retries > 0) - retry_lock_error(retries ? retries - 1 : nil, &block) - else - raise e - end - end - end - -end diff --git a/lib/freedom_patches/fast_pluck.rb b/lib/freedom_patches/fast_pluck.rb index a9faa87b320..ef467c56664 100644 --- a/lib/freedom_patches/fast_pluck.rb +++ b/lib/freedom_patches/fast_pluck.rb @@ -51,7 +51,6 @@ class ActiveRecord::Relation relation = apply_join_dependency relation.pluck(*column_names) else - enforce_raw_sql_whitelist(column_names) relation = spawn relation.select_values = column_names diff --git a/lib/freedom_patches/open_uri_redirections.rb b/lib/freedom_patches/open_uri_redirections.rb deleted file mode 100644 index 5886fcc0652..00000000000 --- a/lib/freedom_patches/open_uri_redirections.rb +++ /dev/null @@ -1,100 +0,0 @@ -# frozen_string_literal: true - -##### -# Patch to allow open-uri to follow safe (http to https) -# and unsafe redirections (https to http). -# -# Original gist URL: -# https://gist.github.com/1271420 -# -# Relevant issue: -# https://redmine.ruby-lang.org/issues/3719 -# -# Source here: -# https://github.com/ruby/ruby/blob/trunk/lib/open-uri.rb -# -# Thread-safe implementation adapted from: -# https://github.com/obfusk/open_uri_w_redirect_to_https -# -# Copy and pasted from: -# https://github.com/open-uri-redirections/open_uri_redirections -# - -require "open-uri" - -module OpenURI - class << self - alias_method :open_uri_original, :open_uri - alias_method :redirectable_cautious?, :redirectable? - - def redirectable?(uri1, uri2) - case Thread.current[:__open_uri_redirections__] - when :safe - redirectable_safe? uri1, uri2 - when :all - redirectable_all? uri1, uri2 - else - redirectable_cautious? uri1, uri2 - end - end - - def redirectable_safe?(uri1, uri2) - redirectable_cautious?(uri1, uri2) || http_to_https?(uri1, uri2) - end - - def redirectable_all?(uri1, uri2) - redirectable_safe?(uri1, uri2) || https_to_http?(uri1, uri2) - end - end - - ##### - # Patches the original open_uri method, so that it accepts the - # :allow_redirections argument with these options: - # - # * :safe will allow HTTP => HTTPS redirections. - # * :all will allow HTTP => HTTPS and HTTPS => HTTP redirections. - # - # OpenURI::open can receive different kinds of arguments, like a string for - # the mode or an integer for the permissions, and then a hash with options - # like UserAgent, etc. - # - # To find the :allow_redirections option, we look for this options hash. - # - def self.open_uri(name, *rest, &block) - Thread.current[:__open_uri_redirections__] = allow_redirections(rest) - - block2 = lambda do |io| - Thread.current[:__open_uri_redirections__] = nil - block[io] - end - - begin - open_uri_original name, *rest, &(block ? block2 : nil) - ensure - Thread.current[:__open_uri_redirections__] = nil - end - end - - private - - def self.allow_redirections(args) - options = first_hash_argument(args) - options.delete :allow_redirections if options - end - - def self.first_hash_argument(arguments) - arguments.select { |arg| arg.is_a? Hash }.first - end - - def self.http_to_https?(uri1, uri2) - schemes_from([uri1, uri2]) == %w(http https) - end - - def self.https_to_http?(uri1, uri2) - schemes_from([uri1, uri2]) == %w(https http) - end - - def self.schemes_from(uris) - uris.map { |u| u.scheme.downcase } - end -end diff --git a/lib/freedom_patches/safe_buffer.rb b/lib/freedom_patches/safe_buffer.rb index 545096e0d77..5bc2feb01e4 100644 --- a/lib/freedom_patches/safe_buffer.rb +++ b/lib/freedom_patches/safe_buffer.rb @@ -5,42 +5,32 @@ # # The alternative is a broken website when this happens -class ActiveSupport::SafeBuffer - def concat(value, raise_encoding_err = false) - if !html_safe? || value.html_safe? +module FreedomPatches + module SafeBuffer + def concat(value, raise_encoding_err = false) super(value) - else - super(ERB::Util.h(value)) - end - rescue Encoding::CompatibilityError - if raise_encoding_err - raise - else + rescue Encoding::CompatibilityError + raise if raise_encoding_err encoding_diags = +"internal encoding #{Encoding.default_internal}, external encoding #{Encoding.default_external}" - if encoding != Encoding::UTF_8 encoding_diags << " my encoding is #{encoding} " - - self.force_encoding("UTF-8") + force_encoding("UTF-8") unless valid_encoding? encode!("utf-16", "utf-8", invalid: :replace) encode!("utf-8", "utf-16") end Rails.logger.warn("Encountered a non UTF-8 string in SafeBuffer - #{self} - #{encoding_diags}") end - if value.encoding != Encoding::UTF_8 - encoding_diags << " attempted to append encoding #{value.encoding} " - value = value.dup.force_encoding("UTF-8").scrub Rails.logger.warn("Attempted to concat a non UTF-8 string in SafeBuffer - #{value} - #{encoding_diags}") end - concat(value, _raise = true) end - end - alias << concat + ActiveSupport::SafeBuffer.prepend(self) + ActiveSupport::SafeBuffer.class_eval("alias << concat") + end end diff --git a/lib/freedom_patches/schema_migration_details.rb b/lib/freedom_patches/schema_migration_details.rb index 5b06a81c26a..6b72cc8fdcf 100644 --- a/lib/freedom_patches/schema_migration_details.rb +++ b/lib/freedom_patches/schema_migration_details.rb @@ -48,9 +48,6 @@ SQL rval end + ActiveRecord::Migration.prepend(self) end end - -class ActiveRecord::Migration - prepend FreedomPatches::SchemaMigrationDetails -end diff --git a/lib/freedom_patches/sprockets_patches.rb b/lib/freedom_patches/sprockets_patches.rb index eb85a23ca60..d0cb960d306 100644 --- a/lib/freedom_patches/sprockets_patches.rb +++ b/lib/freedom_patches/sprockets_patches.rb @@ -8,41 +8,8 @@ # 2. Stop using a concatenator that does tons of work checking for semicolons when # when rebuilding an asset -if Rails.env.development? || Rails.env.test? - module ActionView::Helpers::AssetUrlHelper - - def asset_path(source, options = {}) - source = source.to_s - return "" unless source.present? - return source if source =~ URI_REGEXP - - tail, source = source[/([\?#].+)$/], source.sub(/([\?#].+)$/, '') - - if extname = compute_asset_extname(source, options) - source = "#{source}#{extname}" - end - - if source[0] != ?/ - # CODE REMOVED - # source = compute_asset_path(source, options) - source = "/assets/#{source}" - end - - relative_url_root = defined?(config.relative_url_root) && config.relative_url_root - if relative_url_root - source = File.join(relative_url_root, source) unless source.starts_with?("#{relative_url_root}/") - end - - if host = compute_asset_host(source, options) - source = File.join(host, source) - end - - "#{source}#{tail}" - end - alias_method :path_to_asset, :asset_path # aliased to avoid conflicts with an asset_path named route - end - - module ::SprocketHack +module FreedomPatches + module SprocketsPatches def self.concat_javascript_sources(buf, source) if buf.bytesize > 0 # CODE REMOVED HERE @@ -51,8 +18,18 @@ if Rails.env.development? || Rails.env.test? end buf << source end + + if Rails.env.development? || Rails.env.test? + Sprockets.register_bundle_metadata_reducer 'application/javascript', :data, proc { +"" }, method(:concat_javascript_sources) + end + end +end + +if Rails.env.development? || Rails.env.test? + ActiveSupport.on_load(:action_view) do + def compute_asset_path(source, _options = {}) + "/assets/#{source}" + end + alias_method :public_compute_asset_path, :compute_asset_path end - - Sprockets.register_bundle_metadata_reducer 'application/javascript', :data, proc { +"" }, ::SprocketHack.method(:concat_javascript_sources) - end diff --git a/lib/sql_builder.rb b/lib/sql_builder.rb deleted file mode 100644 index 1517cc812ec..00000000000 --- a/lib/sql_builder.rb +++ /dev/null @@ -1,121 +0,0 @@ -# frozen_string_literal: true - -class SqlBuilder - - def initialize(template, klass = nil) - - Discourse.deprecate("SqlBuilder is deprecated and will be removed, please use DB.build instead!", drop_from: '2.9.0') - - @args = {} - @sql = template - @sections = {} - @klass = klass - end - - [:set, :where2, :where, :order_by, :limit, :left_join, :join, :offset, :select].each do |k| - define_method k do |data, args = {}| - @args.merge!(args) - @sections[k] ||= [] - @sections[k] << data - self - end - end - - def secure_category(secure_category_ids, category_alias = 'c') - if secure_category_ids.present? - where("NOT COALESCE(" << category_alias << ".read_restricted, false) OR " << category_alias << ".id IN (:secure_category_ids)", secure_category_ids: secure_category_ids) - else - where("NOT COALESCE(" << category_alias << ".read_restricted, false)") - end - self - end - - def to_sql - sql = @sql.dup - - @sections.each do |k, v| - joined = nil - case k - when :select - joined = "SELECT " << v.join(" , ") - when :where, :where2 - joined = "WHERE " << v.map { |c| "(" << c << ")" }.join(" AND ") - when :join - joined = v.map { |item| "JOIN " << item }.join("\n") - when :left_join - joined = v.map { |item| "LEFT JOIN " << item }.join("\n") - when :limit - joined = "LIMIT " << v.last.to_s - when :offset - joined = "OFFSET " << v.last.to_s - when :order_by - joined = "ORDER BY " << v.join(" , ") - when :set - joined = "SET " << v.join(" , ") - end - - sql.sub!("/*#{k}*/", joined) - end - sql - end - - def exec(args = {}) - @args.merge!(args) - - sql = to_sql - if @klass - @klass.find_by_sql(ActiveRecord::Base.public_send(:sanitize_sql_array, [sql, @args])) - else - if @args == {} - ActiveRecord::Base.exec_sql(sql) - else - ActiveRecord::Base.exec_sql(sql, @args) - end - end - end - - def self.map_exec(klass, sql, args = {}) - self.new(sql).map_exec(klass, args) - end - - class RailsDateTimeDecoder < PG::SimpleDecoder - def decode(string, tuple = nil, field = nil) - @caster ||= ActiveRecord::Type::DateTime.new - @caster.cast(string) - end - end - - class ActiveRecordTypeMap < PG::BasicTypeMapForResults - def initialize(connection) - super(connection) - rm_coder 0, 1114 - add_coder RailsDateTimeDecoder.new(name: "timestamp", oid: 1114, format: 0) - # we don't need deprecations - self.default_type_map = PG::TypeMapInRuby.new - end - end - - def self.pg_type_map - conn = ActiveRecord::Base.connection.raw_connection - @typemap ||= ActiveRecordTypeMap.new(conn) - end - - def map_exec(klass = OpenStruct, args = {}) - results = exec(args) - results.type_map = SqlBuilder.pg_type_map - - setters = results.fields.each_with_index.map do |f, index| - f.dup << "=" - end - - values = results.values - values.map! do |row| - mapped = klass.new - setters.each_with_index do |name, index| - mapped.public_send name, row[index] - end - mapped - end - end - -end diff --git a/spec/lib/migration/column_dropper_spec.rb b/spec/lib/migration/column_dropper_spec.rb index 1aa0a359b04..2684bf17688 100644 --- a/spec/lib/migration/column_dropper_spec.rb +++ b/spec/lib/migration/column_dropper_spec.rb @@ -94,8 +94,8 @@ RSpec.describe Migration::ColumnDropper do SQL expect( - ActiveRecord::Base.exec_sql("SELECT * FROM #{table_name};").values - ).to include([2, "something@email.com"]) + DB.query("SELECT * FROM #{table_name};").first.values + ).to include 2, "something@email.com" end it 'should prevent insertions to the readonly column' do