diff options
83 files changed, 1130 insertions, 360 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index 033f7adf49..6eca479d9d 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -99,6 +99,9 @@ Layout/SpaceAfterColon: Layout/SpaceAfterComma: Enabled: true +Layout/SpaceAfterSemicolon: + Enabled: true + Layout/SpaceAroundEqualsInParameterDefault: Enabled: true @@ -109,10 +112,10 @@ Layout/SpaceAroundOperators: Enabled: true Layout/SpaceBeforeComma: - Enabled: true + Enabled: true Layout/SpaceBeforeFirstArg: - Enabled: true + Enabled: true Style/DefWithParentheses: Enabled: true diff --git a/.travis.yml b/.travis.yml index 2d0ec1efcf..6acba41c0e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -64,13 +64,13 @@ env: - "GEM=ac:integration" rvm: - - 2.4.4 - - 2.5.1 + - 2.4.5 + - 2.5.3 - ruby-head matrix: include: - - rvm: 2.4.4 + - rvm: 2.4.5 env: "GEM=railties" sudo: required before_install: @@ -79,7 +79,7 @@ matrix: - "travis_retry gem install bundler" - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - "sudo service postgresql restart 10" - - rvm: 2.5.1 + - rvm: 2.5.3 env: "GEM=railties" sudo: required before_install: @@ -97,9 +97,9 @@ matrix: - "travis_retry gem install bundler" - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - "sudo service postgresql restart 10" - - rvm: 2.5.1 + - rvm: 2.5.3 env: "GEM=av:ujs" - - rvm: 2.4.4 + - rvm: 2.4.5 sudo: required env: "GEM=aj:integration" services: @@ -110,9 +110,9 @@ matrix: - sudo sed -i -e '/local.*peer/s/postgres/all/' -e 's/peer\|md5/trust/g' /etc/postgresql/*/main/pg_hba.conf - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - "sudo service postgresql restart 10" - - "[ -f /tmp/beanstalkd-1.10/Makefile ] || (curl -L https://github.com/kr/beanstalkd/archive/v1.10.tar.gz | tar xz -C /tmp)" + - "[ -f /tmp/beanstalkd-1.10/Makefile ] || (curl -L https://github.com/beanstalkd/beanstalkd/archive/v1.10.tar.gz | tar xz -C /tmp)" - "pushd /tmp/beanstalkd-1.10 && make && (./beanstalkd &); popd" - - rvm: 2.5.1 + - rvm: 2.5.3 sudo: required env: "GEM=aj:integration" services: @@ -123,7 +123,7 @@ matrix: - sudo sed -i -e '/local.*peer/s/postgres/all/' -e 's/peer\|md5/trust/g' /etc/postgresql/*/main/pg_hba.conf - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - "sudo service postgresql restart 10" - - "[ -f /tmp/beanstalkd-1.10/Makefile ] || (curl -L https://github.com/kr/beanstalkd/archive/v1.10.tar.gz | tar xz -C /tmp)" + - "[ -f /tmp/beanstalkd-1.10/Makefile ] || (curl -L https://github.com/beanstalkd/beanstalkd/archive/v1.10.tar.gz | tar xz -C /tmp)" - "pushd /tmp/beanstalkd-1.10 && make && (./beanstalkd &); popd" - rvm: ruby-head sudo: required @@ -136,16 +136,16 @@ matrix: - sudo sed -i -e '/local.*peer/s/postgres/all/' -e 's/peer\|md5/trust/g' /etc/postgresql/*/main/pg_hba.conf - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - "sudo service postgresql restart 10" - - "[ -f /tmp/beanstalkd-1.10/Makefile ] || (curl -L https://github.com/kr/beanstalkd/archive/v1.10.tar.gz | tar xz -C /tmp)" + - "[ -f /tmp/beanstalkd-1.10/Makefile ] || (curl -L https://github.com/beanstalkd/beanstalkd/archive/v1.10.tar.gz | tar xz -C /tmp)" - "pushd /tmp/beanstalkd-1.10 && make && (./beanstalkd &); popd" - - rvm: 2.4.4 + - rvm: 2.4.5 env: "GEM=ar:mysql2" sudo: required before_install: - "sudo mysql -e \"use mysql; update user set authentication_string='' where User='root'; update user set plugin='mysql_native_password';FLUSH PRIVILEGES;\"" - "sudo mysql_upgrade" - "sudo service mysql restart" - - rvm: 2.5.1 + - rvm: 2.5.3 env: "GEM=ar:mysql2" sudo: required before_install: @@ -159,21 +159,21 @@ matrix: - "sudo mysql -e \"use mysql; update user set authentication_string='' where User='root'; update user set plugin='mysql_native_password';FLUSH PRIVILEGES;\"" - "sudo mysql_upgrade" - "sudo service mysql restart" - - rvm: 2.5.1 + - rvm: 2.5.3 env: - "GEM=ar:mysql2 MYSQL=mariadb" addons: mariadb: 10.3 - - rvm: 2.5.1 + - rvm: 2.5.3 env: - "GEM=ar:sqlite3_mem" - - rvm: 2.4.4 + - rvm: 2.4.5 env: "GEM=ar:postgresql" sudo: required before_install: - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - "sudo service postgresql restart 10" - - rvm: 2.5.1 + - rvm: 2.5.3 env: "GEM=ar:postgresql" sudo: required before_install: diff --git a/Gemfile.lock b/Gemfile.lock index f4f3e430ba..368e5ce01c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -93,7 +93,7 @@ PATH activesupport (= 6.0.0.alpha) method_source rake (>= 0.8.7) - thor (>= 0.19.0, < 2.0) + thor (>= 0.20.3, < 2.0) GEM remote: https://rubygems.org/ @@ -304,7 +304,7 @@ GEM marcel (0.3.3) mimemagic (~> 0.3.2) memoist (0.16.0) - method_source (0.9.1) + method_source (0.9.2) mime-types (3.2.2) mime-types-data (~> 3.2015) mime-types-data (3.2018.0812) @@ -399,7 +399,7 @@ GEM resque (~> 1.26) rufus-scheduler (~> 3.2) retriable (3.1.2) - rubocop (0.60.0) + rubocop (0.61.1) jaro_winkler (~> 1.5.1) parallel (~> 1.10) parser (>= 2.5, != 2.5.1.1) diff --git a/actioncable/lib/action_cable/channel/base.rb b/actioncable/lib/action_cable/channel/base.rb index 70c93ec0f3..ad0d3685cd 100644 --- a/actioncable/lib/action_cable/channel/base.rb +++ b/actioncable/lib/action_cable/channel/base.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "set" +require "active_support/rescuable" module ActionCable module Channel @@ -99,6 +100,7 @@ module ActionCable include Streams include Naming include Broadcasting + include ActiveSupport::Rescuable attr_reader :params, :connection, :identifier delegate :logger, to: :connection @@ -267,6 +269,8 @@ module ActionCable else public_send action end + rescue Exception => exception + rescue_with_handler(exception) || raise end def action_signature(action, data) diff --git a/actioncable/test/channel/base_test.rb b/actioncable/test/channel/base_test.rb index eb0e1673b0..c2968226c7 100644 --- a/actioncable/test/channel/base_test.rb +++ b/actioncable/test/channel/base_test.rb @@ -26,6 +26,9 @@ class ActionCable::Channel::BaseTest < ActionCable::TestCase after_subscribe :toggle_subscribed after_unsubscribe :toggle_subscribed + class SomeCustomError < StandardError; end + rescue_from SomeCustomError, with: :error_handler + def initialize(*) @subscribed = false super @@ -68,10 +71,18 @@ class ActionCable::Channel::BaseTest < ActionCable::TestCase @last_action = [ :receive ] end + def error_action + raise SomeCustomError + end + private def rm_rf @last_action = [ :rm_rf ] end + + def error_handler + @last_action = [ :error_action ] + end end setup do @@ -168,7 +179,7 @@ class ActionCable::Channel::BaseTest < ActionCable::TestCase end test "actions available on Channel" do - available_actions = %w(room last_action subscribed unsubscribed toggle_subscribed leave speak subscribed? get_latest receive chatters topic).to_set + available_actions = %w(room last_action subscribed unsubscribed toggle_subscribed leave speak subscribed? get_latest receive chatters topic error_action).to_set assert_equal available_actions, ChatChannel.action_methods end @@ -256,6 +267,11 @@ class ActionCable::Channel::BaseTest < ActionCable::TestCase end end + test "behaves like rescuable" do + @channel.perform_action "action" => :error_action + assert_equal [ :error_action ], @channel.last_action + end + private def assert_logged(message) old_logger = @connection.logger diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 740c6db06f..13fbbafc0c 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,31 @@ +* Introduce ActionDispatch::HostAuthorization + + This is a new middleware that guards against DNS rebinding attacks by + white-listing the allowed hosts a request can be made to. + + Each host is checked with the case operator (`#===`) to support `RegExp`, + `Proc`, `IPAddr` and custom objects as host allowances. + + *Genadi Samokovarov* + +* Allow using `parsed_body` in `ActionController::TestCase`. + + In addition to `ActionDispatch::IntegrationTest`, allow using + `parsed_body` in `ActionController::TestCase`: + + ``` + class SomeControllerTest < ActionController::TestCase + def test_some_action + post :action, body: { foo: 'bar' } + assert_equal({ "foo" => "bar" }, response.parsed_body) + end + end + ``` + + Fixes #34676. + + *Tobias Bühlmann* + * Raise an error on root route naming conflicts. Raises an ArgumentError when multiple root routes are defined in the diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 0822cdc0a6..a92d336214 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -49,11 +49,13 @@ module ActionDispatch end autoload_under "middleware" do + autoload :HostAuthorization autoload :RequestId autoload :Callbacks autoload :Cookies autoload :DebugExceptions autoload :DebugLocks + autoload :DebugView autoload :ExceptionWrapper autoload :Executor autoload :Flash diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 3af4c176a7..8227749986 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -79,108 +79,108 @@ module ActionDispatch private - def add_params(path, params) - params = { params: params } unless params.is_a?(Hash) - params.reject! { |_, v| v.to_param.nil? } - query = params.to_query - path << "?#{query}" unless query.empty? - end - - def add_anchor(path, anchor) - if anchor - path << "##{Journey::Router::Utils.escape_fragment(anchor.to_param)}" + def add_params(path, params) + params = { params: params } unless params.is_a?(Hash) + params.reject! { |_, v| v.to_param.nil? } + query = params.to_query + path << "?#{query}" unless query.empty? end - end - def extract_domain_from(host, tld_length) - host.split(".").last(1 + tld_length).join(".") - end + def add_anchor(path, anchor) + if anchor + path << "##{Journey::Router::Utils.escape_fragment(anchor.to_param)}" + end + end - def extract_subdomains_from(host, tld_length) - parts = host.split(".") - parts[0..-(tld_length + 2)] - end + def extract_domain_from(host, tld_length) + host.split(".").last(1 + tld_length).join(".") + end - def add_trailing_slash(path) - if path.include?("?") - path.sub!(/\?/, '/\&') - elsif !path.include?(".") - path.sub!(/[^\/]\z|\A\z/, '\&/') + def extract_subdomains_from(host, tld_length) + parts = host.split(".") + parts[0..-(tld_length + 2)] end - end - def build_host_url(host, port, protocol, options, path) - if match = host.match(HOST_REGEXP) - protocol ||= match[1] unless protocol == false - host = match[2] - port = match[3] unless options.key? :port + def add_trailing_slash(path) + if path.include?("?") + path.sub!(/\?/, '/\&') + elsif !path.include?(".") + path.sub!(/[^\/]\z|\A\z/, '\&/') + end end - protocol = normalize_protocol protocol - host = normalize_host(host, options) + def build_host_url(host, port, protocol, options, path) + if match = host.match(HOST_REGEXP) + protocol ||= match[1] unless protocol == false + host = match[2] + port = match[3] unless options.key? :port + end - result = protocol.dup + protocol = normalize_protocol protocol + host = normalize_host(host, options) - if options[:user] && options[:password] - result << "#{Rack::Utils.escape(options[:user])}:#{Rack::Utils.escape(options[:password])}@" - end + result = protocol.dup - result << host - normalize_port(port, protocol) { |normalized_port| - result << ":#{normalized_port}" - } + if options[:user] && options[:password] + result << "#{Rack::Utils.escape(options[:user])}:#{Rack::Utils.escape(options[:password])}@" + end - result.concat path - end + result << host + normalize_port(port, protocol) { |normalized_port| + result << ":#{normalized_port}" + } - def named_host?(host) - IP_HOST_REGEXP !~ host - end + result.concat path + end - def normalize_protocol(protocol) - case protocol - when nil - "http://" - when false, "//" - "//" - when PROTOCOL_REGEXP - "#{$1}://" - else - raise ArgumentError, "Invalid :protocol option: #{protocol.inspect}" + def named_host?(host) + IP_HOST_REGEXP !~ host end - end - def normalize_host(_host, options) - return _host unless named_host?(_host) + def normalize_protocol(protocol) + case protocol + when nil + "http://" + when false, "//" + "//" + when PROTOCOL_REGEXP + "#{$1}://" + else + raise ArgumentError, "Invalid :protocol option: #{protocol.inspect}" + end + end - tld_length = options[:tld_length] || @@tld_length - subdomain = options.fetch :subdomain, true - domain = options[:domain] + def normalize_host(_host, options) + return _host unless named_host?(_host) - host = +"" - if subdomain == true - return _host if domain.nil? + tld_length = options[:tld_length] || @@tld_length + subdomain = options.fetch :subdomain, true + domain = options[:domain] - host << extract_subdomains_from(_host, tld_length).join(".") - elsif subdomain - host << subdomain.to_param + host = +"" + if subdomain == true + return _host if domain.nil? + + host << extract_subdomains_from(_host, tld_length).join(".") + elsif subdomain + host << subdomain.to_param + end + host << "." unless host.empty? + host << (domain || extract_domain_from(_host, tld_length)) + host end - host << "." unless host.empty? - host << (domain || extract_domain_from(_host, tld_length)) - host - end - def normalize_port(port, protocol) - return unless port + def normalize_port(port, protocol) + return unless port - case protocol - when "//" then yield port - when "https://" - yield port unless port.to_i == 443 - else - yield port unless port.to_i == 80 + case protocol + when "//" then yield port + when "https://" + yield port unless port.to_i == 443 + else + yield port unless port.to_i == 80 + end end - end end def initialize diff --git a/actionpack/lib/action_dispatch/journey/nodes/node.rb b/actionpack/lib/action_dispatch/journey/nodes/node.rb index 32f632800c..086d6a3e07 100644 --- a/actionpack/lib/action_dispatch/journey/nodes/node.rb +++ b/actionpack/lib/action_dispatch/journey/nodes/node.rb @@ -65,12 +65,12 @@ module ActionDispatch def literal?; false; end end - %w{ Symbol Slash Dot }.each do |t| - class_eval <<-eoruby, __FILE__, __LINE__ + 1 - class #{t} < Terminal; - def type; :#{t.upcase}; end - end - eoruby + class Slash < Terminal # :nodoc: + def type; :SLASH; end + end + + class Dot < Terminal # :nodoc: + def type; :DOT; end end class Symbol < Terminal # :nodoc: @@ -89,6 +89,7 @@ module ActionDispatch regexp == DEFAULT_EXP end + def type; :SYMBOL; end def symbol?; true; end end diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 7669767ae3..eadb59173d 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -3,53 +3,14 @@ require "action_dispatch/http/request" require "action_dispatch/middleware/exception_wrapper" require "action_dispatch/routing/inspector" + require "action_view" require "action_view/base" -require "pp" - module ActionDispatch # This middleware is responsible for logging exceptions and # showing a debugging page in case the request is local. class DebugExceptions - RESCUES_TEMPLATE_PATH = File.expand_path("templates", __dir__) - - class DebugView < ActionView::Base - def debug_params(params) - clean_params = params.clone - clean_params.delete("action") - clean_params.delete("controller") - - if clean_params.empty? - "None" - else - PP.pp(clean_params, +"", 200) - end - end - - def debug_headers(headers) - if headers.present? - headers.inspect.gsub(",", ",\n") - else - "None" - end - end - - def debug_hash(object) - object.to_hash.sort_by { |k, _| k.to_s }.map { |k, v| "#{k}: #{v.inspect rescue $!.message}" }.join("\n") - end - - def render(*) - logger = ActionView::Base.logger - - if logger && logger.respond_to?(:silence) - logger.silence { super } - else - super - end - end - end - cattr_reader :interceptors, instance_accessor: false, default: [] def self.register_interceptor(object = nil, &block) @@ -152,7 +113,7 @@ module ActionDispatch end def create_template(request, wrapper) - DebugView.new([RESCUES_TEMPLATE_PATH], + DebugView.new( request: request, exception_wrapper: wrapper, exception: wrapper.exception, diff --git a/actionpack/lib/action_dispatch/middleware/debug_view.rb b/actionpack/lib/action_dispatch/middleware/debug_view.rb new file mode 100644 index 0000000000..ac12dc13a1 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/debug_view.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require "pp" + +require "action_view" +require "action_view/base" + +module ActionDispatch + class DebugView < ActionView::Base # :nodoc: + RESCUES_TEMPLATE_PATH = File.expand_path("templates", __dir__) + + def initialize(assigns) + super([RESCUES_TEMPLATE_PATH], assigns) + end + + def debug_params(params) + clean_params = params.clone + clean_params.delete("action") + clean_params.delete("controller") + + if clean_params.empty? + "None" + else + PP.pp(clean_params, +"", 200) + end + end + + def debug_headers(headers) + if headers.present? + headers.inspect.gsub(",", ",\n") + else + "None" + end + end + + def debug_hash(object) + object.to_hash.sort_by { |k, _| k.to_s }.map { |k, v| "#{k}: #{v.inspect rescue $!.message}" }.join("\n") + end + + def render(*) + logger = ActionView::Base.logger + + if logger && logger.respond_to?(:silence) + logger.silence { super } + else + super + end + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/host_authorization.rb b/actionpack/lib/action_dispatch/middleware/host_authorization.rb new file mode 100644 index 0000000000..48f7c25216 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/host_authorization.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +require "action_dispatch/http/request" + +module ActionDispatch + # This middleware guards from DNS rebinding attacks by white-listing the + # hosts a request can be sent to. + # + # When a request comes to an unauthorized host, the +response_app+ + # application will be executed and rendered. If no +response_app+ is given, a + # default one will run, which responds with +403 Forbidden+. + class HostAuthorization + class Permissions # :nodoc: + def initialize(hosts) + @hosts = sanitize_hosts(hosts) + end + + def empty? + @hosts.empty? + end + + def allows?(host) + @hosts.any? do |allowed| + begin + allowed === host + rescue + # IPAddr#=== raises an error if you give it a hostname instead of + # IP. Treat similar errors as blocked access. + false + end + end + end + + private + + def sanitize_hosts(hosts) + Array(hosts).map do |host| + case host + when Regexp then sanitize_regexp(host) + when String then sanitize_string(host) + else host + end + end + end + + def sanitize_regexp(host) + /\A#{host}\z/ + end + + def sanitize_string(host) + if host.start_with?(".") + /\A(.+\.)?#{Regexp.escape(host[1..-1])}\z/ + else + host + end + end + end + + DEFAULT_RESPONSE_APP = -> env do + request = Request.new(env) + + format = request.xhr? ? "text/plain" : "text/html" + template = DebugView.new(host: request.host) + body = template.render(template: "rescues/blocked_host", layout: "rescues/layout") + + [403, { + "Content-Type" => "#{format}; charset=#{Response.default_charset}", + "Content-Length" => body.bytesize.to_s, + }, [body]] + end + + def initialize(app, hosts, response_app = nil) + @app = app + @permissions = Permissions.new(hosts) + @response_app = response_app || DEFAULT_RESPONSE_APP + end + + def call(env) + return @app.call(env) if @permissions.empty? + + request = Request.new(env) + + if authorized?(request) + mark_as_authorized(request) + @app.call(env) + else + @response_app.call(env) + end + end + + private + + def authorized?(request) + origin_host = request.get_header("HTTP_HOST").to_s.sub(/:\d+\z/, "") + forwarded_host = request.x_forwarded_host.to_s.split(/,\s?/).last.to_s.sub(/:\d+\z/, "") + + @permissions.allows?(origin_host) && + (forwarded_host.blank? || @permissions.allows?(forwarded_host)) + end + + def mark_as_authorized(request) + request.set_header("action_dispatch.authorized_host", request.host) + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.html.erb new file mode 100644 index 0000000000..2fa78dd385 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.html.erb @@ -0,0 +1,7 @@ +<header> + <h1>Blocked host: <%= @host %></h1> +</header> +<div id="container"> + <h2>To allow requests to <%= @host %>, add the following configuration:</h2> + <pre>Rails.application.config.hosts << "<%= @host %>"</pre> +</div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.text.erb new file mode 100644 index 0000000000..4e2d1d0b08 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.text.erb @@ -0,0 +1,5 @@ +Blocked host: <%= @host %> + +To allow requests to <%= @host %>, add the following configuration: + + Rails.application.config.hosts << "<%= @host %>" diff --git a/actionpack/lib/action_dispatch/testing/test_response.rb b/actionpack/lib/action_dispatch/testing/test_response.rb index 1e6b21f235..7c1202dc0e 100644 --- a/actionpack/lib/action_dispatch/testing/test_response.rb +++ b/actionpack/lib/action_dispatch/testing/test_response.rb @@ -14,11 +14,6 @@ module ActionDispatch new response.status, response.headers, response.body end - def initialize(*) # :nodoc: - super - @response_parser = RequestEncoder.parser(content_type) - end - # Was the response successful? def success? ActiveSupport::Deprecation.warn(<<-MSG.squish) @@ -47,7 +42,11 @@ module ActionDispatch end def parsed_body - @parsed_body ||= @response_parser.call(body) + @parsed_body ||= response_parser.call(body) + end + + def response_parser + @response_parser ||= RequestEncoder.parser(content_type) end end end diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index 6fc70d6248..c931e2daac 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -156,6 +156,10 @@ XML render html: '<body class="foo"></body>'.html_safe end + def render_json + render json: request.raw_post + end + def boom raise "boom!" end @@ -965,6 +969,16 @@ XML assert_equal "q=test2", @response.body end + + def test_parsed_body_without_as_option + post :render_json, body: { foo: "heyo" } + assert_equal({ "foo" => "heyo" }, response.parsed_body) + end + + def test_parsed_body_with_as_option + post :render_json, body: { foo: "heyo" }, as: :json + assert_equal({ "foo" => "heyo" }, response.parsed_body) + end end class ResponseDefaultHeadersTest < ActionController::TestCase diff --git a/actionpack/test/dispatch/host_authorization_test.rb b/actionpack/test/dispatch/host_authorization_test.rb new file mode 100644 index 0000000000..dcb59ddb94 --- /dev/null +++ b/actionpack/test/dispatch/host_authorization_test.rb @@ -0,0 +1,160 @@ +# frozen_string_literal: true + +require "abstract_unit" + +class HostAuthorizationTest < ActionDispatch::IntegrationTest + App = -> env { [200, {}, %w(Success)] } + + test "blocks requests to unallowed host" do + @app = ActionDispatch::HostAuthorization.new(App, %w(only.com)) + + get "/" + + assert_response :forbidden + assert_match "Blocked host: www.example.com", response.body + end + + test "passes all requests to if the whitelist is empty" do + @app = ActionDispatch::HostAuthorization.new(App, nil) + + get "/" + + assert_response :ok + assert_equal "Success", body + end + + test "passes requests to allowed host" do + @app = ActionDispatch::HostAuthorization.new(App, %w(www.example.com)) + + get "/" + + assert_response :ok + assert_equal "Success", body + end + + test "the whitelist could be a single element" do + @app = ActionDispatch::HostAuthorization.new(App, "www.example.com") + + get "/" + + assert_response :ok + assert_equal "Success", body + end + + test "passes requests to allowed hosts with domain name notation" do + @app = ActionDispatch::HostAuthorization.new(App, ".example.com") + + get "/" + + assert_response :ok + assert_equal "Success", body + end + + test "does not allow domain name notation in the HOST header itself" do + @app = ActionDispatch::HostAuthorization.new(App, ".example.com") + + get "/", env: { + "HOST" => ".example.com", + } + + assert_response :forbidden + assert_match "Blocked host: .example.com", response.body + end + + test "checks for requests with #=== to support wider range of host checks" do + @app = ActionDispatch::HostAuthorization.new(App, [-> input { input == "www.example.com" }]) + + get "/" + + assert_response :ok + assert_equal "Success", body + end + + test "mark the host when authorized" do + @app = ActionDispatch::HostAuthorization.new(App, ".example.com") + + get "/" + + assert_equal "www.example.com", request.get_header("action_dispatch.authorized_host") + end + + test "sanitizes regular expressions to prevent accidental matches" do + @app = ActionDispatch::HostAuthorization.new(App, [/w.example.co/]) + + get "/" + + assert_response :forbidden + assert_match "Blocked host: www.example.com", response.body + end + + test "blocks requests to unallowed host supporting custom responses" do + @app = ActionDispatch::HostAuthorization.new(App, ["w.example.co"], -> env do + [401, {}, %w(Custom)] + end) + + get "/" + + assert_response :unauthorized + assert_equal "Custom", body + end + + test "blocks requests with spoofed X-FORWARDED-HOST" do + @app = ActionDispatch::HostAuthorization.new(App, [IPAddr.new("127.0.0.1")]) + + get "/", env: { + "HTTP_X_FORWARDED_HOST" => "127.0.0.1", + "HOST" => "www.example.com", + } + + assert_response :forbidden + assert_match "Blocked host: 127.0.0.1", response.body + end + + test "does not consider IP addresses in X-FORWARDED-HOST spoofed when disabled" do + @app = ActionDispatch::HostAuthorization.new(App, nil) + + get "/", env: { + "HTTP_X_FORWARDED_HOST" => "127.0.0.1", + "HOST" => "www.example.com", + } + + assert_response :ok + assert_equal "Success", body + end + + test "detects localhost domain spoofing" do + @app = ActionDispatch::HostAuthorization.new(App, "localhost") + + get "/", env: { + "HTTP_X_FORWARDED_HOST" => "localhost", + "HOST" => "www.example.com", + } + + assert_response :forbidden + assert_match "Blocked host: localhost", response.body + end + + test "forwarded hosts should be permitted" do + @app = ActionDispatch::HostAuthorization.new(App, "domain.com") + + get "/", env: { + "HTTP_X_FORWARDED_HOST" => "sub.domain.com", + "HOST" => "domain.com", + } + + assert_response :forbidden + assert_match "Blocked host: sub.domain.com", response.body + end + + test "forwarded hosts are allowed when permitted" do + @app = ActionDispatch::HostAuthorization.new(App, ".domain.com") + + get "/", env: { + "HTTP_X_FORWARDED_HOST" => "sub.domain.com", + "HOST" => "domain.com", + } + + assert_response :ok + assert_equal "Success", body + end +end diff --git a/actionview/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb index 4e5fdfbb2d..cb4327cf16 100644 --- a/actionview/lib/action_view/rendering.rb +++ b/actionview/lib/action_view/rendering.rb @@ -64,10 +64,11 @@ module ActionView # An instance of a view class. The default view class is ActionView::Base. # # The view class must have the following methods: - # View.new[lookup_context, assigns, controller] - # Create a new ActionView instance for a controller and we can also pass the arguments. - # View#render(option) - # Returns String with the rendered template + # + # * <tt>View.new(lookup_context, assigns, controller)</tt> — Create a new + # ActionView instance for a controller and we can also pass the arguments. + # + # * <tt>View#render(option)</tt> — Returns String with the rendered template. # # Override this method in a module to change the default behavior. def view_context diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index 08dd6fb510..12ae82f8c5 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -378,7 +378,7 @@ module ActionView # This regex match does double duty of finding only files which match # details (instead of just matching the prefix) and also filtering for # case-insensitive file systems. - !filename.match(regex) || + !regex.match?(filename) || File.directory?(filename) end.sort_by do |filename| # Because we scanned the directory, instead of checking for files diff --git a/actionview/test/ujs/public/test/data-disable-with.js b/actionview/test/ujs/public/test/data-disable-with.js index a9a98fb420..10b8870171 100644 --- a/actionview/test/ujs/public/test/data-disable-with.js +++ b/actionview/test/ujs/public/test/data-disable-with.js @@ -330,7 +330,7 @@ asyncTest('form[data-remote] input|button|textarea[data-disable-with] does not d start() }) -asyncTest('ctrl-clicking on a link does not disables the link', 6, function() { +asyncTest('ctrl-clicking on a link does not disable the link', 6, function() { var link = $('a[data-disable-with]') App.checkEnabledState(link, 'Click me') @@ -343,7 +343,7 @@ asyncTest('ctrl-clicking on a link does not disables the link', 6, function() { start() }) -asyncTest('right/mouse-wheel-clicking on a link does not disables the link', 10, function() { +asyncTest('right/mouse-wheel-clicking on a link does not disable the link', 10, function() { var link = $('a[data-disable-with]') App.checkEnabledState(link, 'Click me') diff --git a/activejob/Rakefile b/activejob/Rakefile index 0f88b22e8d..2b9f89853f 100644 --- a/activejob/Rakefile +++ b/activejob/Rakefile @@ -28,6 +28,7 @@ namespace :test do task "env:integration" do ENV["AJ_INTEGRATION_TESTS"] = "1" + ENV["SKIP_REQUIRE_WEBPACKER"] = "true" end ACTIVEJOB_ADAPTERS.each do |adapter| diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 0bc8728e36..38c4d0c3c7 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,3 +1,8 @@ +* Fix numericality equality validation of `BigDecimal` and `Float` + by casting to `BigDecimal` on both ends of the validation. + + *Gannon McGibbon* + * Add `#slice!` method to `ActiveModel::Errors`. *Daniel López Prat* diff --git a/activemodel/lib/active_model/validations/acceptance.rb b/activemodel/lib/active_model/validations/acceptance.rb index ea3a6b52ab..9ff342d72b 100644 --- a/activemodel/lib/active_model/validations/acceptance.rb +++ b/activemodel/lib/active_model/validations/acceptance.rb @@ -54,6 +54,7 @@ module ActiveModel def define_on(klass) attr_readers = attributes.reject { |name| klass.attribute_method?(name) } attr_writers = attributes.reject { |name| klass.attribute_method?("#{name}=") } + klass.define_attribute_methods klass.send(:attr_reader, *attr_readers) klass.send(:attr_writer, *attr_writers) end diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index b9ae42fd39..c5997283ea 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -9,6 +9,9 @@ module ActiveModel RESERVED_OPTIONS = CHECKS.keys + [:only_integer] + INTEGER_REGEX = /\A[+-]?\d+\z/ + DECIMAL_REGEX = /\A[+-]?\d+\.?\d*(e|e[+-])?\d+\z/ + def check_validity! keys = CHECKS.keys - [:odd, :even] options.slice(*keys).each do |option, value| @@ -49,11 +52,7 @@ module ActiveModel return end - if raw_value.is_a?(Numeric) - value = raw_value - else - value = parse_raw_value_as_a_number(raw_value) - end + value = parse_as_number(raw_value) options.slice(*CHECKS.keys).each do |option, option_value| case option @@ -69,6 +68,8 @@ module ActiveModel option_value = record.send(option_value) end + option_value = parse_as_number(option_value) + unless value.send(CHECKS[option], option_value) record.errors.add(attr_name, option, filtered_options(value).merge!(count: option_value)) end @@ -79,22 +80,33 @@ module ActiveModel private def is_number?(raw_value) - !parse_raw_value_as_a_number(raw_value).nil? + !parse_as_number(raw_value).nil? rescue ArgumentError, TypeError false end - def parse_raw_value_as_a_number(raw_value) - return raw_value.to_i if is_integer?(raw_value) - Kernel.Float(raw_value) unless is_hexadecimal_literal?(raw_value) + def parse_as_number(raw_value) + if raw_value.is_a?(Float) + raw_value.to_d + elsif raw_value.is_a?(Numeric) + raw_value + elsif is_integer?(raw_value) + raw_value.to_i + elsif is_decimal?(raw_value) && !is_hexadecimal_literal?(raw_value) + BigDecimal(raw_value) + end end def is_integer?(raw_value) - /\A[+-]?\d+\z/ === raw_value.to_s + INTEGER_REGEX.match?(raw_value.to_s) + end + + def is_decimal?(raw_value) + DECIMAL_REGEX.match?(raw_value.to_s) end def is_hexadecimal_literal?(raw_value) - /\A0[xX]/ === raw_value + /\A0[xX]/.match?(raw_value) end def filtered_options(value) diff --git a/activemodel/test/cases/validations/numericality_validation_test.rb b/activemodel/test/cases/validations/numericality_validation_test.rb index ca3c3bc40d..ca22e38c2d 100644 --- a/activemodel/test/cases/validations/numericality_validation_test.rb +++ b/activemodel/test/cases/validations/numericality_validation_test.rb @@ -289,6 +289,13 @@ class NumericalityValidationTest < ActiveModel::TestCase assert_raise(ArgumentError) { Topic.validates_numericality_of :approved, equal_to: "foo" } end + def test_validates_numericality_equality_for_float_and_big_decimal + Topic.validates_numericality_of :approved, equal_to: BigDecimal("65.6") + + invalid!([Float("65.5"), BigDecimal("65.7")], "must be equal to 65.6") + valid!([Float("65.6"), BigDecimal("65.6")]) + end + private def invalid!(values, error = nil) diff --git a/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb index 43e2f628dc..6adcc14545 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb @@ -19,7 +19,7 @@ module ActiveRecord execute(sql, name).to_a end - READ_QUERY = ActiveRecord::ConnectionAdapters::AbstractAdapter.build_read_query_regexp(:begin, :select, :set, :show, :release, :savepoint) # :nodoc: + READ_QUERY = ActiveRecord::ConnectionAdapters::AbstractAdapter.build_read_query_regexp(:begin, :commit, :explain, :select, :set, :show, :release, :savepoint, :rollback) # :nodoc: private_constant :READ_QUERY def write_query?(sql) # :nodoc: @@ -29,7 +29,7 @@ module ActiveRecord # Executes the SQL statement in the context of this connection. def execute(sql, name = nil) if preventing_writes? && write_query?(sql) - raise ActiveRecord::StatementInvalid, "Write query attempted while in readonly mode: #{sql}" + raise ActiveRecord::ReadOnlyError, "Write query attempted while in readonly mode: #{sql}" end # make sure we carry over any changes to ActiveRecord::Base.default_timezone that have been @@ -40,8 +40,6 @@ module ActiveRecord end def exec_query(sql, name = "SQL", binds = [], prepare: false) - materialize_transactions - if without_prepared_statement?(binds) execute_and_free(sql, name) do |result| if result @@ -62,8 +60,6 @@ module ActiveRecord end def exec_delete(sql, name = nil, binds = []) - materialize_transactions - if without_prepared_statement?(binds) execute_and_free(sql, name) { @connection.affected_rows } else @@ -122,6 +118,12 @@ module ActiveRecord end def exec_stmt_and_free(sql, name, binds, cache_stmt: false) + if preventing_writes? && write_query?(sql) + raise ActiveRecord::ReadOnlyError, "Write query attempted while in readonly mode: #{sql}" + end + + materialize_transactions + # make sure we carry over any changes to ActiveRecord::Base.default_timezone that have been # made since we established the connection @connection.query_options[:database_timezone] = ActiveRecord::Base.default_timezone diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb index 2d5b592639..c70a4fa875 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb @@ -67,7 +67,7 @@ module ActiveRecord end end - READ_QUERY = ActiveRecord::ConnectionAdapters::AbstractAdapter.build_read_query_regexp(:select, :show, :set) # :nodoc: + READ_QUERY = ActiveRecord::ConnectionAdapters::AbstractAdapter.build_read_query_regexp(:begin, :commit, :explain, :select, :set, :show, :release, :savepoint, :rollback) # :nodoc: private_constant :READ_QUERY def write_query?(sql) # :nodoc: @@ -80,7 +80,7 @@ module ActiveRecord # need it specifically, you may want consider the <tt>exec_query</tt> wrapper. def execute(sql, name = nil) if preventing_writes? && write_query?(sql) - raise ActiveRecord::StatementInvalid, "Write query attempted while in readonly mode: #{sql}" + raise ActiveRecord::ReadOnlyError, "Write query attempted while in readonly mode: #{sql}" end materialize_transactions diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 1c4e625ead..381d5ab29b 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -609,6 +609,10 @@ module ActiveRecord FEATURE_NOT_SUPPORTED = "0A000" #:nodoc: def execute_and_clear(sql, name, binds, prepare: false) + if preventing_writes? && write_query?(sql) + raise ActiveRecord::ReadOnlyError, "Write query attempted while in readonly mode: #{sql}" + end + if without_prepared_statement?(binds) result = exec_no_cache(sql, name, []) elsif !prepare diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 615aa0d83e..44c6e99112 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -209,7 +209,7 @@ module ActiveRecord # DATABASE STATEMENTS ====================================== #++ - READ_QUERY = ActiveRecord::ConnectionAdapters::AbstractAdapter.build_read_query_regexp(:select) # :nodoc: + READ_QUERY = ActiveRecord::ConnectionAdapters::AbstractAdapter.build_read_query_regexp(:begin, :commit, :explain, :select, :pragma, :release, :savepoint, :rollback) # :nodoc: private_constant :READ_QUERY def write_query?(sql) # :nodoc: @@ -222,6 +222,10 @@ module ActiveRecord end def exec_query(sql, name = nil, binds = [], prepare: false) + if preventing_writes? && write_query?(sql) + raise ActiveRecord::ReadOnlyError, "Write query attempted while in readonly mode: #{sql}" + end + materialize_transactions type_casted_binds = type_casted_binds(binds) @@ -265,7 +269,7 @@ module ActiveRecord def execute(sql, name = nil) #:nodoc: if preventing_writes? && write_query?(sql) - raise ActiveRecord::StatementInvalid, "Write query attempted while in readonly mode: #{sql}" + raise ActiveRecord::ReadOnlyError, "Write query attempted while in readonly mode: #{sql}" end materialize_transactions diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb index 3ce9aad5fc..558cdeccf2 100644 --- a/activerecord/lib/active_record/connection_handling.rb +++ b/activerecord/lib/active_record/connection_handling.rb @@ -130,6 +130,29 @@ module ActiveRecord end end + # Returns true if role is the current connected role. + # + # ActiveRecord::Base.connected_to(role: :writing) do + # ActiveRecord::Base.connected_to?(role: :writing) #=> true + # ActiveRecord::Base.connected_to?(role: :reading) #=> false + # end + def connected_to?(role:) + current_role == role.to_sym + end + + # Returns the symbol representing the current connected role. + # + # ActiveRecord::Base.connected_to(role: :writing) do + # ActiveRecord::Base.current_role #=> :writing + # end + # + # ActiveRecord::Base.connected_to(role: :reading) do + # ActiveRecord::Base.current_role #=> :reading + # end + def current_role + connection_handlers.key(connection_handler) + end + def lookup_connection_handler(handler_key) # :nodoc: connection_handlers[handler_key] ||= ActiveRecord::ConnectionAdapters::ConnectionHandler.new end diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index 7e6d4c1d46..0858af3874 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -49,6 +49,10 @@ module ActiveRecord class ConnectionNotEstablished < ActiveRecordError end + # Raised when a write to the database is attempted on a read only connection. + class ReadOnlyError < ActiveRecordError + end + # Raised when Active Record cannot find a record by given id or set of ids. class RecordNotFound < ActiveRecordError attr_reader :model, :primary_key, :id diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 1248ed00c5..350b044a3e 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -573,49 +573,49 @@ module ActiveRecord private - def read_and_insert(fixtures_directory, fixture_files, class_names, connection) # :nodoc: - fixtures_map = {} - fixture_sets = fixture_files.map do |fixture_set_name| - klass = class_names[fixture_set_name] - fixtures_map[fixture_set_name] = new( # ActiveRecord::FixtureSet.new - nil, - fixture_set_name, - klass, - ::File.join(fixtures_directory, fixture_set_name) - ) - end - update_all_loaded_fixtures(fixtures_map) - - insert(fixture_sets, connection) + def read_and_insert(fixtures_directory, fixture_files, class_names, connection) # :nodoc: + fixtures_map = {} + fixture_sets = fixture_files.map do |fixture_set_name| + klass = class_names[fixture_set_name] + fixtures_map[fixture_set_name] = new( # ActiveRecord::FixtureSet.new + nil, + fixture_set_name, + klass, + ::File.join(fixtures_directory, fixture_set_name) + ) + end + update_all_loaded_fixtures(fixtures_map) - fixtures_map - end + insert(fixture_sets, connection) - def insert(fixture_sets, connection) # :nodoc: - fixture_sets_by_connection = fixture_sets.group_by do |fixture_set| - fixture_set.model_class&.connection || connection + fixtures_map end - fixture_sets_by_connection.each do |conn, set| - table_rows_for_connection = Hash.new { |h, k| h[k] = [] } + def insert(fixture_sets, connection) # :nodoc: + fixture_sets_by_connection = fixture_sets.group_by do |fixture_set| + fixture_set.model_class&.connection || connection + end + + fixture_sets_by_connection.each do |conn, set| + table_rows_for_connection = Hash.new { |h, k| h[k] = [] } - set.each do |fixture_set| - fixture_set.table_rows.each do |table, rows| - table_rows_for_connection[table].unshift(*rows) + set.each do |fixture_set| + fixture_set.table_rows.each do |table, rows| + table_rows_for_connection[table].unshift(*rows) + end end - end - conn.insert_fixtures_set(table_rows_for_connection, table_rows_for_connection.keys) + conn.insert_fixtures_set(table_rows_for_connection, table_rows_for_connection.keys) - # Cap primary key sequences to max(pk). - if conn.respond_to?(:reset_pk_sequence!) - set.each { |fs| conn.reset_pk_sequence!(fs.table_name) } + # Cap primary key sequences to max(pk). + if conn.respond_to?(:reset_pk_sequence!) + set.each { |fs| conn.reset_pk_sequence!(fs.table_name) } + end end end - end - def update_all_loaded_fixtures(fixtures_map) # :nodoc: - all_loaded_fixtures.update(fixtures_map) - end + def update_all_loaded_fixtures(fixtures_map) # :nodoc: + all_loaded_fixtures.update(fixtures_map) + end end attr_reader :table_name, :name, :fixtures, :model_class, :config diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index d5b6082d13..ba221a333b 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -163,7 +163,7 @@ module ActiveRecord # Attempts to create a record with the given attributes in a table that has a unique constraint # on one or several of its columns. If a row already exists with one or several of these # unique constraints, the exception such an insertion would normally raise is caught, - # and the existing record with those attributes is found using #find_by. + # and the existing record with those attributes is found using #find_by!. # # This is similar to #find_or_create_by, but avoids the problem of stale reads between the SELECT # and the INSERT, as that method needs to first query the table, then attempt to insert a row @@ -173,7 +173,7 @@ module ActiveRecord # # * The underlying table must have the relevant columns defined with unique constraints. # * A unique constraint violation may be triggered by only one, or at least less than all, - # of the given attributes. This means that the subsequent #find_by may fail to find a + # of the given attributes. This means that the subsequent #find_by! may fail to find a # matching record, which will then raise an <tt>ActiveRecord::RecordNotFound</tt> exception, # rather than a record with the given attributes. # * While we avoid the race condition between SELECT -> INSERT from #find_or_create_by, diff --git a/activerecord/lib/active_record/type.rb b/activerecord/lib/active_record/type.rb index 405936aca6..03d00006b7 100644 --- a/activerecord/lib/active_record/type.rb +++ b/activerecord/lib/active_record/type.rb @@ -48,9 +48,9 @@ module ActiveRecord private - def current_adapter_name - ActiveRecord::Base.connection.adapter_name.downcase.to_sym - end + def current_adapter_name + ActiveRecord::Base.connection.adapter_name.downcase.to_sym + end end BigInteger = ActiveModel::Type::BigInteger diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index e40ae3090c..3d31533ec8 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -165,6 +165,55 @@ module ActiveRecord end end + def test_errors_when_an_insert_query_is_called_while_preventing_writes + assert_no_queries do + assert_raises(ActiveRecord::ReadOnlyError) do + @connection.while_preventing_writes do + @connection.transaction do + @connection.insert("INSERT INTO subscribers(nick) VALUES ('138853948594')", nil, false) + end + end + end + end + end + + def test_errors_when_an_update_query_is_called_while_preventing_writes + @connection.insert("INSERT INTO subscribers(nick) VALUES ('138853948594')") + + assert_no_queries do + assert_raises(ActiveRecord::ReadOnlyError) do + @connection.while_preventing_writes do + @connection.transaction do + @connection.update("UPDATE subscribers SET nick = '9989' WHERE nick = '138853948594'") + end + end + end + end + end + + def test_errors_when_a_delete_query_is_called_while_preventing_writes + @connection.insert("INSERT INTO subscribers(nick) VALUES ('138853948594')") + + assert_no_queries do + assert_raises(ActiveRecord::ReadOnlyError) do + @connection.while_preventing_writes do + @connection.transaction do + @connection.delete("DELETE FROM subscribers WHERE nick = '138853948594'") + end + end + end + end + end + + def test_doesnt_error_when_a_select_query_is_called_while_preventing_writes + @connection.insert("INSERT INTO subscribers(nick) VALUES ('138853948594')") + + @connection.while_preventing_writes do + result = @connection.select_all("SELECT subscribers.* FROM subscribers WHERE nick = '138853948594'") + assert_equal 1, result.length + end + end + def test_uniqueness_violations_are_translated_to_specific_exception @connection.execute "INSERT INTO subscribers(nick) VALUES('me')" error = assert_raises(ActiveRecord::RecordNotUnique) do diff --git a/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb b/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb index bddcd309cd..7bc86476b6 100644 --- a/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb +++ b/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb @@ -70,7 +70,7 @@ class Mysql2AdapterTest < ActiveRecord::Mysql2TestCase end def test_errors_when_an_insert_query_is_called_while_preventing_writes - assert_raises(ActiveRecord::StatementInvalid) do + assert_raises(ActiveRecord::ReadOnlyError) do @conn.while_preventing_writes do @conn.insert("INSERT INTO `engines` (`car_id`) VALUES ('138853948594')") end @@ -80,7 +80,7 @@ class Mysql2AdapterTest < ActiveRecord::Mysql2TestCase def test_errors_when_an_update_query_is_called_while_preventing_writes @conn.insert("INSERT INTO `engines` (`car_id`) VALUES ('138853948594')") - assert_raises(ActiveRecord::StatementInvalid) do + assert_raises(ActiveRecord::ReadOnlyError) do @conn.while_preventing_writes do @conn.update("UPDATE `engines` SET `engines`.`car_id` = '9989' WHERE `engines`.`car_id` = '138853948594'") end @@ -90,7 +90,7 @@ class Mysql2AdapterTest < ActiveRecord::Mysql2TestCase def test_errors_when_a_delete_query_is_called_while_preventing_writes @conn.execute("INSERT INTO `engines` (`car_id`) VALUES ('138853948594')") - assert_raises(ActiveRecord::StatementInvalid) do + assert_raises(ActiveRecord::ReadOnlyError) do @conn.while_preventing_writes do @conn.execute("DELETE FROM `engines` where `engines`.`car_id` = '138853948594'") end @@ -100,7 +100,7 @@ class Mysql2AdapterTest < ActiveRecord::Mysql2TestCase def test_errors_when_a_replace_query_is_called_while_preventing_writes @conn.execute("INSERT INTO `engines` (`car_id`) VALUES ('138853948594')") - assert_raises(ActiveRecord::StatementInvalid) do + assert_raises(ActiveRecord::ReadOnlyError) do @conn.while_preventing_writes do @conn.execute("REPLACE INTO `engines` SET `engines`.`car_id` = '249823948'") end diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index 59ddb8457e..34b4fc344b 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -378,7 +378,7 @@ module ActiveRecord def test_errors_when_an_insert_query_is_called_while_preventing_writes with_example_table do - assert_raises(ActiveRecord::StatementInvalid) do + assert_raises(ActiveRecord::ReadOnlyError) do @connection.while_preventing_writes do @connection.execute("INSERT INTO ex (data) VALUES ('138853948594')") end @@ -390,7 +390,7 @@ module ActiveRecord with_example_table do @connection.execute("INSERT INTO ex (data) VALUES ('138853948594')") - assert_raises(ActiveRecord::StatementInvalid) do + assert_raises(ActiveRecord::ReadOnlyError) do @connection.while_preventing_writes do @connection.execute("UPDATE ex SET data = '9989' WHERE data = '138853948594'") end @@ -402,7 +402,7 @@ module ActiveRecord with_example_table do @connection.execute("INSERT INTO ex (data) VALUES ('138853948594')") - assert_raises(ActiveRecord::StatementInvalid) do + assert_raises(ActiveRecord::ReadOnlyError) do @connection.while_preventing_writes do @connection.execute("DELETE FROM ex where data = '138853948594'") end diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index 3b7a28b757..56ceb45040 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -575,7 +575,7 @@ module ActiveRecord def test_errors_when_an_insert_query_is_called_while_preventing_writes with_example_table "id int, data string" do - assert_raises(ActiveRecord::StatementInvalid) do + assert_raises(ActiveRecord::ReadOnlyError) do @conn.while_preventing_writes do @conn.execute("INSERT INTO ex (data) VALUES ('138853948594')") end @@ -587,7 +587,7 @@ module ActiveRecord with_example_table "id int, data string" do @conn.execute("INSERT INTO ex (data) VALUES ('138853948594')") - assert_raises(ActiveRecord::StatementInvalid) do + assert_raises(ActiveRecord::ReadOnlyError) do @conn.while_preventing_writes do @conn.execute("UPDATE ex SET data = '9989' WHERE data = '138853948594'") end @@ -599,7 +599,7 @@ module ActiveRecord with_example_table "id int, data string" do @conn.execute("INSERT INTO ex (data) VALUES ('138853948594')") - assert_raises(ActiveRecord::StatementInvalid) do + assert_raises(ActiveRecord::ReadOnlyError) do @conn.while_preventing_writes do @conn.execute("DELETE FROM ex where data = '138853948594'") end @@ -611,7 +611,7 @@ module ActiveRecord with_example_table "id int, data string" do @conn.execute("INSERT INTO ex (data) VALUES ('138853948594')") - assert_raises(ActiveRecord::StatementInvalid) do + assert_raises(ActiveRecord::ReadOnlyError) do @conn.while_preventing_writes do @conn.execute("REPLACE INTO ex (data) VALUES ('249823948')") end diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 77c3ed954a..bd535357ee 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -374,7 +374,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_delete_association - assert_queries(2) { posts(:welcome);people(:michael); } + assert_queries(2) { posts(:welcome); people(:michael); } assert_queries(1) do posts(:welcome).people.delete(people(:michael)) @@ -601,7 +601,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_replace_association - assert_queries(4) { posts(:welcome);people(:david);people(:michael); posts(:welcome).people.reload } + assert_queries(4) { posts(:welcome); people(:david); people(:michael); posts(:welcome).people.reload } # 1 query to delete the existing reader (michael) # 1 query to associate the new reader (david) @@ -740,7 +740,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_clear_associations - assert_queries(2) { posts(:welcome);posts(:welcome).people.reload } + assert_queries(2) { posts(:welcome); posts(:welcome).people.reload } assert_queries(1) do posts(:welcome).people.clear diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 171983a9f5..4938b6865f 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1490,31 +1490,37 @@ class BasicsTest < ActiveRecord::TestCase end test "creating a record raises if preventing writes" do - assert_raises ActiveRecord::StatementInvalid do + error = assert_raises ActiveRecord::ReadOnlyError do ActiveRecord::Base.connection.while_preventing_writes do Bird.create! name: "Bluejay" end end + + assert_match %r/\AWrite query attempted while in readonly mode: INSERT /, error.message end test "updating a record raises if preventing writes" do bird = Bird.create! name: "Bluejay" - assert_raises ActiveRecord::StatementInvalid do + error = assert_raises ActiveRecord::ReadOnlyError do ActiveRecord::Base.connection.while_preventing_writes do bird.update! name: "Robin" end end + + assert_match %r/\AWrite query attempted while in readonly mode: UPDATE /, error.message end test "deleting a record raises if preventing writes" do bird = Bird.create! name: "Bluejay" - assert_raises ActiveRecord::StatementInvalid do + error = assert_raises ActiveRecord::ReadOnlyError do ActiveRecord::Base.connection.while_preventing_writes do bird.destroy! end end + + assert_match %r/\AWrite query attempted while in readonly mode: DELETE /, error.message end test "selecting a record does not raise if preventing writes" do @@ -1524,4 +1530,22 @@ class BasicsTest < ActiveRecord::TestCase assert_equal bird, Bird.where(name: "Bluejay").first end end + + test "an explain query does not raise if preventing writes" do + Bird.create!(name: "Bluejay") + + ActiveRecord::Base.connection.while_preventing_writes do + assert_queries(2) { Bird.where(name: "Bluejay").explain } + end + end + + test "an empty transaction does not raise if preventing writes" do + ActiveRecord::Base.connection.while_preventing_writes do + assert_queries(2, ignore_none: true) do + Bird.transaction do + ActiveRecord::Base.connection.materialize_transactions + end + end + end + end end diff --git a/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb b/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb index 393b577c1d..79c2c4d827 100644 --- a/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb @@ -108,11 +108,15 @@ module ActiveRecord ActiveRecord::Base.connected_to(role: :reading) do @ro_handler = ActiveRecord::Base.connection_handler assert_equal ActiveRecord::Base.connection_handler, ActiveRecord::Base.connection_handlers[:reading] + assert ActiveRecord::Base.connected_to?(role: :reading) + assert_not ActiveRecord::Base.connected_to?(role: :writing) end ActiveRecord::Base.connected_to(role: :writing) do assert_equal ActiveRecord::Base.connection_handler, ActiveRecord::Base.connection_handlers[:writing] assert_not_equal @ro_handler, ActiveRecord::Base.connection_handler + assert ActiveRecord::Base.connected_to?(role: :writing) + assert_not ActiveRecord::Base.connected_to?(role: :reading) end ensure ActiveRecord::Base.configurations = @prev_configs @@ -125,6 +129,8 @@ module ActiveRecord previous_url, ENV["DATABASE_URL"] = ENV["DATABASE_URL"], "postgres://localhost/foo" ActiveRecord::Base.connected_to(database: { writing: "postgres://localhost/bar" }) do + assert ActiveRecord::Base.connected_to?(role: :writing) + handler = ActiveRecord::Base.connection_handler assert_equal handler, ActiveRecord::Base.connection_handlers[:writing] @@ -142,6 +148,8 @@ module ActiveRecord config = { adapter: "sqlite3", database: "db/readonly.sqlite3" } ActiveRecord::Base.connected_to(database: { writing: config }) do + assert ActiveRecord::Base.connected_to?(role: :writing) + handler = ActiveRecord::Base.connection_handler assert_equal handler, ActiveRecord::Base.connection_handlers[:writing] @@ -179,6 +187,8 @@ module ActiveRecord @prev_configs, ActiveRecord::Base.configurations = ActiveRecord::Base.configurations, config ActiveRecord::Base.connected_to(database: :readonly) do + assert ActiveRecord::Base.connected_to?(role: :readonly) + handler = ActiveRecord::Base.connection_handler assert_equal handler, ActiveRecord::Base.connection_handlers[:readonly] @@ -201,6 +211,7 @@ module ActiveRecord assert_equal 1, ActiveRecord::Base.connection_handlers.size assert_equal ActiveRecord::Base.connection_handler, ActiveRecord::Base.connection_handlers[:writing] + assert ActiveRecord::Base.connected_to?(role: :writing) ensure ActiveRecord::Base.configurations = @prev_configs ActiveRecord::Base.establish_connection(:arunit) diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index e471ee8039..756eeca35f 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1315,6 +1315,13 @@ class RelationTest < ActiveRecord::TestCase assert_not_equal subscriber, Subscriber.create_or_find_by(nick: "cat") end + def test_create_or_find_by_should_not_raise_due_to_validation_errors + assert_nothing_raised do + bird = Bird.create_or_find_by(color: "green") + assert_predicate bird, :invalid? + end + end + def test_create_or_find_by_with_non_unique_attributes Subscriber.create!(nick: "bob", name: "the builder") @@ -1334,6 +1341,38 @@ class RelationTest < ActiveRecord::TestCase end end + def test_create_or_find_by_with_bang + assert_nil Subscriber.find_by(nick: "bob") + + subscriber = Subscriber.create!(nick: "bob") + + assert_equal subscriber, Subscriber.create_or_find_by!(nick: "bob") + assert_not_equal subscriber, Subscriber.create_or_find_by!(nick: "cat") + end + + def test_create_or_find_by_with_bang_should_raise_due_to_validation_errors + assert_raises(ActiveRecord::RecordInvalid) { Bird.create_or_find_by!(color: "green") } + end + + def test_create_or_find_by_with_bang_with_non_unique_attributes + Subscriber.create!(nick: "bob", name: "the builder") + + assert_raises(ActiveRecord::RecordNotFound) do + Subscriber.create_or_find_by!(nick: "bob", name: "the cat") + end + end + + def test_create_or_find_by_with_bang_within_transaction + assert_nil Subscriber.find_by(nick: "bob") + + subscriber = Subscriber.create!(nick: "bob") + + Subscriber.transaction do + assert_equal subscriber, Subscriber.create_or_find_by!(nick: "bob") + assert_not_equal subscriber, Subscriber.create_or_find_by!(nick: "cat") + end + end + def test_find_or_initialize_by assert_nil Bird.find_by(name: "bob") diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index 66763c727f..9a70934b7e 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -144,6 +144,13 @@ class ValidationsTest < ActiveRecord::TestCase assert_equal "100,000", d.salary_before_type_cast end + def test_validates_acceptance_of_with_undefined_attribute_methods + Topic.validates_acceptance_of(:approved) + topic = Topic.new(approved: true) + Topic.undefine_attribute_methods + assert topic.approved + end + def test_validates_acceptance_of_as_database_column Topic.validates_acceptance_of(:approved) topic = Topic.create("approved" => true) diff --git a/activerecord/test/models/bird.rb b/activerecord/test/models/bird.rb index be08636ac6..cfefa555b3 100644 --- a/activerecord/test/models/bird.rb +++ b/activerecord/test/models/bird.rb @@ -6,6 +6,11 @@ class Bird < ActiveRecord::Base accepts_nested_attributes_for :pirate + before_save do + # force materialize_transactions + self.class.connection.materialize_transactions + end + attr_accessor :cancel_save_from_callback before_save :cancel_save_callback_method, if: :cancel_save_from_callback def cancel_save_callback_method diff --git a/activerecord/test/support/config.rb b/activerecord/test/support/config.rb index bd6d5c339b..de0d90a18f 100644 --- a/activerecord/test/support/config.rb +++ b/activerecord/test/support/config.rb @@ -13,34 +13,34 @@ module ARTest private - def config_file - Pathname.new(ENV["ARCONFIG"] || TEST_ROOT + "/config.yml") - end - - def read_config - unless config_file.exist? - FileUtils.cp TEST_ROOT + "/config.example.yml", config_file + def config_file + Pathname.new(ENV["ARCONFIG"] || TEST_ROOT + "/config.yml") end - erb = ERB.new(config_file.read) - expand_config(YAML.parse(erb.result(binding)).transform) - end + def read_config + unless config_file.exist? + FileUtils.cp TEST_ROOT + "/config.example.yml", config_file + end - def expand_config(config) - config["connections"].each do |adapter, connection| - dbs = [["arunit", "activerecord_unittest"], ["arunit2", "activerecord_unittest2"], - ["arunit_without_prepared_statements", "activerecord_unittest"]] - dbs.each do |name, dbname| - unless connection[name].is_a?(Hash) - connection[name] = { "database" => connection[name] } - end + erb = ERB.new(config_file.read) + expand_config(YAML.parse(erb.result(binding)).transform) + end - connection[name]["database"] ||= dbname - connection[name]["adapter"] ||= adapter + def expand_config(config) + config["connections"].each do |adapter, connection| + dbs = [["arunit", "activerecord_unittest"], ["arunit2", "activerecord_unittest2"], + ["arunit_without_prepared_statements", "activerecord_unittest"]] + dbs.each do |name, dbname| + unless connection[name].is_a?(Hash) + connection[name] = { "database" => connection[name] } + end + + connection[name]["database"] ||= dbname + connection[name]["adapter"] ||= adapter + end end - end - config - end + config + end end end diff --git a/activesupport/lib/active_support/deprecation/method_wrappers.rb b/activesupport/lib/active_support/deprecation/method_wrappers.rb index 81482092fe..f0c3e37e65 100644 --- a/activesupport/lib/active_support/deprecation/method_wrappers.rb +++ b/activesupport/lib/active_support/deprecation/method_wrappers.rb @@ -52,27 +52,37 @@ module ActiveSupport options = method_names.extract_options! deprecator = options.delete(:deprecator) || self method_names += options.keys + mod = Module.new method_names.each do |method_name| - aliased_method, punctuation = method_name.to_s.sub(/([?!=])$/, ""), $1 - with_method = "#{aliased_method}_with_deprecation#{punctuation}" - without_method = "#{aliased_method}_without_deprecation#{punctuation}" + if target_module.method_defined?(method_name) || target_module.private_method_defined?(method_name) + aliased_method, punctuation = method_name.to_s.sub(/([?!=])$/, ""), $1 + with_method = "#{aliased_method}_with_deprecation#{punctuation}" + without_method = "#{aliased_method}_without_deprecation#{punctuation}" - target_module.send(:define_method, with_method) do |*args, &block| - deprecator.deprecation_warning(method_name, options[method_name]) - send(without_method, *args, &block) - end + target_module.send(:define_method, with_method) do |*args, &block| + deprecator.deprecation_warning(method_name, options[method_name]) + send(without_method, *args, &block) + end - target_module.send(:alias_method, without_method, method_name) - target_module.send(:alias_method, method_name, with_method) + target_module.send(:alias_method, without_method, method_name) + target_module.send(:alias_method, method_name, with_method) - case - when target_module.protected_method_defined?(without_method) - target_module.send(:protected, method_name) - when target_module.private_method_defined?(without_method) - target_module.send(:private, method_name) + case + when target_module.protected_method_defined?(without_method) + target_module.send(:protected, method_name) + when target_module.private_method_defined?(without_method) + target_module.send(:private, method_name) + end + else + mod.send(:define_method, method_name) do |*args, &block| + deprecator.deprecation_warning(method_name, options[method_name]) + super(*args, &block) + end end end + + target_module.prepend(mod) unless mod.instance_methods(false).empty? end end end diff --git a/activesupport/lib/active_support/descendants_tracker.rb b/activesupport/lib/active_support/descendants_tracker.rb index a4cee788b6..05236d3162 100644 --- a/activesupport/lib/active_support/descendants_tracker.rb +++ b/activesupport/lib/active_support/descendants_tracker.rb @@ -38,12 +38,13 @@ module ActiveSupport end private - def accumulate_descendants(klass, acc) - if direct_descendants = @@direct_descendants[klass] - acc.concat(direct_descendants) - direct_descendants.each { |direct_descendant| accumulate_descendants(direct_descendant, acc) } + + def accumulate_descendants(klass, acc) + if direct_descendants = @@direct_descendants[klass] + acc.concat(direct_descendants) + direct_descendants.each { |direct_descendant| accumulate_descendants(direct_descendant, acc) } + end end - end end def inherited(base) diff --git a/activesupport/lib/active_support/json/decoding.rb b/activesupport/lib/active_support/json/decoding.rb index 8c0e016dc5..402a3fbe60 100644 --- a/activesupport/lib/active_support/json/decoding.rb +++ b/activesupport/lib/active_support/json/decoding.rb @@ -45,32 +45,32 @@ module ActiveSupport private - def convert_dates_from(data) - case data - when nil - nil - when DATE_REGEX - begin - Date.parse(data) - rescue ArgumentError + def convert_dates_from(data) + case data + when nil + nil + when DATE_REGEX + begin + Date.parse(data) + rescue ArgumentError + data + end + when DATETIME_REGEX + begin + Time.zone.parse(data) + rescue ArgumentError + data + end + when Array + data.map! { |d| convert_dates_from(d) } + when Hash + data.each do |key, value| + data[key] = convert_dates_from(value) + end + else data end - when DATETIME_REGEX - begin - Time.zone.parse(data) - rescue ArgumentError - data - end - when Array - data.map! { |d| convert_dates_from(d) } - when Hash - data.each do |key, value| - data[key] = convert_dates_from(value) - end - else - data end - end end end end diff --git a/activesupport/lib/active_support/notifications.rb b/activesupport/lib/active_support/notifications.rb index 01cc363e2b..0ff32bd810 100644 --- a/activesupport/lib/active_support/notifications.rb +++ b/activesupport/lib/active_support/notifications.rb @@ -177,7 +177,7 @@ module ActiveSupport # names, or by passing a Regexp to match all events that match a pattern. # # ActiveSupport::Notifications.subscribe(/render/) do |*args| - # ... + # @event = ActiveSupport::Notifications::Event.new(*args) # end # # The +block+ will receive five parameters with information about the event: @@ -189,6 +189,13 @@ module ActiveSupport # id # => String, unique ID for the instrumenter that fired the event # payload # => Hash, the payload # end + # + # If the block passed to the method only takes one parameter, + # it will yield an event object to the block: + # + # ActiveSupport::Notifications.subscribe(/render/) do |event| + # @event = event + # end def subscribe(*args, &block) notifier.subscribe(*args, &block) end diff --git a/activesupport/test/deprecation/method_wrappers_test.rb b/activesupport/test/deprecation/method_wrappers_test.rb index b04bce7a11..18729941bc 100644 --- a/activesupport/test/deprecation/method_wrappers_test.rb +++ b/activesupport/test/deprecation/method_wrappers_test.rb @@ -21,6 +21,13 @@ class MethodWrappersTest < ActiveSupport::TestCase end end + def test_deprecate_methods_without_alternate_method + warning = /old_method is deprecated and will be removed from Rails \d.\d./ + ActiveSupport::Deprecation.deprecate_methods(@klass, :old_method) + + assert_deprecated(warning) { assert_equal "abc", @klass.new.old_method } + end + def test_deprecate_methods_warning_default warning = /old_method is deprecated and will be removed from Rails \d.\d \(use new_method instead\)/ ActiveSupport::Deprecation.deprecate_methods(@klass, old_method: :new_method) diff --git a/activesupport/test/deprecation_test.rb b/activesupport/test/deprecation_test.rb index 105153584d..f25c704586 100644 --- a/activesupport/test/deprecation_test.rb +++ b/activesupport/test/deprecation_test.rb @@ -31,6 +31,9 @@ class Deprecatee def f=(v); end deprecate :f= + deprecate :g + def g; end + module B C = 1 end @@ -425,6 +428,10 @@ class DeprecationTest < ActiveSupport::TestCase end end + def test_deprecate_work_before_define_method + assert_deprecated { @dtc.g } + end + private def deprecator_with_messages klass = Class.new(ActiveSupport::Deprecation) diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb index 54fd4345fb..4e0aef2cc7 100644 --- a/activesupport/test/notifications_test.rb +++ b/activesupport/test/notifications_test.rb @@ -26,7 +26,7 @@ module Notifications end end - class SubscribeEventObjects < TestCase + class SubscribeEventObjectsTest < TestCase def test_subscribe_events events = [] @notifier.subscribe do |event| diff --git a/guides/source/active_record_migrations.md b/guides/source/active_record_migrations.md index 4d195988f8..905c76e5c1 100644 --- a/guides/source/active_record_migrations.md +++ b/guides/source/active_record_migrations.md @@ -126,7 +126,7 @@ generator to handle making it for you: $ rails generate migration AddPartNumberToProducts ``` -This will create an empty but appropriately named migration: +This will create an appropriately named empty migration: ```ruby class AddPartNumberToProducts < ActiveRecord::Migration[5.0] @@ -135,9 +135,14 @@ class AddPartNumberToProducts < ActiveRecord::Migration[5.0] end ``` -If the migration name is of the form "AddXXXToYYY" or "RemoveXXXFromYYY" and is -followed by a list of column names and types then a migration containing the -appropriate `add_column` and `remove_column` statements will be created. +This generator can do much more than append a timestamp to the file name. +Based on naming conventions and additional (optional) arguments it can +also start fleshing out the migration. + +If the migration name is of the form "AddColumnToTable" or +"RemoveColumnFromTable" and is followed by a list of column names and +types then a migration containing the appropriate `add_column` and +`remove_column` statements will be created. ```bash $ rails generate migration AddPartNumberToProducts part_number:string diff --git a/guides/source/active_storage_overview.md b/guides/source/active_storage_overview.md index 65d4e2934f..51f50e8931 100644 --- a/guides/source/active_storage_overview.md +++ b/guides/source/active_storage_overview.md @@ -742,16 +742,22 @@ during the test are complete and you won't receive an error from Active Storage saying it can't find a file. ```ruby +module RemoveUploadedFiles + def after_teardown + super + remove_uploaded_files + end + + private + + def remove_uploaded_files + FileUtils.rm_rf(Rails.root.join('tmp', 'storage')) + end +end + module ActionDispatch class IntegrationTest - def remove_uploaded_files - FileUtils.rm_rf(Rails.root.join('tmp', 'storage')) - end - - def after_teardown - super - remove_uploaded_files - end + prepend RemoveUploadedFiles end end ``` diff --git a/guides/source/active_support_instrumentation.md b/guides/source/active_support_instrumentation.md index 64db141381..5e68b3f400 100644 --- a/guides/source/active_support_instrumentation.md +++ b/guides/source/active_support_instrumentation.md @@ -648,6 +648,18 @@ ActiveSupport::Notifications.subscribe "process_action.action_controller" do |*a end ``` +You may also pass block with only one argument, it will yield an event object to the block: + +```ruby +ActiveSupport::Notifications.subscribe "process_action.action_controller" do |event| + event.name # => "process_action.action_controller" + event.duration # => 10 (in milliseconds) + event.payload # => {:extra=>information} + + Rails.logger.info "#{event} Received!" +end +``` + Most times you only care about the data itself. Here is a shortcut to just get the data. ```ruby @@ -672,7 +684,7 @@ Creating custom events Adding your own events is easy as well. `ActiveSupport::Notifications` will take care of all the heavy lifting for you. Simply call `instrument` with a `name`, `payload` and a block. The notification will be sent after the block returns. `ActiveSupport` will generate the start and end times -and add the instrumenter's unique ID. All data passed into the `instrument` call will make +and add the instrumenter's unique ID. All data passed into the `instrument` call will make it into the payload. Here's an example: diff --git a/guides/source/rails_application_templates.md b/guides/source/rails_application_templates.md index bc68a555c5..982df26987 100644 --- a/guides/source/rails_application_templates.md +++ b/guides/source/rails_application_templates.md @@ -195,6 +195,12 @@ You can also run commands as a super-user: rails_command "log:clear", sudo: true ``` +You can also run commands that should abort application generation if they fail: + +```ruby +rails_command "db:migrate", abort_on_failure: true +``` + ### route(routing_code) Adds a routing entry to the `config/routes.rb` file. In the steps above, we generated a person scaffold and also removed `README.rdoc`. Now, to make `PeopleController#index` the default page for the application: diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index a0553c1ccc..e74985c5b0 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -407,7 +407,7 @@ want to add this feature it will need to be turned on in an initializer. Rails 5 now supports per-form CSRF tokens to mitigate against code-injection attacks with forms created by JavaScript. With this option turned on, forms in your application will each have their -own CSRF token that is specified to the action and method for that form. +own CSRF token that is specific to the action and method for that form. config.action_controller.per_form_csrf_tokens = true diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 448fd48f10..59024b13ef 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,60 @@ +* Introduce guard against DNS rebinding attacks + + The `ActionDispatch::HostAuthorization` is a new middleware that prevent + against DNS rebinding and other `Host` header attacks. It is included in + the development environment by default with the following configuration: + + Rails.application.config.hosts = [ + IPAddr.new("0.0.0.0/0"), # All IPv4 addresses. + IPAddr.new("::/0"), # All IPv6 addresses. + "localhost" # The localhost reserved domain. + ] + + In other environments `Rails.application.config.hosts` is empty and no + `Host` header checks will be done. If you want to guard against header + attacks on production, you have to manually whitelist the allowed hosts + with: + + Rails.application.config.hosts << "product.com" + + The host of a request is checked against the `hosts` entries with the case + operator (`#===`), which lets `hosts` support entries of type `RegExp`, + `Proc` and `IPAddr` to name a few. Here is an example with a regexp. + + # Allow requests from subdomains like `www.product.com` and + # `beta1.product.com`. + Rails.application.config.hosts << /.*\.product\.com/ + + A special case is supported that allows you to whitelist all sub-domains: + + # Allow requests from subdomains like `www.product.com` and + # `beta1.product.com`. + Rails.application.config.hosts << ".product.com" + + *Genadi Samokovarov* + +* Remove redundant suffixes on generated helpers. + + *Gannon McGibbon* + +* Remove redundant suffixes on generated integration tests. + + *Gannon McGibbon* + +* Fix boolean interaction in scaffold system tests. + + *Gannon McGibbon* + +* Remove redundant suffixes on generated system tests. + + *Gannon McGibbon* + +* Add an `abort_on_failure` boolean option to the generator method that shell + out (`generate`, `rake`, `rails_command`) to abort the generator if the + command fails. + + *David Rodríguez* + * Remove `app/assets` and `app/javascript` from `eager_load_paths` and `autoload_paths`. *Gannon McGibbon* diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index f30e5d2a0f..22a82c051d 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "ipaddr" require "active_support/core_ext/kernel/reporting" require "active_support/file_update_checker" require "rails/engine/configuration" @@ -11,7 +12,7 @@ module Rails attr_accessor :allow_concurrency, :asset_host, :autoflush_log, :cache_classes, :cache_store, :consider_all_requests_local, :console, :eager_load, :exceptions_app, :file_watcher, :filter_parameters, - :force_ssl, :helpers_paths, :logger, :log_formatter, :log_tags, + :force_ssl, :helpers_paths, :hosts, :logger, :log_formatter, :log_tags, :railties_order, :relative_url_root, :secret_key_base, :secret_token, :ssl_options, :public_file_server, :session_options, :time_zone, :reload_classes_only_on_change, @@ -29,6 +30,7 @@ module Rails @filter_parameters = [] @filter_redirect = [] @helpers_paths = [] + @hosts = Array(([IPAddr.new("0.0.0.0/0"), IPAddr.new("::/0"), "localhost"] if Rails.env.development?)) @public_file_server = ActiveSupport::OrderedOptions.new @public_file_server.enabled = true @public_file_server.index_name = "index" diff --git a/railties/lib/rails/application/default_middleware_stack.rb b/railties/lib/rails/application/default_middleware_stack.rb index 433a7ab41f..193cc59f3a 100644 --- a/railties/lib/rails/application/default_middleware_stack.rb +++ b/railties/lib/rails/application/default_middleware_stack.rb @@ -13,6 +13,8 @@ module Rails def build_stack ActionDispatch::MiddlewareStack.new do |middleware| + middleware.use ::ActionDispatch::HostAuthorization, config.hosts, config.action_dispatch.hosts_response_app + if config.force_ssl middleware.use ::ActionDispatch::SSL, config.ssl_options end diff --git a/railties/lib/rails/generators/actions.rb b/railties/lib/rails/generators/actions.rb index 78d2471890..4646a55316 100644 --- a/railties/lib/rails/generators/actions.rb +++ b/railties/lib/rails/generators/actions.rb @@ -221,9 +221,11 @@ module Rails # generate(:authenticated, "user session") def generate(what, *args) log :generate, what + + options = args.extract_options! argument = args.flat_map(&:to_s).join(" ") - in_root { run_ruby_script("bin/rails generate #{what} #{argument}", verbose: false) } + execute_command :rails, "generate #{what} #{argument}", options end # Runs the supplied rake task (invoked with 'rake ...') @@ -307,6 +309,7 @@ module Rails config = { verbose: false } config[:capture] = options[:capture] if options[:capture] + config[:abort_on_failure] = options[:abort_on_failure] if options[:abort_on_failure] in_root { run("#{sudo}#{extify(executor)} #{command} RAILS_ENV=#{env}", config) } end diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 4dc4d27a46..f3b99ff937 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -364,7 +364,7 @@ module Rails gems end - def bundle_command(command) + def bundle_command(command, env = {}) say_status :run, "bundle #{command}" # We are going to shell out rather than invoking Bundler::CLI.new(command) @@ -381,9 +381,9 @@ module Rails Bundler.with_clean_env do full_command = %Q["#{Gem.ruby}" "#{_bundle_command}" #{command}] if options[:quiet] - system(full_command, out: File::NULL) + system(env, full_command, out: File::NULL) else - system(full_command) + system(env, full_command) end end end @@ -417,7 +417,7 @@ module Rails end def run_bundle - bundle_command("install") if bundle_install? + bundle_command("install", "BUNDLE_IGNORE_MESSAGES" => "1") if bundle_install? end def run_webpack diff --git a/railties/lib/rails/generators/generated_attribute.rb b/railties/lib/rails/generators/generated_attribute.rb index 3f20f5a718..a8f7729fd3 100644 --- a/railties/lib/rails/generators/generated_attribute.rb +++ b/railties/lib/rails/generators/generated_attribute.rb @@ -39,23 +39,23 @@ module Rails private - # parse possible attribute options like :limit for string/text/binary/integer, :precision/:scale for decimals or :polymorphic for references/belongs_to - # when declaring options curly brackets should be used - def parse_type_and_options(type) - case type - when /(string|text|binary|integer)\{(\d+)\}/ - return $1, limit: $2.to_i - when /decimal\{(\d+)[,.-](\d+)\}/ - return :decimal, precision: $1.to_i, scale: $2.to_i - when /(references|belongs_to)\{(.+)\}/ - type = $1 - provided_options = $2.split(/[,.-]/) - options = Hash[provided_options.map { |opt| [opt.to_sym, true] }] - return type, options - else - return type, {} + # parse possible attribute options like :limit for string/text/binary/integer, :precision/:scale for decimals or :polymorphic for references/belongs_to + # when declaring options curly brackets should be used + def parse_type_and_options(type) + case type + when /(string|text|binary|integer)\{(\d+)\}/ + return $1, limit: $2.to_i + when /decimal\{(\d+)[,.-](\d+)\}/ + return :decimal, precision: $1.to_i, scale: $2.to_i + when /(references|belongs_to)\{(.+)\}/ + type = $1 + provided_options = $2.split(/[,.-]/) + options = Hash[provided_options.map { |opt| [opt.to_sym, true] }] + return type, options + else + return type, {} + end end - end end def initialize(name, type = nil, index_type = false, attr_options = {}) diff --git a/railties/lib/rails/generators/rails/helper/helper_generator.rb b/railties/lib/rails/generators/rails/helper/helper_generator.rb index 3837c10ca0..542eb4c9e8 100644 --- a/railties/lib/rails/generators/rails/helper/helper_generator.rb +++ b/railties/lib/rails/generators/rails/helper/helper_generator.rb @@ -10,6 +10,11 @@ module Rails end hook_for :test_framework + + private + def file_name + @_file_name ||= super.sub(/_helper\z/i, "") + end end end end diff --git a/railties/lib/rails/generators/test_unit/integration/integration_generator.rb b/railties/lib/rails/generators/test_unit/integration/integration_generator.rb index ae307c5cd9..ba27ed329b 100644 --- a/railties/lib/rails/generators/test_unit/integration/integration_generator.rb +++ b/railties/lib/rails/generators/test_unit/integration/integration_generator.rb @@ -10,6 +10,12 @@ module TestUnit # :nodoc: def create_test_files template "integration_test.rb", File.join("test/integration", class_path, "#{file_name}_test.rb") end + + private + + def file_name + @_file_name ||= super.sub(/_test\z/i, "") + end end end end diff --git a/railties/lib/rails/generators/test_unit/scaffold/scaffold_generator.rb b/railties/lib/rails/generators/test_unit/scaffold/scaffold_generator.rb index e2e8b18eab..6df50c3217 100644 --- a/railties/lib/rails/generators/test_unit/scaffold/scaffold_generator.rb +++ b/railties/lib/rails/generators/test_unit/scaffold/scaffold_generator.rb @@ -54,6 +54,11 @@ module TestUnit # :nodoc: end end.sort.to_h end + + def boolean?(name) + attribute = attributes.find { |attr| attr.name == name } + attribute&.type == :boolean + end end end end diff --git a/railties/lib/rails/generators/test_unit/scaffold/templates/system_test.rb.tt b/railties/lib/rails/generators/test_unit/scaffold/templates/system_test.rb.tt index 15bd7956b6..4f5bbf1108 100644 --- a/railties/lib/rails/generators/test_unit/scaffold/templates/system_test.rb.tt +++ b/railties/lib/rails/generators/test_unit/scaffold/templates/system_test.rb.tt @@ -16,8 +16,12 @@ class <%= class_name.pluralize %>Test < ApplicationSystemTestCase click_on "New <%= class_name.titleize %>" <%- attributes_hash.each do |attr, value| -%> + <%- if boolean?(attr) -%> + check "<%= attr.humanize %>" if <%= value %> + <%- else -%> fill_in "<%= attr.humanize %>", with: <%= value %> <%- end -%> + <%- end -%> click_on "Create <%= human_name %>" assert_text "<%= human_name %> was successfully created" @@ -29,8 +33,12 @@ class <%= class_name.pluralize %>Test < ApplicationSystemTestCase click_on "Edit", match: :first <%- attributes_hash.each do |attr, value| -%> + <%- if boolean?(attr) -%> + check "<%= attr.humanize %>" if <%= value %> + <%- else -%> fill_in "<%= attr.humanize %>", with: <%= value %> <%- end -%> + <%- end -%> click_on "Update <%= human_name %>" assert_text "<%= human_name %> was successfully updated" diff --git a/railties/lib/rails/generators/test_unit/system/system_generator.rb b/railties/lib/rails/generators/test_unit/system/system_generator.rb index 08504d4124..adecf74b70 100644 --- a/railties/lib/rails/generators/test_unit/system/system_generator.rb +++ b/railties/lib/rails/generators/test_unit/system/system_generator.rb @@ -14,6 +14,11 @@ module TestUnit # :nodoc: template "system_test.rb", File.join("test/system", class_path, "#{file_name.pluralize}_test.rb") end + + private + def file_name + @_file_name ||= super.sub(/_test\z/i, "") + end end end end diff --git a/railties/lib/rails/info.rb b/railties/lib/rails/info.rb index b8173c8d11..c68405619d 100644 --- a/railties/lib/rails/info.rb +++ b/railties/lib/rails/info.rb @@ -67,7 +67,7 @@ module Rails # The Ruby version and platform, e.g. "2.0.0-p247 (x86_64-darwin12.4.0)". property "Ruby version" do - "#{RUBY_VERSION}-p#{RUBY_PATCHLEVEL} (#{RUBY_PLATFORM})" + RUBY_DESCRIPTION end # The RubyGems version, if it's installed. diff --git a/railties/lib/rails/info_controller.rb b/railties/lib/rails/info_controller.rb index 50fe176946..14459623ac 100644 --- a/railties/lib/rails/info_controller.rb +++ b/railties/lib/rails/info_controller.rb @@ -4,7 +4,7 @@ require "rails/application_controller" require "action_dispatch/routing/inspector" class Rails::InfoController < Rails::ApplicationController # :nodoc: - prepend_view_path ActionDispatch::DebugExceptions::RESCUES_TEMPLATE_PATH + prepend_view_path ActionDispatch::DebugView::RESCUES_TEMPLATE_PATH layout -> { request.xhr? ? false : "application" } before_action :require_local! diff --git a/railties/lib/rails/mailers_controller.rb b/railties/lib/rails/mailers_controller.rb index e2d36d7654..95dae3ec2d 100644 --- a/railties/lib/rails/mailers_controller.rb +++ b/railties/lib/rails/mailers_controller.rb @@ -3,7 +3,7 @@ require "rails/application_controller" class Rails::MailersController < Rails::ApplicationController # :nodoc: - prepend_view_path ActionDispatch::DebugExceptions::RESCUES_TEMPLATE_PATH + prepend_view_path ActionDispatch::DebugView::RESCUES_TEMPLATE_PATH before_action :require_local!, unless: :show_previews? before_action :find_preview, :set_locale, only: :preview diff --git a/railties/lib/rails/templates/rails/welcome/index.html.erb b/railties/lib/rails/templates/rails/welcome/index.html.erb index b6823457c0..6750e01029 100644 --- a/railties/lib/rails/templates/rails/welcome/index.html.erb +++ b/railties/lib/rails/templates/rails/welcome/index.html.erb @@ -66,7 +66,7 @@ <p class="version"> <strong>Rails version:</strong> <%= Rails.version %><br /> - <strong>Ruby version:</strong> <%= RUBY_VERSION %> (<%= RUBY_PLATFORM %>) + <strong>Ruby version:</strong> <%= RUBY_DESCRIPTION %> </p> </section> </div> diff --git a/railties/railties.gemspec b/railties/railties.gemspec index 4e4a504c97..91407ee1ae 100644 --- a/railties/railties.gemspec +++ b/railties/railties.gemspec @@ -37,7 +37,7 @@ Gem::Specification.new do |s| s.add_dependency "actionpack", version s.add_dependency "rake", ">= 0.8.7" - s.add_dependency "thor", ">= 0.19.0", "< 2.0" + s.add_dependency "thor", ">= 0.20.3", "< 2.0" s.add_dependency "method_source" s.add_development_dependency "actionview", version diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index 631f5bac7f..a6396d3509 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -26,6 +26,7 @@ module ApplicationTests assert_equal [ "Webpacker::DevServerProxy", + "ActionDispatch::HostAuthorization", "Rack::Sendfile", "ActionDispatch::Static", "ActionDispatch::Executor", @@ -58,6 +59,7 @@ module ApplicationTests assert_equal [ "Webpacker::DevServerProxy", + "ActionDispatch::HostAuthorization", "Rack::Sendfile", "ActionDispatch::Static", "ActionDispatch::Executor", @@ -140,7 +142,7 @@ module ApplicationTests add_to_config "config.ssl_options = { redirect: { host: 'example.com' } }" boot! - assert_equal [{ redirect: { host: "example.com" } }], Rails.application.middleware[1].args + assert_equal [{ redirect: { host: "example.com" } }], Rails.application.middleware[2].args end test "removing Active Record omits its middleware" do @@ -224,7 +226,7 @@ module ApplicationTests test "insert middleware after" do add_to_config "config.middleware.insert_after Rack::Sendfile, Rack::Config" boot! - assert_equal "Rack::Config", middleware.third + assert_equal "Rack::Config", middleware.fourth end test "unshift middleware" do @@ -236,19 +238,19 @@ module ApplicationTests test "Rails.cache does not respond to middleware" do add_to_config "config.cache_store = :memory_store" boot! - assert_equal "Rack::Runtime", middleware.fifth + assert_equal "Rack::Runtime", middleware[5] end test "Rails.cache does respond to middleware" do boot! - assert_equal "ActiveSupport::Cache::Strategy::LocalCache", middleware.fifth - assert_equal "Rack::Runtime", middleware[5] + assert_equal "ActiveSupport::Cache::Strategy::LocalCache", middleware[5] + assert_equal "Rack::Runtime", middleware[6] end test "insert middleware before" do add_to_config "config.middleware.insert_before Rack::Sendfile, Rack::Config" boot! - assert_equal "Rack::Config", middleware.second + assert_equal "Rack::Config", middleware.third end test "can't change middleware after it's built" do diff --git a/railties/test/commands/server_test.rb b/railties/test/commands/server_test.rb index e5b1da6ea4..fbdd3f3ebb 100644 --- a/railties/test/commands/server_test.rb +++ b/railties/test/commands/server_test.rb @@ -225,10 +225,10 @@ class Rails::Command::ServerCommandTest < ActiveSupport::TestCase end def test_records_user_supplied_options - server_options = parse_arguments(["-p", 3001]) + server_options = parse_arguments(["-p", "3001"]) assert_equal [:Port], server_options[:user_supplied_options] - server_options = parse_arguments(["--port", 3001]) + server_options = parse_arguments(["--port", "3001"]) assert_equal [:Port], server_options[:user_supplied_options] server_options = parse_arguments(["-p3001", "-C", "--binding", "127.0.0.1"]) diff --git a/railties/test/credentials_test.rb b/railties/test/credentials_test.rb index 03370e0fc7..11765b0de5 100644 --- a/railties/test/credentials_test.rb +++ b/railties/test/credentials_test.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true require "isolation/abstract_unit" +require "env_helpers" class Rails::CredentialsTest < ActiveSupport::TestCase - include ActiveSupport::Testing::Isolation + include ActiveSupport::Testing::Isolation, EnvHelpers setup :build_app teardown :teardown_app @@ -38,6 +39,21 @@ class Rails::CredentialsTest < ActiveSupport::TestCase end end + test "reads credentials using environment variable key" do + with_credentials do |content, key| + Dir.chdir(app_path) do + Dir.mkdir("config/credentials") + File.write("config/credentials/production.yml.enc", content) + end + + switch_env("RAILS_MASTER_KEY", key) do + app("production") + + assert_equal "revealed", Rails.application.credentials.mystery + end + end + end + private def with_credentials key = "2117e775dc2024d4f49ddf3aeb585919" diff --git a/railties/test/generators/actions_test.rb b/railties/test/generators/actions_test.rb index a2b35124c5..af475400a1 100644 --- a/railties/test/generators/actions_test.rb +++ b/railties/test/generators/actions_test.rb @@ -303,9 +303,21 @@ class ActionsTest < Rails::Generators::TestCase end def test_generate_should_run_script_generate_with_argument_and_options - assert_called_with(generator, :run_ruby_script, ["bin/rails generate model MyModel", verbose: false]) do - action :generate, "model", "MyModel" + run_generator + action :generate, "model", "MyModel" + assert_file "app/models/my_model.rb", /MyModel/ + end + + def test_generate_aborts_when_subprocess_fails_if_requested + run_generator + content = capture(:stderr) do + assert_raises SystemExit do + action :generate, "model", "MyModel:ADsad", abort_on_failure: true + action :generate, "model", "MyModel" + end end + assert_match(/wrong constant name MyModel:aDsad/, content) + assert_no_file "app/models/my_model.rb" end def test_rake_should_run_rake_command_with_default_env diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 32d00f3157..154cd3e80c 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -1059,7 +1059,7 @@ class AppGeneratorTest < Rails::Generators::TestCase end def assert_bundler_command_called(target_command) - command_check = -> command do + command_check = -> (command, env = {}) do @command_called ||= 0 case command diff --git a/railties/test/generators/helper_generator_test.rb b/railties/test/generators/helper_generator_test.rb index 4cdb6adf82..5d6b6f8f67 100644 --- a/railties/test/generators/helper_generator_test.rb +++ b/railties/test/generators/helper_generator_test.rb @@ -38,4 +38,11 @@ class HelperGeneratorTest < Rails::Generators::TestCase end end end + + def test_helper_suffix_is_not_duplicated + run_generator %w(products_helper) + + assert_no_file "app/helpers/products_helper_helper.rb" + assert_file "app/helpers/products_helper.rb" + end end diff --git a/railties/test/generators/integration_test_generator_test.rb b/railties/test/generators/integration_test_generator_test.rb index 82791f1a27..2ec4895096 100644 --- a/railties/test/generators/integration_test_generator_test.rb +++ b/railties/test/generators/integration_test_generator_test.rb @@ -15,4 +15,11 @@ class IntegrationTestGeneratorTest < Rails::Generators::TestCase run_generator %w(iguchi/integration) assert_file "test/integration/iguchi/integration_test.rb", /class Iguchi::IntegrationTest < ActionDispatch::IntegrationTest/ end + + def test_test_suffix_is_not_duplicated + run_generator %w(integration_test) + + assert_no_file "test/integration/integration_test_test.rb" + assert_file "test/integration/integration_test.rb" + end end diff --git a/railties/test/generators/scaffold_generator_test.rb b/railties/test/generators/scaffold_generator_test.rb index 3cffdddd38..f672e301a7 100644 --- a/railties/test/generators/scaffold_generator_test.rb +++ b/railties/test/generators/scaffold_generator_test.rb @@ -5,7 +5,7 @@ require "rails/generators/rails/scaffold/scaffold_generator" class ScaffoldGeneratorTest < Rails::Generators::TestCase include GeneratorsTestHelper - arguments %w(product_line title:string product:belongs_to user:references) + arguments %w(product_line title:string approved:boolean product:belongs_to user:references) setup :copy_routes @@ -17,6 +17,7 @@ class ScaffoldGeneratorTest < Rails::Generators::TestCase assert_file "test/models/product_line_test.rb", /class ProductLineTest < ActiveSupport::TestCase/ assert_file "test/fixtures/product_lines.yml" assert_migration "db/migrate/create_product_lines.rb", /belongs_to :product/ + assert_migration "db/migrate/create_product_lines.rb", /boolean :approved/ assert_migration "db/migrate/create_product_lines.rb", /references :user/ # Route @@ -60,8 +61,8 @@ class ScaffoldGeneratorTest < Rails::Generators::TestCase assert_file "test/controllers/product_lines_controller_test.rb" do |test| assert_match(/class ProductLinesControllerTest < ActionDispatch::IntegrationTest/, test) - assert_match(/post product_lines_url, params: \{ product_line: \{ product_id: @product_line\.product_id, title: @product_line\.title, user_id: @product_line\.user_id \} \}/, test) - assert_match(/patch product_line_url\(@product_line\), params: \{ product_line: \{ product_id: @product_line\.product_id, title: @product_line\.title, user_id: @product_line\.user_id \} \}/, test) + assert_match(/post product_lines_url, params: \{ product_line: \{ approved: @product_line\.approved, product_id: @product_line\.product_id, title: @product_line\.title, user_id: @product_line\.user_id \} \}/, test) + assert_match(/patch product_line_url\(@product_line\), params: \{ product_line: \{ approved: @product_line\.approved, product_id: @product_line\.product_id, title: @product_line\.title, user_id: @product_line\.user_id \} \}/, test) end # System tests @@ -69,6 +70,7 @@ class ScaffoldGeneratorTest < Rails::Generators::TestCase assert_match(/class ProductLinesTest < ApplicationSystemTestCase/, test) assert_match(/visit product_lines_url/, test) assert_match(/fill_in "Title", with: @product_line\.title/, test) + assert_match(/check "Approved" if @product_line\.approved/, test) assert_match(/assert_text "Product line was successfully updated"/, test) end diff --git a/railties/test/generators/system_test_generator_test.rb b/railties/test/generators/system_test_generator_test.rb index efa70a050b..5742ba444d 100644 --- a/railties/test/generators/system_test_generator_test.rb +++ b/railties/test/generators/system_test_generator_test.rb @@ -16,4 +16,18 @@ class SystemTestGeneratorTest < Rails::Generators::TestCase run_generator %w(admin/user) assert_file "test/system/admin/users_test.rb", /class Admin::UsersTest < ApplicationSystemTestCase/ end + + def test_test_name_is_pluralized + run_generator %w(user) + + assert_no_file "test/system/user_test.rb" + assert_file "test/system/users_test.rb" + end + + def test_test_suffix_is_not_duplicated + run_generator %w(users_test) + + assert_no_file "test/system/users_test_test.rb" + assert_file "test/system/users_test.rb" + end end diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index d4eed69a87..39c936428f 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -197,6 +197,7 @@ module TestHelpers end add_to_config <<-RUBY + config.hosts << proc { true } config.eager_load = false config.session_store :cookie_store, key: "_myapp_session" config.active_support.deprecation = :log @@ -220,6 +221,7 @@ module TestHelpers @app = Class.new(Rails::Application) do def self.name; "RailtiesTestApp"; end end + @app.config.hosts << proc { true } @app.config.eager_load = false @app.config.session_store :cookie_store, key: "_myapp_session" @app.config.active_support.deprecation = :log |