DEV: Clean up freedom patches

This patch removes some of our freedom patches that have been deprecated
for some time now.
Some of them have been updated so we’re not shipping code based on an
old version of Rails.
This commit is contained in:
Loïc Guitaut 2022-04-05 12:08:28 +02:00 committed by Loïc Guitaut
parent bf3260faea
commit 357011eb3b
12 changed files with 30 additions and 354 deletions

View File

@ -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

View File

@ -1,3 +0,0 @@
# frozen_string_literal: true
require 'sql_builder'

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -48,9 +48,6 @@ SQL
rval
end
ActiveRecord::Migration.prepend(self)
end
end
class ActiveRecord::Migration
prepend FreedomPatches::SchemaMigrationDetails
end

View File

@ -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

View File

@ -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

View File

@ -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