diff options
49 files changed, 617 insertions, 207 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6871664a22..961b48733c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,7 +4,7 @@ * **Ensure the bug was not already reported** by searching on GitHub under [Issues](https://github.com/rails/rails/issues). -* If unable to find an open issue addressing the problem, [open a new one](https://github.com/rails/rails/issues/new). Be sure to include a **title and clear description**, as much relevant information as possible, and a **code sample** or an **executable test case** demonstrating the expected behavior that is not occurring. +* If you're unable to find an open issue addressing the problem, [open a new one](https://github.com/rails/rails/issues/new). Be sure to include a **title and clear description**, as much relevant information as possible, and a **code sample** or an **executable test case** demonstrating the expected behavior that is not occurring. * If possible, use the relevant bug report templates to create the issue. Simply copy the content of the appropriate template into a .rb file, make the necessary changes to demonstrate the issue, and **paste the content into the issue description**: * [**Active Record** (models, database) issues](https://github.com/rails/rails/blob/master/guides/bug_report_templates/active_record_master.rb) diff --git a/Gemfile.lock b/Gemfile.lock index baa8dc15c4..4f17b7a81a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -61,7 +61,6 @@ PATH globalid (>= 0.3.6) activemodel (5.0.0.beta1) activesupport (= 5.0.0.beta1) - builder (~> 3.1) activerecord (5.0.0.beta1) activemodel (= 5.0.0.beta1) activesupport (= 5.0.0.beta1) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index ac767652b8..b93ae8f8ff 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,23 @@ +* More explicit error message when running `rake routes`. `CONTROLLER` argument + can now be supplied in different ways: + `Rails::WelcomeController`, `Rails::Welcome`, `rails/welcome` + + Fixes #22918 + + *Edouard Chin* + +* Allow `ActionController::Parameters` instances as an argument to URL + helper methods. An `ArgumentError` will be raised if the passed parameters + are not secure. + + Fixes #22832 + + *Prathamesh Sonpatki* + +* Add option for per-form CSRF tokens. + + *Greg Ose & Ben Toews* + * Add tests and documentation for `ActionController::Renderers::use_renderers`. *Benjamin Fleischer* diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 26c4550f89..91b3403ad5 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -81,6 +81,10 @@ module ActionController #:nodoc: config_accessor :forgery_protection_origin_check self.forgery_protection_origin_check = false + # Controls whether form-action/method specific CSRF tokens are used. + config_accessor :per_form_csrf_tokens + self.per_form_csrf_tokens = false + helper_method :form_authenticity_token helper_method :protect_against_forgery? end @@ -277,16 +281,25 @@ module ActionController #:nodoc: end # Sets the token value for the current session. - def form_authenticity_token - masked_authenticity_token(session) + def form_authenticity_token(form_options: {}) + masked_authenticity_token(session, form_options: form_options) end # Creates a masked version of the authenticity token that varies # on each request. The masking is used to mitigate SSL attacks # like BREACH. - def masked_authenticity_token(session) + def masked_authenticity_token(session, form_options: {}) + action, method = form_options.values_at(:action, :method) + + raw_token = if per_form_csrf_tokens && action && method + action_path = normalize_action_path(action) + per_form_csrf_token(session, action_path, method) + else + real_csrf_token(session) + end + one_time_pad = SecureRandom.random_bytes(AUTHENTICITY_TOKEN_LENGTH) - encrypted_csrf_token = xor_byte_strings(one_time_pad, real_csrf_token(session)) + encrypted_csrf_token = xor_byte_strings(one_time_pad, raw_token) masked_token = one_time_pad + encrypted_csrf_token Base64.strict_encode64(masked_token) end @@ -316,28 +329,54 @@ module ActionController #:nodoc: compare_with_real_token masked_token, session elsif masked_token.length == AUTHENTICITY_TOKEN_LENGTH * 2 - # Split the token into the one-time pad and the encrypted - # value and decrypt it - one_time_pad = masked_token[0...AUTHENTICITY_TOKEN_LENGTH] - encrypted_csrf_token = masked_token[AUTHENTICITY_TOKEN_LENGTH..-1] - csrf_token = xor_byte_strings(one_time_pad, encrypted_csrf_token) - - compare_with_real_token csrf_token, session + csrf_token = unmask_token(masked_token) + compare_with_real_token(csrf_token, session) || + valid_per_form_csrf_token?(csrf_token, session) else false # Token is malformed end end + def unmask_token(masked_token) + # Split the token into the one-time pad and the encrypted + # value and decrypt it + one_time_pad = masked_token[0...AUTHENTICITY_TOKEN_LENGTH] + encrypted_csrf_token = masked_token[AUTHENTICITY_TOKEN_LENGTH..-1] + xor_byte_strings(one_time_pad, encrypted_csrf_token) + end + def compare_with_real_token(token, session) ActiveSupport::SecurityUtils.secure_compare(token, real_csrf_token(session)) end + def valid_per_form_csrf_token?(token, session) + if per_form_csrf_tokens + correct_token = per_form_csrf_token( + session, + normalize_action_path(request.fullpath), + request.request_method + ) + + ActiveSupport::SecurityUtils.secure_compare(token, correct_token) + else + false + end + end + def real_csrf_token(session) session[:_csrf_token] ||= SecureRandom.base64(AUTHENTICITY_TOKEN_LENGTH) Base64.strict_decode64(session[:_csrf_token]) end + def per_form_csrf_token(session, action_path, method) + OpenSSL::HMAC.digest( + OpenSSL::Digest::SHA256.new, + real_csrf_token(session), + [action_path, method.downcase].join("#") + ) + end + def xor_byte_strings(s1, s2) s1.bytes.zip(s2.bytes).map { |(c1,c2)| c1 ^ c2 }.pack('c*') end @@ -362,5 +401,9 @@ module ActionController #:nodoc: true end end + + def normalize_action_path(action_path) + action_path.split('?').first.to_s.chomp('/') + end end end diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index f3a5268d2e..69e6dd5215 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -65,7 +65,7 @@ module ActionDispatch routes = collect_routes(routes_to_display) if routes.none? - formatter.no_routes + formatter.no_routes(collect_routes(@routes), filter) return formatter.result end @@ -84,7 +84,8 @@ module ActionDispatch def filter_routes(filter) if filter - @routes.select { |route| route.defaults[:controller] == filter } + filter_name = filter.underscore.sub(/_controller$/, '') + @routes.select { |route| route.defaults[:controller] == filter_name } else @routes end @@ -136,17 +137,27 @@ module ActionDispatch @buffer << draw_header(routes) end - def no_routes - @buffer << <<-MESSAGE.strip_heredoc + def no_routes(routes, filter) + @buffer << + if routes.none? + <<-MESSAGE.strip_heredoc You don't have any routes defined! Please add some routes in config/routes.rb. - - For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html. MESSAGE + elsif missing_controller?(filter) + "The controller #{filter} does not exist!" + else + "No routes were found for this controller" + end + @buffer << "For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html." end private + def missing_controller?(controller_name) + [ controller_name.camelize, "#{controller_name.camelize}Controller" ].none?(&:safe_constantize) + end + def draw_section(routes) header_lengths = ['Prefix', 'Verb', 'URI Pattern'].map(&:length) name_width, verb_width, path_width = widths(routes).zip(header_lengths).map(&:max) @@ -187,7 +198,7 @@ module ActionDispatch def header(routes) end - def no_routes + def no_routes(*) @buffer << <<-MESSAGE.strip_heredoc <p>You don't have any routes defined!</p> <ul> diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 18cd205bad..522012063d 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -387,24 +387,6 @@ module ActionDispatch end module Base - # You can specify what Rails should route "/" to with the root method: - # - # root to: 'pages#main' - # - # For options, see +match+, as +root+ uses it internally. - # - # You can also pass a string which will expand - # - # root 'pages#main' - # - # You should put the root route at the top of <tt>config/routes.rb</tt>, - # because this means it will be matched first. As this is the most popular route - # of most Rails applications, this is beneficial. - def root(options = {}) - name = has_named_route?(:root) ? nil : :root - match '/', { as: name, via: :get }.merge!(options) - end - # Matches a url pattern to one or more routes. # # You should not use the +match+ method in your router @@ -1689,7 +1671,20 @@ to this: @set.add_route(mapping, ast, as, anchor) end - def root(path, options={}) + # You can specify what Rails should route "/" to with the root method: + # + # root to: 'pages#main' + # + # For options, see +match+, as +root+ uses it internally. + # + # You can also pass a string which will expand + # + # root 'pages#main' + # + # You should put the root route at the top of <tt>config/routes.rb</tt>, + # because this means it will be matched first. As this is the most popular route + # of most Rails applications, this is beneficial. + def root(path, options = {}) if path.is_a?(String) options[:to] = path elsif path.is_a?(Hash) and options.empty? @@ -1701,11 +1696,11 @@ to this: if @scope.resources? with_scope_level(:root) do path_scope(parent_resource.path) do - super(options) + match_root_route(options) end end else - super(options) + match_root_route(options) end end @@ -1900,6 +1895,11 @@ to this: ensure @scope = @scope.parent end + + def match_root_route(options) + name = has_named_route?(:root) ? nil : :root + match '/', { :as => name, :via => :get }.merge!(options) + end end # Routing Concerns allow you to declare common routes that can be reused diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 2bd2e53252..846b5fa1fc 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -281,8 +281,17 @@ module ActionDispatch helper = UrlHelper.create(route, opts, route_key, url_strategy) mod.module_eval do define_method(name) do |*args| - options = nil - options = args.pop if args.last.is_a? Hash + last = args.last + options = case last + when Hash + args.pop + when ActionController::Parameters + if last.permitted? + args.pop.to_h + else + raise ArgumentError, "Generating an URL from non sanitized request parameters is insecure!" + end + end helper.call self, args, options end end diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 87a8ed3dc9..1984ad8825 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -128,6 +128,23 @@ class CustomAuthenticityParamController < RequestForgeryProtectionControllerUsin end end +class PerFormTokensController < ActionController::Base + protect_from_forgery :with => :exception + self.per_form_csrf_tokens = true + + def index + render inline: "<%= form_tag (params[:form_path] || '/per_form_tokens/post_one'), method: (params[:form_method] || :post) %>" + end + + def post_one + render plain: '' + end + + def post_two + render plain: '' + end +end + # common test methods module RequestForgeryProtectionTests def setup @@ -623,3 +640,158 @@ class CustomAuthenticityParamControllerTest < ActionController::TestCase end end end + +class PerFormTokensControllerTest < ActionController::TestCase + def test_per_form_token_is_same_size_as_global_token + get :index + expected = ActionController::RequestForgeryProtection::AUTHENTICITY_TOKEN_LENGTH + actual = @controller.send(:per_form_csrf_token, session, '/path', 'post').size + assert_equal expected, actual + end + + def test_accepts_token_for_correct_path_and_method + get :index + + form_token = nil + assert_select 'input[name=custom_authenticity_token]' do |elts| + form_token = elts.first['value'] + assert_not_nil form_token + end + + actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) + expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'post') + assert_equal expected, actual + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one' + assert_nothing_raised do + post :post_one, params: {custom_authenticity_token: form_token} + end + assert_response :success + end + + def test_rejects_token_for_incorrect_path + get :index + + form_token = nil + assert_select 'input[name=custom_authenticity_token]' do |elts| + form_token = elts.first['value'] + assert_not_nil form_token + end + + actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) + expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'post') + assert_equal expected, actual + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_two' + assert_raises(ActionController::InvalidAuthenticityToken) do + post :post_two, params: {custom_authenticity_token: form_token} + end + end + + def test_rejects_token_for_incorrect_method + get :index + + form_token = nil + assert_select 'input[name=custom_authenticity_token]' do |elts| + form_token = elts.first['value'] + assert_not_nil form_token + end + + actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) + expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'post') + assert_equal expected, actual + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one' + assert_raises(ActionController::InvalidAuthenticityToken) do + patch :post_one, params: {custom_authenticity_token: form_token} + end + end + + def test_accepts_global_csrf_token + get :index + + token = @controller.send(:form_authenticity_token) + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one' + assert_nothing_raised do + post :post_one, params: {custom_authenticity_token: token} + end + assert_response :success + end + + def test_ignores_params + get :index, params: {form_path: '/per_form_tokens/post_one?foo=bar'} + + form_token = nil + assert_select 'input[name=custom_authenticity_token]' do |elts| + form_token = elts.first['value'] + assert_not_nil form_token + end + + actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) + expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'post') + assert_equal expected, actual + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one?foo=baz' + assert_nothing_raised do + post :post_one, params: {custom_authenticity_token: form_token, baz: 'foo'} + end + assert_response :success + end + + def test_ignores_trailing_slash_during_generation + get :index, params: {form_path: '/per_form_tokens/post_one/'} + + form_token = nil + assert_select 'input[name=custom_authenticity_token]' do |elts| + form_token = elts.first['value'] + assert_not_nil form_token + end + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one' + assert_nothing_raised do + post :post_one, params: {custom_authenticity_token: form_token} + end + assert_response :success + end + + def test_ignores_trailing_slash_during_validation + get :index + + form_token = nil + assert_select 'input[name=custom_authenticity_token]' do |elts| + form_token = elts.first['value'] + assert_not_nil form_token + end + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one/' + assert_nothing_raised do + post :post_one, params: {custom_authenticity_token: form_token} + end + assert_response :success + end + + def test_method_is_case_insensitive + get :index, params: {form_method: "POST"} + + form_token = nil + assert_select 'input[name=custom_authenticity_token]' do |elts| + form_token = elts.first['value'] + assert_not_nil form_token + end + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one/' + assert_nothing_raised do + post :post_one, params: {custom_authenticity_token: form_token} + end + assert_response :success + end +end diff --git a/actionpack/test/dispatch/routing/inspector_test.rb b/actionpack/test/dispatch/routing/inspector_test.rb index a17d07c40b..7382c267c7 100644 --- a/actionpack/test/dispatch/routing/inspector_test.rb +++ b/actionpack/test/dispatch/routing/inspector_test.rb @@ -7,6 +7,9 @@ class MountedRackApp end end +class Rails::DummyController +end + module ActionDispatch module Routing class RoutesInspectorTest < ActiveSupport::TestCase @@ -331,6 +334,41 @@ module ActionDispatch " cart GET /cart(.:format) cart#show" ], output end + + def test_routes_with_undefined_filter + output = draw(:filter => 'Rails::MissingController') do + get 'photos/:id' => 'photos#show', :id => /[A-Z]\d{5}/ + end + + assert_equal [ + "The controller Rails::MissingController does not exist!", + "For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html." + ], output + end + + def test_no_routes_matched_filter + output = draw(:filter => 'rails/dummy') do + get 'photos/:id' => 'photos#show', :id => /[A-Z]\d{5}/ + end + + assert_equal [ + "No routes were found for this controller", + "For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html." + ], output + end + + def test_no_routes_were_defined + output = draw(:filter => 'Rails::DummyController') { } + + assert_equal [ + "You don't have any routes defined!", + "", + "Please add some routes in config/routes.rb.", + "", + "For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html." + ], output + end + end end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 82222a141c..62d65ec5c0 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3578,6 +3578,27 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal 'HEAD', @response.body end + def test_passing_action_parameters_to_url_helpers_raises_error_if_parameters_are_not_permitted + draw do + root :to => 'projects#index' + end + params = ActionController::Parameters.new(id: '1') + + assert_raises ArgumentError do + root_path(params) + end + end + + def test_passing_action_parameters_to_url_helpers_is_allowed_if_parameters_are_permitted + draw do + root :to => 'projects#index' + end + params = ActionController::Parameters.new(id: '1') + params.permit! + + assert_equal '/?id=1', root_path(params) + end + private def draw(&block) diff --git a/actionpack/test/dispatch/ssl_test.rb b/actionpack/test/dispatch/ssl_test.rb index 11251c86e7..c66a0e6a7a 100644 --- a/actionpack/test/dispatch/ssl_test.rb +++ b/actionpack/test/dispatch/ssl_test.rb @@ -199,7 +199,7 @@ class SecureCookiesTest < SSLTest def test_cookies_as_not_secure_with_secure_cookies_disabled get headers: { 'Set-Cookie' => DEFAULT }, ssl_options: { secure_cookies: false } - assert_cookies *DEFAULT.split("\n") + assert_cookies(*DEFAULT.split("\n")) end def test_no_cookies diff --git a/actionview/lib/action_view/helpers/capture_helper.rb b/actionview/lib/action_view/helpers/capture_helper.rb index 93c7cba395..df8d0affd0 100644 --- a/actionview/lib/action_view/helpers/capture_helper.rb +++ b/actionview/lib/action_view/helpers/capture_helper.rb @@ -9,8 +9,8 @@ module ActionView # It provides a method to capture blocks into variables through capture and # a way to capture a block of markup for use in a layout through content_for. module CaptureHelper - # The capture method allows you to extract part of a template into a - # variable. You can then use this variable anywhere in your templates or layout. + # The capture method extracts part of a template as a String object. + # You can then use this object anywhere in your templates, layout, or helpers. # # The capture method can be used in ERB templates... # diff --git a/actionview/lib/action_view/helpers/form_tag_helper.rb b/actionview/lib/action_view/helpers/form_tag_helper.rb index 79a1a242bf..d521553481 100644 --- a/actionview/lib/action_view/helpers/form_tag_helper.rb +++ b/actionview/lib/action_view/helpers/form_tag_helper.rb @@ -870,10 +870,16 @@ module ActionView '' when /^post$/i, "", nil html_options["method"] = "post" - token_tag(authenticity_token) + token_tag(authenticity_token, form_options: { + action: html_options["action"], + method: "post" + }) else html_options["method"] = "post" - method_tag(method) + token_tag(authenticity_token) + method_tag(method) + token_tag(authenticity_token, form_options: { + action: html_options["action"], + method: method + }) end if html_options.delete("enforce_utf8") { true } diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb index baebc34b4b..3a4561a083 100644 --- a/actionview/lib/action_view/helpers/url_helper.rb +++ b/actionview/lib/action_view/helpers/url_helper.rb @@ -311,7 +311,11 @@ module ActionView form_options[:action] = url form_options[:'data-remote'] = true if remote - request_token_tag = form_method == 'post' ? token_tag : '' + request_token_tag = if form_method == 'post' + token_tag(nil, form_options: form_options) + else + '' + end html_options = convert_options_to_data_attributes(options, html_options) html_options['type'] = 'submit' @@ -579,9 +583,9 @@ module ActionView html_options["data-method"] = method end - def token_tag(token=nil) + def token_tag(token=nil, form_options: {}) if token != false && protect_against_forgery? - token ||= form_authenticity_token + token ||= form_authenticity_token(form_options: form_options) tag(:input, type: "hidden", name: request_forgery_protection_token.to_s, value: token) else '' diff --git a/actionview/test/lib/controller/fake_models.rb b/actionview/test/lib/controller/fake_models.rb index a3e7e4d980..a122fe17c9 100644 --- a/actionview/test/lib/controller/fake_models.rb +++ b/actionview/test/lib/controller/fake_models.rb @@ -38,6 +38,7 @@ class TicketType < Struct.new(:name) def initialize(*args) super + @persisted = false end def persisted=(boolean) diff --git a/actionview/test/template/url_helper_test.rb b/actionview/test/template/url_helper_test.rb index 784a48ed8d..89cabb8f6b 100644 --- a/actionview/test/template/url_helper_test.rb +++ b/actionview/test/template/url_helper_test.rb @@ -582,7 +582,7 @@ class UrlHelperTest < ActiveSupport::TestCase self.request_forgery end - def form_authenticity_token + def form_authenticity_token(*args) "secret" end diff --git a/activemodel/activemodel.gemspec b/activemodel/activemodel.gemspec index 8d00b3aa27..53206580f0 100644 --- a/activemodel/activemodel.gemspec +++ b/activemodel/activemodel.gemspec @@ -19,6 +19,4 @@ Gem::Specification.new do |s| s.require_path = 'lib' s.add_dependency 'activesupport', version - - s.add_dependency 'builder', '~> 3.1' end diff --git a/activemodel/lib/active_model/type/time.rb b/activemodel/lib/active_model/type/time.rb index fe09f63a87..34e09f0aba 100644 --- a/activemodel/lib/active_model/type/time.rb +++ b/activemodel/lib/active_model/type/time.rb @@ -38,7 +38,7 @@ module ActiveModel fast_string_to_time(dummy_time_value) || begin time_hash = ::Date._parse(dummy_time_value) return if time_hash[:hour].nil? - new_time(*time_hash.values_at(:year, :mon, :mday, :hour, :min, :sec, :sec_fraction)) + new_time(*time_hash.values_at(:year, :mon, :mday, :hour, :min, :sec, :sec_fraction, :offset)) end end end diff --git a/activemodel/test/cases/types_test.rb b/activemodel/test/cases/types_test.rb index f937208580..558c56f157 100644 --- a/activemodel/test/cases/types_test.rb +++ b/activemodel/test/cases/types_test.rb @@ -64,6 +64,9 @@ module ActiveModel time_string = Time.now.utc.strftime("%T") assert_equal time_string, type.cast(time_string).strftime("%T") + + assert_equal ::Time.utc(2000, 1, 1, 16, 45, 54), type.cast('2015-06-13T19:45:54+03:00') + assert_equal ::Time.utc(1999, 12, 31, 21, 7, 8), type.cast('06:07:08+09:00') end def test_type_cast_datetime_and_timestamp diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 9144ab6695..5790528e97 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,27 @@ +* Add short-hand methods for text and blob types in MySQL. + + In Pg and Sqlite3, `:text` and `:binary` have variable unlimited length. + But in MySQL, these have limited length for each types (ref #21591, #21619). + This change adds short-hand methods for each text and blob types. + + Example: + + create_table :foos do |t| + t.tinyblob :tiny_blob + t.mediumblob :medium_blob + t.longblob :long_blob + t.tinytext :tiny_text + t.mediumtext :medium_text + t.longtext :long_text + end + + *Ryuta Kamizono* + +* Take into account UTC offset when assigning string representation of + timestamp with offset specified to attribute of time type. + + *Andrey Novikov* + * When calling `first` with a `limit` argument, return directly from the `loaded?` records if available. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index d3bc378bea..0ac5e80119 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -58,8 +58,8 @@ module ActiveRecord # Returns an array of the values of the first column in a select: # select_values("SELECT id FROM companies LIMIT 3") => [1,2,3] - def select_values(arel, name = nil) - arel, binds = binds_from_relation arel, [] + def select_values(arel, name = nil, binds = []) + arel, binds = binds_from_relation arel, binds select_rows(to_sql(arel, binds), name, binds).map(&:first) end @@ -115,20 +115,21 @@ module ActiveRecord # If the next id was calculated in advance (as in Oracle), it should be # passed in as +id_value+. def insert(arel, name = nil, pk = nil, id_value = nil, sequence_name = nil, binds = []) - sql, binds = sql_for_insert(to_sql(arel, binds), pk, id_value, sequence_name, binds) - value = exec_insert(sql, name, binds, pk, sequence_name) - id_value || last_inserted_id(value) + insert_sql(to_sql(arel, binds), name, pk, id_value, sequence_name, binds) end + alias create insert # Executes the update statement and returns the number of rows affected. def update(arel, name = nil, binds = []) exec_update(to_sql(arel, binds), name, binds) end + alias update_sql update # Executes the delete statement and returns the number of rows affected. def delete(arel, name = nil, binds = []) exec_delete(to_sql(arel, binds), name, binds) end + alias delete_sql delete # Returns +true+ when the connection adapter supports prepared statement # caching, otherwise returns +false+ @@ -351,6 +352,13 @@ module ActiveRecord end alias join_to_delete join_to_update + # Executes an INSERT query and returns the new record's ID + def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil, binds = []) + sql, binds = sql_for_insert(sql, pk, id_value, sequence_name, binds) + value = exec_insert(sql, name, binds, pk, sequence_name) + id_value || last_inserted_id(value) + end + protected # Returns a subquery for the given key using the join information. @@ -369,22 +377,6 @@ module ActiveRecord exec_query(sql, name, binds, prepare: true) end - # Returns the last auto-generated ID from the affected table. - def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) - execute(sql, name) - id_value - end - - # Executes the update statement and returns the number of rows affected. - def update_sql(sql, name = nil) - execute(sql, name) - end - - # Executes the delete statement and returns the number of rows affected. - def delete_sql(sql, name = nil) - update_sql(sql, name) - end - def sql_for_insert(sql, pk, id_value, sequence_name, binds) [sql, binds] end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 9cbc973f2e..dce34a208f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -23,6 +23,7 @@ module ActiveRecord autoload :TableDefinition autoload :Table autoload :AlterTable + autoload :ReferenceDefinition end autoload_at 'active_record/connection_adapters/abstract/connection_pool' do diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index b55995bc8d..f63074ac21 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -393,11 +393,6 @@ module ActiveRecord yield execute(sql, name) end - def update_sql(sql, name = nil) #:nodoc: - super - @connection.affected_rows - end - def begin_db_transaction execute "BEGIN" end diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb index ca7dfda80d..157e75dbf7 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb @@ -11,6 +11,30 @@ module ActiveRecord args.each { |name| column(name, :blob, options) } end + def tinyblob(*args, **options) + args.each { |name| column(name, :tinyblob, options) } + end + + def mediumblob(*args, **options) + args.each { |name| column(name, :mediumblob, options) } + end + + def longblob(*args, **options) + args.each { |name| column(name, :longblob, options) } + end + + def tinytext(*args, **options) + args.each { |name| column(name, :tinytext, options) } + end + + def mediumtext(*args, **options) + args.each { |name| column(name, :mediumtext, options) } + end + + def longtext(*args, **options) + args.each { |name| column(name, :longtext, options) } + end + def json(*args, **options) args.each { |name| column(name, :json, options) } end diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 96a3a44b30..c3c5b660fd 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -136,12 +136,6 @@ module ActiveRecord alias exec_without_stmt exec_query - def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) - super - id_value || @connection.last_id - end - alias :create :insert_sql - def exec_insert(sql, name, binds, pk = nil, sequence_name = nil) execute to_sql(sql, binds), name end 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 0e0c0e993a..11a151edd5 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb @@ -52,8 +52,8 @@ module ActiveRecord end end - def select_values(arel, name = nil) - arel, binds = binds_from_relation arel, [] + def select_values(arel, name = nil, binds = []) + arel, binds = binds_from_relation arel, binds sql = to_sql(arel, binds) execute_and_clear(sql, name, binds) do |result| if result.nfields > 0 @@ -73,25 +73,13 @@ module ActiveRecord end # Executes an INSERT query and returns the new record's ID - def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) + def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil, binds = []) # :nodoc: unless pk # Extract the table from the insert sql. Yuck. table_ref = extract_table_ref_from_insert_sql(sql) pk = primary_key(table_ref) if table_ref end - - if pk && use_insert_returning? - select_value("#{sql} RETURNING #{quote_column_name(pk)}") - elsif pk - super - last_insert_id_value(sequence_name || default_sequence_name(table_ref, pk)) - else - super - end - end - - def create - super.insert + super end # The internal PostgreSQL identifier of the money data type. @@ -175,12 +163,6 @@ module ActiveRecord alias :exec_update :exec_delete def sql_for_insert(sql, pk, id_value, sequence_name, binds) - unless pk - # Extract the table from the insert sql. Yuck. - table_ref = extract_table_ref_from_insert_sql(sql) - pk = primary_key(table_ref) if table_ref - end - if pk && use_insert_returning? sql = "#{sql} RETURNING #{quote_column_name(pk)}" end @@ -202,11 +184,6 @@ module ActiveRecord end end - # Executes an UPDATE query and returns the number of affected tuples. - def update_sql(sql, name = nil) - super.cmd_tuples - end - # Begins a transaction. def begin_db_transaction execute "BEGIN" diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 163cbb875f..d1893f35f5 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -280,22 +280,6 @@ module ActiveRecord log(sql, name) { @connection.execute(sql) } end - def update_sql(sql, name = nil) #:nodoc: - super - @connection.changes - end - - def delete_sql(sql, name = nil) #:nodoc: - sql += " WHERE 1=1" unless sql =~ /WHERE/i - super sql, name - end - - def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) #:nodoc: - super - id_value || @connection.last_insert_row_id - end - alias :create :insert_sql - def select_rows(sql, name = nil, binds = []) exec_query(sql, name, binds).rows end diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index 831bfa2df3..1b94573870 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -41,8 +41,9 @@ module ActiveRecord end def remove_index(table_name, options = {}) - index_name = index_name_for_remove(table_name, options) - execute "DROP INDEX #{quote_column_name(index_name)} ON #{quote_table_name(table_name)}" + options = { column: options } unless options.is_a?(Hash) + options[:name] = index_name_for_remove(table_name, options) + super(table_name, options) end private diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index f5e69ec4fb..38916f7376 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -57,8 +57,10 @@ module ActiveRecord console do |app| require "active_record/railties/console_sandbox" if app.sandbox? require "active_record/base" - console = ActiveSupport::Logger.new(STDERR) - Rails.logger.extend ActiveSupport::Logger.broadcast console + unless ActiveSupport::Logger.logger_outputs_to?(Rails.logger, STDERR, STDOUT) + console = ActiveSupport::Logger.new(STDERR) + Rails.logger.extend ActiveSupport::Logger.broadcast console + end end runner do @@ -69,6 +71,7 @@ module ActiveRecord ActiveSupport.on_load(:active_record) do self.time_zone_aware_attributes = true self.default_timezone = :utc + self.time_zone_aware_types = ActiveRecord::Base.time_zone_aware_types end end diff --git a/activerecord/test/cases/database_statements_test.rb b/activerecord/test/cases/database_statements_test.rb index c689e97d83..ba085991e0 100644 --- a/activerecord/test/cases/database_statements_test.rb +++ b/activerecord/test/cases/database_statements_test.rb @@ -6,14 +6,23 @@ class DatabaseStatementsTest < ActiveRecord::TestCase end def test_insert_should_return_the_inserted_id + assert_not_nil return_the_inserted_id(method: :insert) + end + + def test_create_should_return_the_inserted_id + assert_not_nil return_the_inserted_id(method: :create) + end + + private + + def return_the_inserted_id(method:) # Oracle adapter uses prefetched primary key values from sequence and passes them to connection adapter insert method if current_adapter?(:OracleAdapter) sequence_name = "accounts_seq" id_value = @connection.next_sequence_value(sequence_name) - id = @connection.insert("INSERT INTO accounts (id, firm_id,credit_limit) VALUES (accounts_seq.nextval,42,5000)", nil, :id, id_value, sequence_name) + @connection.send(method, "INSERT INTO accounts (id, firm_id,credit_limit) VALUES (accounts_seq.nextval,42,5000)", nil, :id, id_value, sequence_name) else - id = @connection.insert("INSERT INTO accounts (firm_id,credit_limit) VALUES (42,5000)") + @connection.send(method, "INSERT INTO accounts (firm_id,credit_limit) VALUES (42,5000)") end - assert_not_nil id end end diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb index 267d2fcccc..b1e1d72944 100644 --- a/activerecord/test/cases/migration/compatibility_test.rb +++ b/activerecord/test/cases/migration/compatibility_test.rb @@ -21,6 +21,7 @@ module ActiveRecord teardown do connection.drop_table :testings rescue nil ActiveRecord::Migration.verbose = @verbose_was + ActiveRecord::SchemaMigration.delete_all end def test_migration_doesnt_remove_named_index @@ -37,6 +38,21 @@ module ActiveRecord assert_raise(StandardError) { ActiveRecord::Migrator.new(:up, [migration]).migrate } assert connection.index_exists?(:testings, :foo, name: "custom_index_name") end + + def test_migration_does_remove_unnamed_index + connection.add_index :testings, :bar + + migration = Class.new(ActiveRecord::Migration[4.2]) { + def version; 101 end + def migrate(x) + remove_index :testings, :bar + end + }.new + + assert connection.index_exists?(:testings, :bar) + ActiveRecord::Migrator.new(:up, [migration]).migrate + assert_not connection.index_exists?(:testings, :bar) + end end end end diff --git a/activerecord/test/schema/mysql2_specific_schema.rb b/activerecord/test/schema/mysql2_specific_schema.rb index 9e1fe32c2d..752572a79c 100644 --- a/activerecord/test/schema/mysql2_specific_schema.rb +++ b/activerecord/test/schema/mysql2_specific_schema.rb @@ -2,18 +2,18 @@ ActiveRecord::Schema.define do create_table :binary_fields, force: true do |t| t.binary :var_binary, limit: 255 t.binary :var_binary_large, limit: 4095 - t.blob :tiny_blob, limit: 255 - t.binary :normal_blob, limit: 65535 - t.binary :medium_blob, limit: 16777215 - t.binary :long_blob, limit: 2147483647 - t.text :tiny_text, limit: 255 - t.text :normal_text, limit: 65535 - t.text :medium_text, limit: 16777215 - t.text :long_text, limit: 2147483647 + t.tinyblob :tiny_blob + t.blob :normal_blob + t.mediumblob :medium_blob + t.longblob :long_blob + t.tinytext :tiny_text + t.text :normal_text + t.mediumtext :medium_text + t.longtext :long_text + + t.index :var_binary end - add_index :binary_fields, :var_binary - create_table :key_tests, force: true, :options => 'ENGINE=MyISAM' do |t| t.string :awesome t.string :pizza diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index ba696374b5..cebe19be89 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,7 @@ +* Match `HashWithIndifferentAccess#default`'s behaviour with `Hash#default` + + *David Cornu* + * Adds `:exception_object` key to `ActiveSupport::Notifications::Instrumenter` payload when an exception is raised. Adds new key/value pair to payload when an exception is raised: e.g. `:exception_object => #<RuntimeError: FAIL>`. diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index 4ff35a45a1..b878f31e75 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -68,12 +68,10 @@ module ActiveSupport end end - def default(key = nil) - if key.is_a?(Symbol) && include?(key = key.to_s) - self[key] - else - super - end + def default(*args) + key = args.first + args[0] = key.to_s if key.is_a?(Symbol) + super(*args) end def self.new_from_hash_copying_default(hash) @@ -159,6 +157,20 @@ module ActiveSupport alias_method :has_key?, :key? alias_method :member?, :key? + + # Same as <tt>Hash#[]</tt> where the key passed as argument can be + # either a string or a symbol: + # + # counters = ActiveSupport::HashWithIndifferentAccess.new + # counters[:foo] = 1 + # + # counters['foo'] # => 1 + # counters[:foo] # => 1 + # counters[:zoo] # => nil + def [](key) + super(convert_key(key)) + end + # Same as <tt>Hash#fetch</tt> where the key passed as argument can be # either a string or a symbol: # diff --git a/activesupport/lib/active_support/logger.rb b/activesupport/lib/active_support/logger.rb index 65049f8498..7626b28108 100644 --- a/activesupport/lib/active_support/logger.rb +++ b/activesupport/lib/active_support/logger.rb @@ -5,26 +5,27 @@ module ActiveSupport class Logger < ::Logger include LoggerSilence - # If +true+, will broadcast all messages sent to this logger to any - # logger linked to this one via +broadcast+. + # Returns true if the logger destination matches one of the sources # - # If +false+, the logger will still forward calls to +close+, +progname=+, - # +formatter=+ and +level+ to any linked loggers, but no calls to +add+ or - # +<<+. - # - # Defaults to +true+. - attr_accessor :broadcast_messages # :nodoc: + # logger = Logger.new(STDOUT) + # ActiveSupport::Logger.logger_outputs_to?(logger, STDOUT) + # # => true + def self.logger_outputs_to?(logger, *sources) + logdev = logger.instance_variable_get("@logdev") + logger_source = logdev.dev if logdev.respond_to?(:dev) + sources.any? { |source| source == logger_source } + end # Broadcasts logs to multiple loggers. def self.broadcast(logger) # :nodoc: Module.new do define_method(:add) do |*args, &block| - logger.add(*args, &block) if broadcast_messages + logger.add(*args, &block) super(*args, &block) end define_method(:<<) do |x| - logger << x if broadcast_messages + logger << x super(x) end @@ -53,7 +54,6 @@ module ActiveSupport def initialize(*args) super @formatter = SimpleFormatter.new - @broadcast_messages = true after_initialize if respond_to? :after_initialize end diff --git a/activesupport/lib/active_support/logger_silence.rb b/activesupport/lib/active_support/logger_silence.rb index 690e5596f7..125d81d973 100644 --- a/activesupport/lib/active_support/logger_silence.rb +++ b/activesupport/lib/active_support/logger_silence.rb @@ -42,4 +42,4 @@ module LoggerSilence yield self end end -end +end
\ No newline at end of file diff --git a/activesupport/test/broadcast_logger_test.rb b/activesupport/test/broadcast_logger_test.rb index e7d56c80c3..6d4e3b74f7 100644 --- a/activesupport/test/broadcast_logger_test.rb +++ b/activesupport/test/broadcast_logger_test.rb @@ -2,69 +2,56 @@ require 'abstract_unit' module ActiveSupport class BroadcastLoggerTest < TestCase - attr_reader :logger, :receiving_logger + attr_reader :logger, :log1, :log2 def setup - @logger = FakeLogger.new - @receiving_logger = FakeLogger.new - @logger.extend Logger.broadcast @receiving_logger + @log1 = FakeLogger.new + @log2 = FakeLogger.new + @log1.extend Logger.broadcast @log2 + @logger = @log1 end def test_debug logger.debug "foo" - assert_equal 'foo', logger.adds.first[2] - assert_equal 'foo', receiving_logger.adds.first[2] - end - - def test_debug_without_message_broadcasts - logger.broadcast_messages = false - logger.debug "foo" - assert_equal 'foo', logger.adds.first[2] - assert_equal [], receiving_logger.adds + assert_equal 'foo', log1.adds.first[2] + assert_equal 'foo', log2.adds.first[2] end def test_close logger.close - assert logger.closed, 'should be closed' - assert receiving_logger.closed, 'should be closed' + assert log1.closed, 'should be closed' + assert log2.closed, 'should be closed' end def test_chevrons logger << "foo" - assert_equal %w{ foo }, logger.chevrons - assert_equal %w{ foo }, receiving_logger.chevrons - end - - def test_chevrons_without_message_broadcasts - logger.broadcast_messages = false - logger << "foo" - assert_equal %w{ foo }, logger.chevrons - assert_equal [], receiving_logger.chevrons + assert_equal %w{ foo }, log1.chevrons + assert_equal %w{ foo }, log2.chevrons end def test_level assert_nil logger.level logger.level = 10 - assert_equal 10, logger.level - assert_equal 10, receiving_logger.level + assert_equal 10, log1.level + assert_equal 10, log2.level end def test_progname assert_nil logger.progname logger.progname = 10 - assert_equal 10, logger.progname - assert_equal 10, receiving_logger.progname + assert_equal 10, log1.progname + assert_equal 10, log2.progname end def test_formatter assert_nil logger.formatter logger.formatter = 10 - assert_equal 10, logger.formatter - assert_equal 10, receiving_logger.formatter + assert_equal 10, log1.formatter + assert_equal 10, log2.formatter end class FakeLogger attr_reader :adds, :closed, :chevrons - attr_accessor :level, :progname, :formatter, :broadcast_messages + attr_accessor :level, :progname, :formatter def initialize @adds = [] @@ -73,7 +60,6 @@ module ActiveSupport @level = nil @progname = nil @formatter = nil - @broadcast_messages = true end def debug msg, &block diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index 2119352df0..1b66f784e4 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -1587,9 +1587,9 @@ class HashToXmlTest < ActiveSupport::TestCase assert_equal 3, hash_wia[:new_key] end - def test_should_use_default_proc_if_no_key_is_supplied + def test_should_return_nil_if_no_key_is_supplied hash_wia = HashWithIndifferentAccess.new { 1 + 2 } - assert_equal 3, hash_wia.default + assert_equal nil, hash_wia.default end def test_should_use_default_value_for_unknown_key diff --git a/activesupport/test/logger_test.rb b/activesupport/test/logger_test.rb index a57dc7a241..317e09b7f2 100644 --- a/activesupport/test/logger_test.rb +++ b/activesupport/test/logger_test.rb @@ -17,6 +17,14 @@ class LoggerTest < ActiveSupport::TestCase @logger = Logger.new(@output) end + def test_log_outputs_to + assert Logger.logger_outputs_to?(@logger, @output), "Expected logger_outputs_to? @output to return true but was false" + assert Logger.logger_outputs_to?(@logger, @output, STDOUT), "Expected logger_outputs_to? @output or STDOUT to return true but was false" + + assert_not Logger.logger_outputs_to?(@logger, STDOUT), "Expected logger_outputs_to? to STDOUT to return false, but was true" + assert_not Logger.logger_outputs_to?(@logger, STDOUT, STDERR), "Expected logger_outputs_to? to STDOUT or STDERR to return false, but was true" + end + def test_write_binary_data_to_existing_file t = Tempfile.new ['development', 'log'] t.binmode @@ -65,7 +73,7 @@ class LoggerTest < ActiveSupport::TestCase def test_should_not_log_debug_messages_when_log_level_is_info @logger.level = Logger::INFO @logger.add(Logger::DEBUG, @message) - assert ! @output.string.include?(@message) + assert_not @output.string.include?(@message) end def test_should_add_message_passed_as_block_when_using_add @@ -129,7 +137,7 @@ class LoggerTest < ActiveSupport::TestCase @logger.error "THIS IS HERE" end - assert !@output.string.include?("NOT THERE") + assert_not @output.string.include?("NOT THERE") assert @output.string.include?("THIS IS HERE") end diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 14ba343520..e9261a3dab 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -345,6 +345,8 @@ The schema dumper adds one additional configuration option: * `config.action_controller.forgery_protection_origin_check` configures whether the HTTP `Origin` header should be checked against the site's origin as an additional CSRF defense. +* `config.action_controller.per_form_csrf_tokens` configures whether CSRF tokens are only valid for the method/action they were generated for. + * `config.action_controller.relative_url_root` can be used to tell Rails that you are [deploying to a subdirectory](configuring.html#deploy-to-a-subdirectory-relative-url-root). The default is `ENV['RAILS_RELATIVE_URL_ROOT']`. * `config.action_controller.permit_all_parameters` sets all the parameters for mass assignment to be permitted by default. The default value is `false`. diff --git a/railties/lib/rails/commands/runner.rb b/railties/lib/rails/commands/runner.rb index 86bce9b2fe..babb197ba1 100644 --- a/railties/lib/rails/commands/runner.rb +++ b/railties/lib/rails/commands/runner.rb @@ -58,5 +58,11 @@ elsif File.exist?(code_or_file) $0 = code_or_file Kernel.load code_or_file else - eval(code_or_file, binding, __FILE__, __LINE__) + begin + eval(code_or_file, binding, __FILE__, __LINE__) + rescue SyntaxError,NameError => err + $stderr.puts "Please specify a valid ruby command or the path of a script to run." + $stderr.puts "Run '#{$0} -h' for help." + exit 1 + end end diff --git a/railties/lib/rails/commands/server.rb b/railties/lib/rails/commands/server.rb index d3ea441f8e..45d649ec31 100644 --- a/railties/lib/rails/commands/server.rb +++ b/railties/lib/rails/commands/server.rb @@ -133,11 +133,13 @@ module Rails def log_to_stdout wrapped_app # touch the app so the logger is set up - console = ActiveSupport::Logger.new($stdout) + console = ActiveSupport::Logger.new(STDOUT) console.formatter = Rails.logger.formatter console.level = Rails.logger.level - Rails.logger.extend(ActiveSupport::Logger.broadcast(console)) + unless ActiveSupport::Logger.logger_outputs_to?(Rails.logger, STDOUT) + Rails.logger.extend(ActiveSupport::Logger.broadcast(console)) + end end end end diff --git a/railties/lib/rails/generators/named_base.rb b/railties/lib/rails/generators/named_base.rb index 658d883883..efbf51ddfb 100644 --- a/railties/lib/rails/generators/named_base.rb +++ b/railties/lib/rails/generators/named_base.rb @@ -161,6 +161,10 @@ module Rails @route_url ||= class_path.collect {|dname| "/" + dname }.join + "/" + plural_file_name end + def url_helper_prefix + @url_helper_prefix ||= (class_path + [file_name]).join('_') + end + # Tries to retrieve the application name or simply return application. def application_name if defined?(Rails) && Rails.application diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt index 8d59a6fcf3..82509f5ef5 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt @@ -54,7 +54,7 @@ Rails.application.configure do config.log_level = :debug # Prepend all log lines with the following tags. - # config.log_tags = [ :subdomain, :request_id ] + config.log_tags = [ :request_id ] # Use a different logger for distributed setups. # require 'syslog/logger' diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/per_form_csrf_tokens.rb b/railties/lib/rails/generators/rails/app/templates/config/initializers/per_form_csrf_tokens.rb new file mode 100644 index 0000000000..1f569dedfd --- /dev/null +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/per_form_csrf_tokens.rb @@ -0,0 +1,4 @@ +# Be sure to restart your server when you modify this file. + +# Enable per-form CSRF tokens. +Rails.application.config.action_controller.per_form_csrf_tokens = true diff --git a/railties/lib/rails/generators/test_unit/controller/templates/functional_test.rb b/railties/lib/rails/generators/test_unit/controller/templates/functional_test.rb index 4f2ceb8589..ff41fef9e9 100644 --- a/railties/lib/rails/generators/test_unit/controller/templates/functional_test.rb +++ b/railties/lib/rails/generators/test_unit/controller/templates/functional_test.rb @@ -13,7 +13,7 @@ class <%= class_name %>ControllerTest < ActionDispatch::IntegrationTest <% else -%> <% actions.each do |action| -%> test "should get <%= action %>" do - get <%= file_name %>_<%= action %>_url + get <%= url_helper_prefix %>_<%= action %>_url assert_response :success end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 50d343865c..7bcfc86d03 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -79,6 +79,24 @@ module ApplicationTests end end + test "By default logs tags are not set in development" do + restore_default_config + + with_rails_env "development" do + app 'development' + assert Rails.application.config.log_tags.blank? + end + end + + test "By default logs are tagged with :request_id in production" do + restore_default_config + + with_rails_env "production" do + app 'production' + assert_equal [:request_id], Rails.application.config.log_tags + end + end + test "lib dir is on LOAD_PATH during config" do app_file 'lib/my_logger.rb', <<-RUBY require "logger" @@ -657,7 +675,7 @@ module ApplicationTests private - def form_authenticity_token; token; end # stub the authenticy token + def form_authenticity_token(*args); token; end # stub the authenticy token end RUBY diff --git a/railties/test/application/runner_test.rb b/railties/test/application/runner_test.rb index 0c180339b4..9f15ce5e85 100644 --- a/railties/test/application/runner_test.rb +++ b/railties/test/application/runner_test.rb @@ -74,6 +74,16 @@ module ApplicationTests assert_match "development", Dir.chdir(app_path) { `bin/rails runner "puts Rails.env"` } end + def test_runner_detects_syntax_errors + Dir.chdir(app_path) { `bin/rails runner "puts 'hello world" 2>&1` } + refute $?.success? + end + + def test_runner_detects_bad_script_name + Dir.chdir(app_path) { `bin/rails runner "iuiqwiourowe" 2>&1` } + refute $?.success? + end + def test_environment_with_rails_env with_rails_env "production" do assert_match "production", Dir.chdir(app_path) { `bin/rails runner "puts Rails.env"` } diff --git a/railties/test/application/test_runner_test.rb b/railties/test/application/test_runner_test.rb index 92a9b99fd8..868153762d 100644 --- a/railties/test/application/test_runner_test.rb +++ b/railties/test/application/test_runner_test.rb @@ -234,6 +234,11 @@ module ApplicationTests assert_match "0 failures, 0 errors, 0 skips", run_test_command('') end + def test_generated_controller_works_with_rails_test + create_controller + assert_match "0 failures, 0 errors, 0 skips", run_test_command('') + end + def test_run_multiple_folders create_test_file :models, 'account' create_test_file :controllers, 'accounts_controller' @@ -449,6 +454,10 @@ module ApplicationTests run_migration end + def create_controller + script 'generate controller admin/dashboard index' + end + def run_migration Dir.chdir(app_path) { `bin/rake db:migrate` } end |