diff options
65 files changed, 835 insertions, 589 deletions
@@ -76,7 +76,7 @@ We encourage you to contribute to Ruby on Rails! Please check out the ## Code Status -* [](https://travis-ci.org/rails/rails) +* [](https://travis-ci.org/rails/rails) ## License diff --git a/actionmailer/lib/action_mailer/preview.rb b/actionmailer/lib/action_mailer/preview.rb index fde0e86f5b..33a9faa7e7 100644 --- a/actionmailer/lib/action_mailer/preview.rb +++ b/actionmailer/lib/action_mailer/preview.rb @@ -20,7 +20,7 @@ module ActionMailer interceptors.flatten.compact.each { |interceptor| register_preview_interceptor(interceptor) } end - # Register am Interceptor which will be called before mail is previewed. + # Register an Interceptor which will be called before mail is previewed. # Either a class or a string can be passed in as the Interceptor. If a # string is passed in it will be <tt>constantize</tt>d. def register_preview_interceptor(interceptor) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 15541d58b5..342f670e78 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,21 @@ +* Add new config option `config.action_dispatch.cookies_serializer` for + specifying a serializer for the signed and encrypted cookie jars. + + The possible values are: + + * `:json` - serialize cookie values with `JSON` + * `:marshal` - serialize cookie values with `Marshal` + * `:hybrid` - transparently migrate existing `Marshal` cookie values to `JSON` + + For new apps `:json` option is added by default and `:marshal` is used + when no option is specified to maintain backwards compatibility. + + *Łukasz Sarnacki*, *Matt Aimonetti*, *Guillermo Iguaran*, *Godfrey Chan*, *Rafael Mendonça França* + +* `FlashHash` now behaves like a `HashWithIndifferentAccess`. + + *Guillermo Iguaran* + * Set the `:shallow_path` scope option as each scope is generated rather than waiting until the `shallow` option is set. Also make the behavior of the `:shallow` resource option consistent with the behavior of the `shallow` method. @@ -16,21 +34,6 @@ *Josh Jordan* -* Add `:serializer` option for `config.session_store :cookie_store`. This - changes default serializer when using `:cookie_store`. - - It is possible to pass: - - * `:json` which is a secure wrapper on JSON using `JSON.parse` and - `JSON.generate` methods with quirks mode; - * `:marshal` which is a wrapper on Marshal; - * serializer class with `load` and `dump` methods defined. - - For new apps `:json` option is added by default and :marshal is used - when no option is specified. - - *Łukasz Sarnacki*, *Matt Aimonetti* - * Ensure that `request.filtered_parameters` is reset between calls to `process` in `ActionController::TestCase`. diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index d5e08b7034..1974bbf529 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -236,6 +236,18 @@ module ActionController #:nodoc: # end # end # + # You can also set an array of variants: + # + # request.variant = [:tablet, :phone] + # + # which will work similarly to formats and MIME types negotiation. If there will be no + # :tablet variant declared, :phone variant will be picked: + # + # respond_to do |format| + # format.html.none + # format.html.phone # this gets rendered + # end + # # Be sure to check the documentation of +respond_with+ and # <tt>ActionController::MimeResponds.respond_to</tt> for more examples. def respond_to(*mimes, &block) @@ -488,7 +500,7 @@ module ActionController #:nodoc: response else # `format.html{ |variant| variant.phone }` - variant block syntax variant_collector = VariantCollector.new(@variant) - response.call(variant_collector) #call format block with variants collector + response.call(variant_collector) # call format block with variants collector variant_collector.variant end end @@ -519,15 +531,15 @@ module ActionController #:nodoc: end def variant - key = if @variant.nil? - :none - elsif @variants.has_key?(@variant) - @variant + if @variant.nil? + @variants[:none] || @variants[:any] + elsif (@variants.keys & @variant).any? + @variant.each do |v| + return @variants[v] if @variants.key?(v) + end else - :any + @variants[:any] end - - @variants[key] end end end diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index a56d827b1a..3dd2e2a45c 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -84,8 +84,6 @@ module ActionDispatch autoload :CookieStore, 'action_dispatch/middleware/session/cookie_store' autoload :MemCacheStore, 'action_dispatch/middleware/session/mem_cache_store' autoload :CacheStore, 'action_dispatch/middleware/session/cache_store' - autoload :JsonSerializer, 'action_dispatch/middleware/session/json_serializer' - autoload :MarshalSerializer, 'action_dispatch/middleware/session/marshal_serializer' end mattr_accessor :test_app diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index c33ba201e1..b803ce8b6f 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -68,10 +68,12 @@ module ActionDispatch # Sets the \variant for template. def variant=(variant) - if variant.is_a? Symbol + if variant.is_a?(Symbol) + @variant = [variant] + elsif variant.is_a?(Array) && variant.any? && variant.all?{ |v| v.is_a?(Symbol) } @variant = variant else - raise ArgumentError, "request.variant must be set to a Symbol, not a #{variant.class}. " \ + raise ArgumentError, "request.variant must be set to a Symbol or an Array of Symbols, not a #{variant.class}. " \ "For security reasons, never directly set the variant to a user-provided value, " \ "like params[:variant].to_sym. Check user-provided value against a whitelist first, " \ "then set the variant: request.variant = :tablet if params[:variant] == 'tablet'" diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 531654895b..18e64704f6 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -89,7 +89,7 @@ module ActionDispatch ENCRYPTED_SIGNED_COOKIE_SALT = "action_dispatch.encrypted_signed_cookie_salt".freeze SECRET_TOKEN = "action_dispatch.secret_token".freeze SECRET_KEY_BASE = "action_dispatch.secret_key_base".freeze - SESSION_SERIALIZER = "action_dispatch.session_serializer".freeze + COOKIES_SERIALIZER = "action_dispatch.cookies_serializer".freeze # Cookies can typically store 4096 bytes. MAX_COOKIE_SIZE = 4096 @@ -181,7 +181,7 @@ module ActionDispatch def verify_and_upgrade_legacy_signed_message(name, signed_message) @legacy_verifier.verify(signed_message).tap do |value| - self[name] = value + self[name] = { value: value } end rescue ActiveSupport::MessageVerifier::InvalidSignature nil @@ -212,7 +212,7 @@ module ActionDispatch secret_token: env[SECRET_TOKEN], secret_key_base: env[SECRET_KEY_BASE], upgrade_legacy_signed_cookies: env[SECRET_TOKEN].present? && env[SECRET_KEY_BASE].present?, - session_serializer: env[SESSION_SERIALIZER] + serializer: env[COOKIES_SERIALIZER] } end @@ -374,28 +374,89 @@ module ActionDispatch end end + class JsonSerializer + def self.load(value) + JSON.parse(value, quirks_mode: true) + end + + def self.dump(value) + JSON.generate(value, quirks_mode: true) + end + end + + # Passing the NullSerializer downstream to the Message{Encryptor,Verifier} + # allows us to handle the (de)serialization step within the cookie jar, + # which gives us the opportunity to detect and migrate legacy cookies. + class NullSerializer + def self.load(value) + value + end + + def self.dump(value) + value + end + end + + module SerializedCookieJars + MARSHAL_SIGNATURE = "\x04\x08".freeze + + protected + def needs_migration?(value) + @options[:serializer] == :hybrid && value.start_with?(MARSHAL_SIGNATURE) + end + + def serialize(name, value) + serializer.dump(value) + end + + def deserialize(name, value) + if value + if needs_migration?(value) + Marshal.load(value).tap do |v| + self[name] = { value: v } + end + else + serializer.load(value) + end + end + end + + def serializer + serializer = @options[:serializer] || :marshal + case serializer + when :marshal + Marshal + when :json, :hybrid + JsonSerializer + else + serializer + end + end + end + class SignedCookieJar #:nodoc: include ChainedCookieJars + include SerializedCookieJars def initialize(parent_jar, key_generator, options = {}) @parent_jar = parent_jar @options = options secret = key_generator.generate_key(@options[:signed_cookie_salt]) - @verifier = ActiveSupport::MessageVerifier.new(secret) + @verifier = ActiveSupport::MessageVerifier.new(secret, serializer: NullSerializer) end def [](name) if signed_message = @parent_jar[name] - verify(signed_message) + deserialize name, verify(signed_message) end end def []=(name, options) if options.is_a?(Hash) options.symbolize_keys! - options[:value] = @verifier.generate(options[:value]) + options[:value] = @verifier.generate(serialize(name, options[:value])) else - options = { :value => @verifier.generate(options) } + options = { :value => @verifier.generate(serialize(name, options)) } end raise CookieOverflow if options[:value].size > MAX_COOKIE_SIZE @@ -419,13 +480,14 @@ module ActionDispatch def [](name) if signed_message = @parent_jar[name] - verify(signed_message) || verify_and_upgrade_legacy_signed_message(name, signed_message) + deserialize(name, verify(signed_message)) || verify_and_upgrade_legacy_signed_message(name, signed_message) end end end class EncryptedCookieJar #:nodoc: include ChainedCookieJars + include SerializedCookieJars def initialize(parent_jar, key_generator, options = {}) if ActiveSupport::LegacyKeyGenerator === key_generator @@ -437,12 +499,12 @@ module ActionDispatch @options = options secret = key_generator.generate_key(@options[:encrypted_cookie_salt]) sign_secret = key_generator.generate_key(@options[:encrypted_signed_cookie_salt]) - @encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: serializer) + @encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: NullSerializer) end def [](name) if encrypted_message = @parent_jar[name] - decrypt_and_verify(encrypted_message) + deserialize name, decrypt_and_verify(encrypted_message) end end @@ -452,7 +514,8 @@ module ActionDispatch else options = { :value => options } end - options[:value] = @encryptor.encrypt_and_sign(options[:value]) + + options[:value] = @encryptor.encrypt_and_sign(serialize(name, options[:value])) raise CookieOverflow if options[:value].size > MAX_COOKIE_SIZE @parent_jar[name] = options @@ -464,18 +527,6 @@ module ActionDispatch rescue ActiveSupport::MessageVerifier::InvalidSignature, ActiveSupport::MessageEncryptor::InvalidMessage nil end - - def serializer - serializer = @options[:session_serializer] || :marshal - case serializer - when :marshal - ActionDispatch::Session::MarshalSerializer - when :json - ActionDispatch::Session::JsonSerializer - else - serializer - end - end end # UpgradeLegacyEncryptedCookieJar is used by ActionDispatch::Session::CookieStore @@ -487,7 +538,7 @@ module ActionDispatch def [](name) if encrypted_or_signed_message = @parent_jar[name] - decrypt_and_verify(encrypted_or_signed_message) || verify_and_upgrade_legacy_signed_message(name, encrypted_or_signed_message) + deserialize(name, decrypt_and_verify(encrypted_or_signed_message)) || verify_and_upgrade_legacy_signed_message(name, encrypted_or_signed_message) end end end diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index 89003e7a5e..4821d2a899 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/hash/keys' + module ActionDispatch class Request < Rack::Request # Access the contents of the flash. Use <tt>flash["notice"]</tt> to @@ -50,13 +52,14 @@ module ActionDispatch end def []=(k, v) + k = k.to_s @flash[k] = v @flash.discard(k) v end def [](k) - @flash[k] + @flash[k.to_s] end # Convenience accessor for <tt>flash.now[:alert]=</tt>. @@ -92,8 +95,8 @@ module ActionDispatch end def initialize(flashes = {}, discard = []) #:nodoc: - @discard = Set.new(discard) - @flashes = flashes + @discard = Set.new(stringify_array(discard)) + @flashes = flashes.stringify_keys @now = nil end @@ -106,17 +109,18 @@ module ActionDispatch end def []=(k, v) + k = k.to_s @discard.delete k @flashes[k] = v end def [](k) - @flashes[k] + @flashes[k.to_s] end def update(h) #:nodoc: - @discard.subtract h.keys - @flashes.update h + @discard.subtract stringify_array(h.keys) + @flashes.update h.stringify_keys self end @@ -129,6 +133,7 @@ module ActionDispatch end def delete(key) + key = key.to_s @discard.delete key @flashes.delete key self @@ -155,7 +160,7 @@ module ActionDispatch def replace(h) #:nodoc: @discard.clear - @flashes.replace h + @flashes.replace h.stringify_keys self end @@ -186,6 +191,7 @@ module ActionDispatch # flash.keep # keeps the entire flash # flash.keep(:notice) # keeps only the "notice" entry, the rest of the flash is discarded def keep(k = nil) + k = k.to_s if k @discard.subtract Array(k || keys) k ? self[k] : self end @@ -195,6 +201,7 @@ module ActionDispatch # flash.discard # discard the entire flash at the end of the current action # flash.discard(:warning) # discard only the "warning" entry at the end of the current action def discard(k = nil) + k = k.to_s if k @discard.merge Array(k || keys) k ? self[k] : self end @@ -231,6 +238,12 @@ module ActionDispatch def now_is_loaded? @now end + + def stringify_array(array) + array.map do |item| + item.kind_of?(Symbol) ? item.to_s : item + end + end end def initialize(app) diff --git a/actionpack/lib/action_dispatch/middleware/session/json_serializer.rb b/actionpack/lib/action_dispatch/middleware/session/json_serializer.rb deleted file mode 100644 index d341853f7a..0000000000 --- a/actionpack/lib/action_dispatch/middleware/session/json_serializer.rb +++ /dev/null @@ -1,13 +0,0 @@ -module ActionDispatch - module Session - class JsonSerializer - def self.load(value) - JSON.parse(value, quirks_mode: true) - end - - def self.dump(value) - JSON.generate(value, quirks_mode: true) - end - end - end -end diff --git a/actionpack/lib/action_dispatch/middleware/session/marshal_serializer.rb b/actionpack/lib/action_dispatch/middleware/session/marshal_serializer.rb deleted file mode 100644 index 26622f682d..0000000000 --- a/actionpack/lib/action_dispatch/middleware/session/marshal_serializer.rb +++ /dev/null @@ -1,14 +0,0 @@ -module ActionDispatch - module Session - class MarshalSerializer - def self.load(value) - Marshal.load(value) - end - - def self.dump(value) - Marshal.dump(value) - end - end - end -end - diff --git a/actionpack/test/controller/flash_hash_test.rb b/actionpack/test/controller/flash_hash_test.rb index 5490d9394b..50b36a0567 100644 --- a/actionpack/test/controller/flash_hash_test.rb +++ b/actionpack/test/controller/flash_hash_test.rb @@ -67,6 +67,16 @@ module ActionDispatch assert_equal({'flashes' => {'message' => 'Hello'}, 'discard' => %w[message]}, hash.to_session_value) end + def test_from_session_value_on_json_serializer + decrypted_data = "{ \"session_id\":\"d98bdf6d129618fc2548c354c161cfb5\", \"flash\":{\"discard\":[], \"flashes\":{\"message\":\"hey you\"}} }" + session = ActionDispatch::Cookies::JsonSerializer.load(decrypted_data) + hash = Flash::FlashHash.from_session_value(session['flash']) + + assert_equal({'discard' => %w[message], 'flashes' => { 'message' => 'hey you'}}, hash.to_session_value) + assert_equal "hey you", hash[:message] + assert_equal "hey you", hash["message"] + end + def test_empty? assert @hash.empty? @hash['zomg'] = 'bears' diff --git a/actionpack/test/controller/flash_test.rb b/actionpack/test/controller/flash_test.rb index 9ceab91e42..25a4857eba 100644 --- a/actionpack/test/controller/flash_test.rb +++ b/actionpack/test/controller/flash_test.rb @@ -175,13 +175,13 @@ class FlashTest < ActionController::TestCase assert_equal(:foo_indeed, flash.discard(:foo)) # valid key passed assert_nil flash.discard(:unknown) # non existent key passed - assert_equal({:foo => :foo_indeed, :bar => :bar_indeed}, flash.discard().to_hash) # nothing passed - assert_equal({:foo => :foo_indeed, :bar => :bar_indeed}, flash.discard(nil).to_hash) # nothing passed + assert_equal({"foo" => :foo_indeed, "bar" => :bar_indeed}, flash.discard().to_hash) # nothing passed + assert_equal({"foo" => :foo_indeed, "bar" => :bar_indeed}, flash.discard(nil).to_hash) # nothing passed assert_equal(:foo_indeed, flash.keep(:foo)) # valid key passed assert_nil flash.keep(:unknown) # non existent key passed - assert_equal({:foo => :foo_indeed, :bar => :bar_indeed}, flash.keep().to_hash) # nothing passed - assert_equal({:foo => :foo_indeed, :bar => :bar_indeed}, flash.keep(nil).to_hash) # nothing passed + assert_equal({"foo" => :foo_indeed, "bar" => :bar_indeed}, flash.keep().to_hash) # nothing passed + assert_equal({"foo" => :foo_indeed, "bar" => :bar_indeed}, flash.keep(nil).to_hash) # nothing passed end def test_redirect_to_with_alert diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index 84e4936f31..499c62cc35 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -671,6 +671,10 @@ class RespondToControllerTest < ActionController::TestCase end def test_variant_any_any + get :variant_any_any + assert_equal "text/html", @response.content_type + assert_equal "any", @response.body + @request.variant = :phone get :variant_any_any assert_equal "text/html", @response.content_type @@ -740,4 +744,25 @@ class RespondToControllerTest < ActionController::TestCase assert_equal "text/javascript", @response.content_type assert_equal "tablet", @response.body end + + def test_variant_negotiation_inline_syntax + @request.variant = [:tablet, :phone] + get :variant_inline_syntax_without_block + assert_equal "text/html", @response.content_type + assert_equal "phone", @response.body + end + + def test_variant_negotiation_block_syntax + @request.variant = [:tablet, :phone] + get :variant_plus_none_for_format + assert_equal "text/html", @response.content_type + assert_equal "phone", @response.body + end + + def test_variant_negotiation_without_block + @request.variant = [:tablet, :phone] + get :variant_inline_syntax_without_block + assert_equal "text/html", @response.content_type + assert_equal "phone", @response.body + end end diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 6101acdc25..ba7aaa338d 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -11,6 +11,16 @@ require 'active_support/key_generator' require 'active_support/message_verifier' class CookiesTest < ActionController::TestCase + class CustomSerializer + def self.load(value) + value.to_s + " and loaded" + end + + def self.dump(value) + value.to_s + " was dumped" + end + end + class TestController < ActionController::Base def authenticate cookies["user_name"] = "david" @@ -359,9 +369,72 @@ class CookiesTest < ActionController::TestCase assert_equal 'Jamie', @controller.send(:cookies).permanent[:user_name] end - def test_signed_cookie + def test_signed_cookie_using_default_serializer get :set_signed_cookie - assert_equal 45, @controller.send(:cookies).signed[:user_id] + cookies = @controller.send :cookies + assert_not_equal 45, cookies[:user_id] + assert_equal 45, cookies.signed[:user_id] + end + + def test_signed_cookie_using_marshal_serializer + @request.env["action_dispatch.cookies_serializer"] = :marshal + get :set_signed_cookie + cookies = @controller.send :cookies + assert_not_equal 45, cookies[:user_id] + assert_equal 45, cookies.signed[:user_id] + end + + def test_signed_cookie_using_json_serializer + @request.env["action_dispatch.cookies_serializer"] = :json + get :set_signed_cookie + cookies = @controller.send :cookies + assert_not_equal 45, cookies[:user_id] + assert_equal 45, cookies.signed[:user_id] + end + + def test_signed_cookie_using_custom_serializer + @request.env["action_dispatch.cookies_serializer"] = CustomSerializer + get :set_signed_cookie + assert_not_equal 45, cookies[:user_id] + assert_equal '45 was dumped and loaded', cookies.signed[:user_id] + end + + def test_signed_cookie_using_hybrid_serializer_can_migrate_marshal_dumped_value_to_json + @request.env["action_dispatch.cookies_serializer"] = :hybrid + + key_generator = @request.env["action_dispatch.key_generator"] + signed_cookie_salt = @request.env["action_dispatch.signed_cookie_salt"] + secret = key_generator.generate_key(signed_cookie_salt) + + marshal_value = ActiveSupport::MessageVerifier.new(secret, serializer: Marshal).generate(45) + @request.headers["Cookie"] = "user_id=#{marshal_value}" + + get :get_signed_cookie + + cookies = @controller.send :cookies + assert_not_equal 45, cookies[:user_id] + assert_equal 45, cookies.signed[:user_id] + + verifier = ActiveSupport::MessageVerifier.new(secret, serializer: JSON) + assert_equal 45, verifier.verify(@response.cookies['user_id']) + end + + def test_signed_cookie_using_hybrid_serializer_can_read_from_json_dumped_value + @request.env["action_dispatch.cookies_serializer"] = :hybrid + + key_generator = @request.env["action_dispatch.key_generator"] + signed_cookie_salt = @request.env["action_dispatch.signed_cookie_salt"] + secret = key_generator.generate_key(signed_cookie_salt) + json_value = ActiveSupport::MessageVerifier.new(secret, serializer: JSON).generate(45) + @request.headers["Cookie"] = "user_id=#{json_value}" + + get :get_signed_cookie + + cookies = @controller.send :cookies + assert_not_equal 45, cookies[:user_id] + assert_equal 45, cookies.signed[:user_id] + + assert_nil @response.cookies["user_id"] end def test_accessing_nonexistant_signed_cookie_should_not_raise_an_invalid_signature @@ -369,43 +442,87 @@ class CookiesTest < ActionController::TestCase assert_nil @controller.send(:cookies).signed[:non_existant_attribute] end - def test_encrypted_cookie + def test_encrypted_cookie_using_default_serializer get :set_encrypted_cookie cookies = @controller.send :cookies assert_not_equal 'bar', cookies[:foo] - assert_raises TypeError do + assert_raise TypeError do cookies.signed[:foo] end assert_equal 'bar', cookies.encrypted[:foo] end - class CustomJsonSerializer - def self.load(value) - JSON.load(value) + " and loaded" - end - - def self.dump(value) - JSON.dump(value + " was dumped") - end - end - - def test_encrypted_cookie_using_serializer_object - @request.env["action_dispatch.session_serializer"] = CustomJsonSerializer + def test_encrypted_cookie_using_marshal_serializer + @request.env["action_dispatch.cookies_serializer"] = :marshal get :set_encrypted_cookie - assert_equal 'bar was dumped and loaded', cookies.encrypted[:foo] + cookies = @controller.send :cookies + assert_not_equal 'bar', cookies[:foo] + assert_raises TypeError do + cookies.signed[:foo] + end + assert_equal 'bar', cookies.encrypted[:foo] end def test_encrypted_cookie_using_json_serializer - @request.env["action_dispatch.session_serializer"] = :json + @request.env["action_dispatch.cookies_serializer"] = :json get :set_encrypted_cookie cookies = @controller.send :cookies assert_not_equal 'bar', cookies[:foo] - assert_raises TypeError do + assert_raises ::JSON::ParserError do cookies.signed[:foo] end assert_equal 'bar', cookies.encrypted[:foo] end + def test_encrypted_cookie_using_custom_serializer + @request.env["action_dispatch.cookies_serializer"] = CustomSerializer + get :set_encrypted_cookie + assert_not_equal 'bar', cookies.encrypted[:foo] + assert_equal 'bar was dumped and loaded', cookies.encrypted[:foo] + end + + def test_encrypted_cookie_using_hybrid_serializer_can_migrate_marshal_dumped_value_to_json + @request.env["action_dispatch.cookies_serializer"] = :hybrid + + key_generator = @request.env["action_dispatch.key_generator"] + encrypted_cookie_salt = @request.env["action_dispatch.encrypted_cookie_salt"] + encrypted_signed_cookie_salt = @request.env["action_dispatch.encrypted_signed_cookie_salt"] + secret = key_generator.generate_key(encrypted_cookie_salt) + sign_secret = key_generator.generate_key(encrypted_signed_cookie_salt) + + marshal_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: Marshal).encrypt_and_sign("bar") + @request.headers["Cookie"] = "foo=#{marshal_value}" + + get :get_encrypted_cookie + + cookies = @controller.send :cookies + assert_not_equal "bar", cookies[:foo] + assert_equal "bar", cookies.encrypted[:foo] + + encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: JSON) + assert_equal "bar", encryptor.decrypt_and_verify(@response.cookies["foo"]) + end + + def test_encrypted_cookie_using_hybrid_serializer_can_read_from_json_dumped_value + @request.env["action_dispatch.cookies_serializer"] = :hybrid + + key_generator = @request.env["action_dispatch.key_generator"] + encrypted_cookie_salt = @request.env["action_dispatch.encrypted_cookie_salt"] + encrypted_signed_cookie_salt = @request.env["action_dispatch.encrypted_signed_cookie_salt"] + secret = key_generator.generate_key(encrypted_cookie_salt) + sign_secret = key_generator.generate_key(encrypted_signed_cookie_salt) + json_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: JSON).encrypt_and_sign("bar") + @request.headers["Cookie"] = "foo=#{json_value}" + + get :get_encrypted_cookie + + cookies = @controller.send :cookies + assert_not_equal "bar", cookies[:foo] + assert_equal "bar", cookies.encrypted[:foo] + + assert_nil @response.cookies["foo"] + end + def test_accessing_nonexistant_encrypted_cookie_should_not_raise_invalid_message get :set_encrypted_cookie assert_nil @controller.send(:cookies).encrypted[:non_existant_attribute] @@ -721,8 +838,6 @@ class CookiesTest < ActionController::TestCase assert_equal "dhh", cookies['user_name'] end - - def test_setting_request_cookies_is_indifferent_access cookies.clear cookies[:user_name] = "andrew" diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index f79fe47897..40e32cb4d3 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -846,8 +846,20 @@ class RequestTest < ActiveSupport::TestCase test "setting variant" do request = stub_request + request.variant = :mobile - assert_equal :mobile, request.variant + assert_equal [:mobile], request.variant + + request.variant = [:phone, :tablet] + assert_equal [:phone, :tablet], request.variant + + assert_raise ArgumentError do + request.variant = [:phone, "tablet"] + end + + assert_raise ArgumentError do + request.variant = "yolo" + end end test "setting variant with non symbol value" do diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 30dbc20f18..a0f298a6b1 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,17 @@ +* Date select helpers accept a format string for the months selector via the + new option `:month_format_string`. + + When rendered, the format string gets passed keys `:number` (integer), and + `:name` (string), in order to be able to interpolate them as in + + '%{name} (%<number>02d)' + + for example. + + This option is motivated by #13618. + + *Xavier Noria* + * Added `config.action_view.raise_on_missing_translations` to define whether an error should be raised for missing translations. diff --git a/actionview/lib/action_view/helpers/date_helper.rb b/actionview/lib/action_view/helpers/date_helper.rb index 3d091c4a00..698f0ca31c 100644 --- a/actionview/lib/action_view/helpers/date_helper.rb +++ b/actionview/lib/action_view/helpers/date_helper.rb @@ -169,6 +169,9 @@ module ActionView # "2 - February" instead of "February"). # * <tt>:use_month_names</tt> - Set to an array with 12 month names if you want to customize month names. # Note: You can also use Rails' i18n functionality for this. + # * <tt>:month_format_string</tt> - Set to a format string. The string gets passed keys +:number+ (integer) + # and +:name+ (string). A format string would be something like "%{name} (%<number>02d)" for example. + # See <tt>Kernel.sprintf</tt> for documentation on format sequences. # * <tt>:date_separator</tt> - Specifies a string to separate the date fields. Default is "" (i.e. nothing). # * <tt>:start_year</tt> - Set the start year for the year select. Default is <tt>Date.today.year - 5</tt>if # you are creating new record. While editing existing record, <tt>:start_year</tt> defaults to @@ -850,24 +853,36 @@ module ActionView I18n.translate(key, :locale => @options[:locale]) end - # Lookup month name for number. - # month_name(1) => "January" + # Looks up month names by number (1-based): # - # If <tt>:use_month_numbers</tt> option is passed - # month_name(1) => 1 + # month_name(1) # => "January" # - # If <tt>:use_two_month_numbers</tt> option is passed - # month_name(1) => '01' + # If the <tt>:use_month_numbers</tt> option is passed: # - # If <tt>:add_month_numbers</tt> option is passed - # month_name(1) => "1 - January" + # month_name(1) # => 1 + # + # If the <tt>:use_two_month_numbers</tt> option is passed: + # + # month_name(1) # => '01' + # + # If the <tt>:add_month_numbers</tt> option is passed: + # + # month_name(1) # => "1 - January" + # + # If the <tt>:month_format_string</tt> option is passed: + # + # month_name(1) # => "January (01)" + # + # depending on the format string. def month_name(number) if @options[:use_month_numbers] number elsif @options[:use_two_digit_numbers] - sprintf "%02d", number + '%02d' % number elsif @options[:add_month_numbers] "#{number} - #{month_names[number]}" + elsif format_string = @options[:month_format_string] + format_string % {number: number, name: month_names[number]} else month_names[number] end diff --git a/actionview/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb index 99b95fdfb7..7c17220d14 100644 --- a/actionview/lib/action_view/rendering.rb +++ b/actionview/lib/action_view/rendering.rb @@ -94,7 +94,7 @@ module ActionView variant = options[:variant] lookup_context.rendered_format = nil if options[:formats] - lookup_context.variants = [variant] if variant + lookup_context.variants = variant if variant view_renderer.render(view_context, options) end diff --git a/actionview/test/template/date_helper_test.rb b/actionview/test/template/date_helper_test.rb index 5f09aef249..6f77c3c99d 100644 --- a/actionview/test/template/date_helper_test.rb +++ b/actionview/test/template/date_helper_test.rb @@ -326,6 +326,16 @@ class DateHelperTest < ActionView::TestCase assert_dom_equal expected, select_month(8, :add_month_numbers => true) end + def test_select_month_with_format_string + expected = %(<select id="date_month" name="date[month]">\n) + expected << %(<option value="1">January (01)</option>\n<option value="2">February (02)</option>\n<option value="3">March (03)</option>\n<option value="4">April (04)</option>\n<option value="5">May (05)</option>\n<option value="6">June (06)</option>\n<option value="7">July (07)</option>\n<option value="8" selected="selected">August (08)</option>\n<option value="9">September (09)</option>\n<option value="10">October (10)</option>\n<option value="11">November (11)</option>\n<option value="12">December (12)</option>\n) + expected << "</select>\n" + + format_string = '%{name} (%<number>02d)' + assert_dom_equal expected, select_month(Time.mktime(2003, 8, 16), :month_format_string => format_string) + assert_dom_equal expected, select_month(8, :month_format_string => format_string) + end + def test_select_month_with_numbers_and_names_with_abbv expected = %(<select id="date_month" name="date[month]">\n) expected << %(<option value="1">1 - Jan</option>\n<option value="2">2 - Feb</option>\n<option value="3">3 - Mar</option>\n<option value="4">4 - Apr</option>\n<option value="5">5 - May</option>\n<option value="6">6 - Jun</option>\n<option value="7">7 - Jul</option>\n<option value="8" selected="selected">8 - Aug</option>\n<option value="9">9 - Sep</option>\n<option value="10">10 - Oct</option>\n<option value="11">11 - Nov</option>\n<option value="12">12 - Dec</option>\n) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 1fd9003009..458b9d77c2 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,28 @@ +* Perform necessary deeper encoding when hstore is inside an array. + + Fixes #11135. + + *Josh Goodall*, *Genadi Samokovarov* + +* Properly detect if a connection is still active before using it + in multi-threaded environments. + + Fixes #12867. + + *Kevin Casey*, *Matthew Draper*, *William (B.J.) Snow Orvis* + +* When inverting add_index use the index name if present instead of + the columns. + + If there are two indices with matching columns and one of them is + explicitly named then reverting the migration adding the named one + would instead drop the unnamed one. + + The inversion of add_index will now drop the index by its name if + it is present. + + *Hubert Dąbrowski* + * Add flag to disable schema dump after migration. Add a config parameter on Active Record named `dump_schema_after_migration` @@ -508,22 +533,6 @@ *Damien Mathieu* -* Improve the default select when `from` is used. - - Previously, if you did something like Topic.from(:temp_topics), it - would generate SQL like: - - SELECT topics.* FROM temp_topics; - - Which is will cause an error since there's not a topics table to select - from. - - Now the default if you use from is just `*`: - - SELECT * FROM temp_topics; - - *Cody Cutrer* - * Fix `PostgreSQL` insert to properly extract table name from multiline string SQL. Previously, executing an insert SQL in `PostgreSQL` with a command like this: @@ -738,7 +747,7 @@ * Raise `ActiveRecord::RecordNotDestroyed` when a replaced child marked with `dependent: destroy` fails to be destroyed. - Fixex #12812. + Fixes #12812. *Brian Thomas Storti* diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index b5e21cbede..142d21ce92 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -130,7 +130,6 @@ module ActiveRecord autoload :JoinDependency, 'active_record/associations/join_dependency' autoload :AssociationScope, 'active_record/associations/association_scope' autoload :AliasTracker, 'active_record/associations/alias_tracker' - autoload :JoinHelper, 'active_record/associations/join_helper' end # Clears out the association cache. diff --git a/activerecord/lib/active_record/associations/alias_tracker.rb b/activerecord/lib/active_record/associations/alias_tracker.rb index 0c23029981..85109aee6c 100644 --- a/activerecord/lib/active_record/associations/alias_tracker.rb +++ b/activerecord/lib/active_record/associations/alias_tracker.rb @@ -5,16 +5,48 @@ module ActiveRecord # Keeps track of table aliases for ActiveRecord::Associations::ClassMethods::JoinDependency and # ActiveRecord::Associations::ThroughAssociationScope class AliasTracker # :nodoc: - attr_reader :aliases, :table_joins, :connection + attr_reader :aliases, :connection + + def self.empty(connection) + new connection, Hash.new(0) + end + + def self.create(connection, table_joins) + if table_joins.empty? + empty connection + else + aliases = Hash.new { |h,k| + h[k] = initial_count_for(connection, k, table_joins) + } + new connection, aliases + end + end + + def self.initial_count_for(connection, name, table_joins) + # quoted_name should be downcased as some database adapters (Oracle) return quoted name in uppercase + quoted_name = connection.quote_table_name(name).downcase + + counts = table_joins.map do |join| + if join.is_a?(Arel::Nodes::StringJoin) + # Table names + table aliases + join.left.downcase.scan( + /join(?:\s+\w+)?\s+(\S+\s+)?#{quoted_name}\son/ + ).size + else + join.left.table_name == name ? 1 : 0 + end + end + + counts.sum + end # table_joins is an array of arel joins which might conflict with the aliases we assign here - def initialize(connection = Base.connection, table_joins = []) - @aliases = Hash.new { |h,k| h[k] = initial_count_for(k) } - @table_joins = table_joins - @connection = connection + def initialize(connection, aliases) + @aliases = aliases + @connection = connection end - def aliased_table_for(table_name, aliased_name = nil) + def aliased_table_for(table_name, aliased_name) table_alias = aliased_name_for(table_name, aliased_name) if table_alias == table_name @@ -24,9 +56,7 @@ module ActiveRecord end end - def aliased_name_for(table_name, aliased_name = nil) - aliased_name ||= table_name - + def aliased_name_for(table_name, aliased_name) if aliases[table_name].zero? # If it's zero, we can have our table_name aliases[table_name] = 1 @@ -48,26 +78,6 @@ module ActiveRecord private - def initial_count_for(name) - return 0 if Arel::Table === table_joins - - # quoted_name should be downcased as some database adapters (Oracle) return quoted name in uppercase - quoted_name = connection.quote_table_name(name).downcase - - counts = table_joins.map do |join| - if join.is_a?(Arel::Nodes::StringJoin) - # Table names + table aliases - join.left.downcase.scan( - /join(?:\s+\w+)?\s+(\S+\s+)?#{quoted_name}\son/ - ).size - else - join.left.table_name == name ? 1 : 0 - end - end - - counts.sum - end - def truncate(name) name.slice(0, connection.table_alias_length - 2) end diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 67ea489b22..4e46256862 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -94,7 +94,7 @@ module ActiveRecord # actually gets built. def association_scope if klass - @association_scope ||= AssociationScope.new(self).scope + @association_scope ||= AssociationScope.scope(self, klass.connection) end end diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 5a0ba9e6b1..27fd9e35db 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -1,54 +1,77 @@ -require 'active_record/associations/join_helper' - module ActiveRecord module Associations class AssociationScope #:nodoc: - include JoinHelper + INSTANCE = new - attr_reader :association, :alias_tracker + def self.scope(association, connection) + INSTANCE.scope association, connection + end - delegate :klass, :owner, :reflection, :interpolate, :to => :association - delegate :chain, :scope_chain, :options, :source_options, :active_record, :to => :reflection + def scope(association, connection) + klass = association.klass + reflection = association.reflection + scope = klass.unscoped + owner = association.owner + alias_tracker = AliasTracker.empty connection - def initialize(association) - @association = association - @alias_tracker = AliasTracker.new klass.connection + scope.extending! Array(reflection.options[:extend]) + add_constraints(scope, owner, klass, reflection, alias_tracker) end - def scope - scope = klass.unscoped - scope.extending! Array(options[:extend]) - add_constraints(scope) + def join_type + Arel::Nodes::InnerJoin end private - def column_for(table_name, column_name) + def construct_tables(chain, klass, refl, alias_tracker) + chain.map do |reflection| + alias_tracker.aliased_table_for( + table_name_for(reflection, klass, refl), + table_alias_for(reflection, refl, reflection != refl) + ) + end + end + + def table_alias_for(reflection, refl, join = false) + name = "#{reflection.plural_name}_#{alias_suffix(refl)}" + name << "_join" if join + name + end + + def join(table, constraint) + table.create_join(table, table.create_on(constraint), join_type) + end + + def column_for(table_name, column_name, alias_tracker) columns = alias_tracker.connection.schema_cache.columns_hash(table_name) columns[column_name] end - def bind_value(scope, column, value) + def bind_value(scope, column, value, alias_tracker) substitute = alias_tracker.connection.substitute_at( column, scope.bind_values.length) scope.bind_values += [[column, value]] substitute end - def bind(scope, table_name, column_name, value) - column = column_for table_name, column_name - bind_value scope, column, value + def bind(scope, table_name, column_name, value, tracker) + column = column_for table_name, column_name, tracker + bind_value scope, column, value, tracker end - def add_constraints(scope) - tables = construct_tables + def add_constraints(scope, owner, assoc_klass, refl, tracker) + chain = refl.chain + scope_chain = refl.scope_chain + + tables = construct_tables(chain, assoc_klass, refl, tracker) chain.each_with_index do |reflection, i| table, foreign_table = tables.shift, tables.first if reflection.source_macro == :belongs_to if reflection.options[:polymorphic] - key = reflection.association_primary_key(self.klass) + key = reflection.association_primary_key(assoc_klass) else key = reflection.association_primary_key end @@ -60,12 +83,12 @@ module ActiveRecord end if reflection == chain.last - bind_val = bind scope, table.table_name, key.to_s, owner[foreign_key] + bind_val = bind scope, table.table_name, key.to_s, owner[foreign_key], tracker scope = scope.where(table[key].eq(bind_val)) if reflection.type value = owner.class.base_class.name - bind_val = bind scope, table.table_name, reflection.type.to_s, value + bind_val = bind scope, table.table_name, reflection.type.to_s, value, tracker scope = scope.where(table[reflection.type].eq(bind_val)) end else @@ -73,7 +96,7 @@ module ActiveRecord if reflection.type value = chain[i + 1].klass.base_class.name - bind_val = bind scope, table.table_name, reflection.type.to_s, value + bind_val = bind scope, table.table_name, reflection.type.to_s, value, tracker scope = scope.where(table[reflection.type].eq(bind_val)) end @@ -81,14 +104,14 @@ module ActiveRecord end is_first_chain = i == 0 - klass = is_first_chain ? self.klass : reflection.klass + klass = is_first_chain ? assoc_klass : reflection.klass # Exclude the scope of the association itself, because that # was already merged in the #scope method. scope_chain[i].each do |scope_chain_item| - item = eval_scope(klass, scope_chain_item) + item = eval_scope(klass, scope_chain_item, owner) - if scope_chain_item == self.reflection.scope + if scope_chain_item == refl.scope scope.merge! item.except(:where, :includes, :bind) end @@ -104,22 +127,22 @@ module ActiveRecord scope end - def alias_suffix - reflection.name + def alias_suffix(refl) + refl.name end - def table_name_for(reflection) - if reflection == self.reflection + def table_name_for(reflection, klass, refl) + if reflection == refl # If this is a polymorphic belongs_to, we want to get the klass from the # association because it depends on the polymorphic_type attribute of # the owner klass.table_name else - super + reflection.table_name end end - def eval_scope(klass, scope) + def eval_scope(klass, scope, owner) if scope.is_a?(Relation) scope else diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 89b7945c78..03ca00fa70 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -24,6 +24,10 @@ module ActiveRecord # If you need to work on all current children, new and existing records, # +load_target+ and the +loaded+ flag are your friends. class CollectionAssociation < Association #:nodoc: + def initialize(owner, reflection) + super + @proxy = CollectionProxy.create(klass, self) + end # Implements the reader method, e.g. foo.items for Foo.has_many :items def reader(force_reload = false) @@ -33,7 +37,7 @@ module ActiveRecord reload end - @proxy ||= CollectionProxy.create(klass, self) + @proxy end # Implements the writer method, e.g. foo.items= for Foo.has_many :items diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index 27069157be..94f69d4c2d 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -93,8 +93,8 @@ module ActiveRecord # joins # => [] # def initialize(base, associations, joins) - @alias_tracker = AliasTracker.new(base.connection, joins) - @alias_tracker.aliased_name_for(base.table_name) # Updates the count for base.table_name to 1 + @alias_tracker = AliasTracker.create(base.connection, joins) + @alias_tracker.aliased_name_for(base.table_name, base.table_name) # Updates the count for base.table_name to 1 tree = self.class.make_tree associations @join_root = JoinBase.new base, build(tree, base) @join_root.children.each { |child| construct_tables! @join_root, child } diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index 0cd2e1a816..cee3c9999f 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -25,7 +25,8 @@ module ActiveRecord joins = [] tables = tables.reverse - scope_chain_iter = scope_chain.reverse_each + scope_chain_index = 0 + scope_chain = scope_chain.reverse # The chain starts with the target table, but we want to end with it here (makes # more sense in this context), so we reverse @@ -44,13 +45,14 @@ module ActiveRecord constraint = build_constraint(klass, table, key, foreign_table, foreign_key) - scope_chain_items = scope_chain_iter.next.map do |item| + scope_chain_items = scope_chain[scope_chain_index].map do |item| if item.is_a?(Relation) item else ActiveRecord::Relation.create(klass, table).instance_exec(node, &item) end end + scope_chain_index += 1 scope_chain_items.concat [klass.send(:build_default_scope)].compact diff --git a/activerecord/lib/active_record/associations/join_helper.rb b/activerecord/lib/active_record/associations/join_helper.rb deleted file mode 100644 index 3471936b9f..0000000000 --- a/activerecord/lib/active_record/associations/join_helper.rb +++ /dev/null @@ -1,36 +0,0 @@ -module ActiveRecord - module Associations - # Helper class module which gets mixed into JoinDependency::JoinAssociation and AssociationScope - module JoinHelper #:nodoc: - - def join_type - Arel::Nodes::InnerJoin - end - - private - - def construct_tables - chain.map do |reflection| - alias_tracker.aliased_table_for( - table_name_for(reflection), - table_alias_for(reflection, reflection != self.reflection) - ) - end - end - - def table_name_for(reflection) - reflection.table_name - end - - def table_alias_for(reflection, join = false) - name = "#{reflection.plural_name}_#{alias_suffix}" - name << "_join" if join - name - end - - def join(table, constraint) - table.create_join(table, table.create_on(constraint), join_type) - end - end - end -end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb b/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb index 35ce881302..3a3b500b1f 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb @@ -35,11 +35,11 @@ module ActiveRecord end end - def hstore_to_string(object) + def hstore_to_string(object, array_member = false) if Hash === object - object.map { |k,v| - "#{escape_hstore(k)}=>#{escape_hstore(v)}" - }.join ',' + string = object.map { |k, v| "#{escape_hstore(k)}=>#{escape_hstore(v)}" }.join(',') + string = escape_hstore(string) if array_member + string else object end @@ -49,10 +49,10 @@ module ActiveRecord if string.nil? nil elsif String === string - Hash[string.scan(HstorePair).map { |k,v| + Hash[string.scan(HstorePair).map { |k, v| v = v.upcase == 'NULL' ? nil : v.gsub(/\A"(.*)"\Z/m,'\1').gsub(/\\(.)/, '\1') k = k.gsub(/\A"(.*)"\Z/m,'\1').gsub(/\\(.)/, '\1') - [k,v] + [k, v] }] else string diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb index c1f978a081..210172cf32 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb @@ -121,7 +121,7 @@ module ActiveRecord end when Hash case column.sql_type - when 'hstore' then PostgreSQLColumn.hstore_to_string(value) + when 'hstore' then PostgreSQLColumn.hstore_to_string(value, array_member) when 'json' then PostgreSQLColumn.json_to_string(value) else super(value, column) end diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb index 9139ad953c..c44d8c1665 100644 --- a/activerecord/lib/active_record/migration/command_recorder.rb +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -140,7 +140,12 @@ module ActiveRecord def invert_add_index(args) table, columns, options = *args - [:remove_index, [table, (options || {}).merge(column: columns)]] + options ||= {} + + index_name = options[:name] + options_hash = index_name ? { name: index_name } : { column: columns } + + [:remove_index, [table, options_hash]] end def invert_remove_index(args) diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 9be734e88e..1d5c80bc01 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -75,7 +75,7 @@ db_namespace = namespace :db do # desc 'Runs the "down" for a given migration VERSION.' task :down => [:environment, :load_config] do version = ENV['VERSION'] ? ENV['VERSION'].to_i : nil - raise 'VERSION is required' unless version + raise 'VERSION is required - To go down one migration, run db:rollback' unless version ActiveRecord::Migrator.run(:down, ActiveRecord::Migrator.migrations_paths, version) db_namespace['_dump'].invoke end diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 01d46f7676..7099bdd285 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -347,7 +347,15 @@ module ActiveRecord end def construct_relation_for_association_calculations - apply_join_dependency(self, construct_join_dependency(arel.froms.first)) + from = arel.froms.first + if Arel::Table === from + apply_join_dependency(self, construct_join_dependency) + else + # FIXME: as far as I can tell, `from` will always be an Arel::Table. + # There are no tests that test this branch, but presumably it's + # possible for `from` to be a list? + apply_join_dependency(self, construct_join_dependency(from)) + end end def apply_join_dependency(relation, join_dependency) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 077f09b67d..5d38f0dce8 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -240,7 +240,7 @@ module ActiveRecord def select!(*fields) # :nodoc: fields.flatten! fields.map! do |field| - klass.attribute_alias?(field) ? klass.attribute_alias(field).to_sym : field + klass.attribute_alias?(field) ? klass.attribute_alias(field) : field end self.select_values += fields self @@ -995,8 +995,6 @@ module ActiveRecord columns_hash.key?(field.to_s) ? arel_table[field] : field end arel.project(*expanded_select) - elsif from_value - arel.project(Arel.star) else arel.project(@klass.arel_table[Arel.star]) end @@ -1052,11 +1050,11 @@ module ActiveRecord order_args.map! do |arg| case arg when Symbol - arg = klass.attribute_alias(arg).to_sym if klass.attribute_alias?(arg) + arg = klass.attribute_alias(arg) if klass.attribute_alias?(arg) table[arg].asc when Hash arg.map { |field, dir| - field = klass.attribute_alias(field).to_sym if klass.attribute_alias?(field) + field = klass.attribute_alias(field) if klass.attribute_alias?(field) table[field].send(dir) } else diff --git a/activerecord/test/cases/adapters/postgresql/hstore_test.rb b/activerecord/test/cases/adapters/postgresql/hstore_test.rb index d8782f5eaa..f2502430de 100644 --- a/activerecord/test/cases/adapters/postgresql/hstore_test.rb +++ b/activerecord/test/cases/adapters/postgresql/hstore_test.rb @@ -24,6 +24,7 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase @connection.transaction do @connection.create_table('hstores') do |t| t.hstore 'tags', :default => '' + t.hstore 'payload', array: true t.hstore 'settings' end end @@ -182,6 +183,30 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase assert_equal({'1' => '2'}, x.tags) end + def test_array_cycle + assert_array_cycle([{"AA" => "BB", "CC" => "DD"}, {"AA" => nil}]) + end + + def test_array_strings_with_quotes + assert_array_cycle([{'this has' => 'some "s that need to be escaped"'}]) + end + + def test_array_strings_with_commas + assert_array_cycle([{'this,has' => 'many,values'}]) + end + + def test_array_strings_with_array_delimiters + assert_array_cycle(['{' => '}']) + end + + def test_array_strings_with_null_strings + assert_array_cycle([{'NULL' => 'NULL'}]) + end + + def test_contains_nils + assert_array_cycle([{'NULL' => nil}]) + end + def test_select_multikey @connection.execute "insert into hstores (tags) VALUES ('1=>2,2=>3')" x = Hstore.first @@ -237,6 +262,20 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase private + def assert_array_cycle(array) + # test creation + x = Hstore.create!(payload: array) + x.reload + assert_equal(array, x.payload) + + # test updating + x = Hstore.create!(payload: []) + x.payload = array + x.save! + x.reload + assert_equal(array, x.payload) + end + def assert_cycle(hash) # test creation x = Hstore.create!(:tags => hash) diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index 131080913c..019406dd84 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -6,21 +6,21 @@ module ActiveRecord class PostgreSQLAdapterTest < ActiveRecord::TestCase def setup @connection = ActiveRecord::Base.connection - @connection.exec_query('drop table if exists ex') - @connection.exec_query('create table ex(id serial primary key, number integer, data character varying(255))') end def test_bad_connection assert_raise ActiveRecord::NoDatabaseError do configuration = ActiveRecord::Base.configurations['arunit'].merge(database: 'should_not_exist-cinco-dog-db') connection = ActiveRecord::Base.postgresql_connection(configuration) - connection.exec_query('drop table if exists ex') + connection.exec_query('SELECT 1') end end def test_valid_column - column = @connection.columns('ex').find { |col| col.name == 'id' } - assert @connection.valid_type?(column.type) + with_example_table do + column = @connection.columns('ex').find { |col| col.name == 'id' } + assert @connection.valid_type?(column.type) + end end def test_invalid_column @@ -28,7 +28,9 @@ module ActiveRecord end def test_primary_key - assert_equal 'id', @connection.primary_key('ex') + with_example_table do + assert_equal 'id', @connection.primary_key('ex') + end end def test_primary_key_works_tables_containing_capital_letters @@ -36,15 +38,15 @@ module ActiveRecord end def test_non_standard_primary_key - @connection.exec_query('drop table if exists ex') - @connection.exec_query('create table ex(data character varying(255) primary key)') - assert_equal 'data', @connection.primary_key('ex') + with_example_table 'data character varying(255) primary key' do + assert_equal 'data', @connection.primary_key('ex') + end end def test_primary_key_returns_nil_for_no_pk - @connection.exec_query('drop table if exists ex') - @connection.exec_query('create table ex(id integer)') - assert_nil @connection.primary_key('ex') + with_example_table 'id integer' do + assert_nil @connection.primary_key('ex') + end end def test_primary_key_raises_error_if_table_not_found @@ -54,32 +56,40 @@ module ActiveRecord end def test_insert_sql_with_proprietary_returning_clause - id = @connection.insert_sql("insert into ex (number) values(5150)", nil, "number") - assert_equal "5150", id + with_example_table do + id = @connection.insert_sql("insert into ex (number) values(5150)", nil, "number") + assert_equal "5150", id + end end def test_insert_sql_with_quoted_schema_and_table_name - id = @connection.insert_sql('insert into "public"."ex" (number) values(5150)') - expect = @connection.query('select max(id) from ex').first.first - assert_equal expect, id + with_example_table do + id = @connection.insert_sql('insert into "public"."ex" (number) values(5150)') + expect = @connection.query('select max(id) from ex').first.first + assert_equal expect, id + end end def test_insert_sql_with_no_space_after_table_name - id = @connection.insert_sql("insert into ex(number) values(5150)") - expect = @connection.query('select max(id) from ex').first.first - assert_equal expect, id + with_example_table do + id = @connection.insert_sql("insert into ex(number) values(5150)") + expect = @connection.query('select max(id) from ex').first.first + assert_equal expect, id + end end def test_multiline_insert_sql - id = @connection.insert_sql(<<-SQL) - insert into ex( - number) - values( - 5152 - ) - SQL - expect = @connection.query('select max(id) from ex').first.first - assert_equal expect, id + with_example_table do + id = @connection.insert_sql(<<-SQL) + insert into ex( + number) + values( + 5152 + ) + SQL + expect = @connection.query('select max(id) from ex').first.first + assert_equal expect, id + end end def test_insert_sql_with_returning_disabled @@ -135,29 +145,31 @@ module ActiveRecord end def test_pk_and_sequence_for - pk, seq = @connection.pk_and_sequence_for('ex') - assert_equal 'id', pk - assert_equal @connection.default_sequence_name('ex', 'id'), seq + with_example_table do + pk, seq = @connection.pk_and_sequence_for('ex') + assert_equal 'id', pk + assert_equal @connection.default_sequence_name('ex', 'id'), seq + end end def test_pk_and_sequence_for_with_non_standard_primary_key - @connection.exec_query('drop table if exists ex') - @connection.exec_query('create table ex(code serial primary key)') - pk, seq = @connection.pk_and_sequence_for('ex') - assert_equal 'code', pk - assert_equal @connection.default_sequence_name('ex', 'code'), seq + with_example_table 'code serial primary key' do + pk, seq = @connection.pk_and_sequence_for('ex') + assert_equal 'code', pk + assert_equal @connection.default_sequence_name('ex', 'code'), seq + end end def test_pk_and_sequence_for_returns_nil_if_no_seq - @connection.exec_query('drop table if exists ex') - @connection.exec_query('create table ex(id integer primary key)') - assert_nil @connection.pk_and_sequence_for('ex') + with_example_table 'id integer primary key' do + assert_nil @connection.pk_and_sequence_for('ex') + end end def test_pk_and_sequence_for_returns_nil_if_no_pk - @connection.exec_query('drop table if exists ex') - @connection.exec_query('create table ex(id integer)') - assert_nil @connection.pk_and_sequence_for('ex') + with_example_table 'id integer' do + assert_nil @connection.pk_and_sequence_for('ex') + end end def test_pk_and_sequence_for_returns_nil_if_table_not_found @@ -165,23 +177,27 @@ module ActiveRecord end def test_exec_insert_number - insert(@connection, 'number' => 10) + with_example_table do + insert(@connection, 'number' => 10) - result = @connection.exec_query('SELECT number FROM ex WHERE number = 10') + result = @connection.exec_query('SELECT number FROM ex WHERE number = 10') - assert_equal 1, result.rows.length - assert_equal "10", result.rows.last.last + assert_equal 1, result.rows.length + assert_equal "10", result.rows.last.last + end end def test_exec_insert_string - str = 'いただきます！' - insert(@connection, 'number' => 10, 'data' => str) + with_example_table do + str = 'いただきます！' + insert(@connection, 'number' => 10, 'data' => str) - result = @connection.exec_query('SELECT number, data FROM ex WHERE number = 10') + result = @connection.exec_query('SELECT number, data FROM ex WHERE number = 10') - value = result.rows.last.last + value = result.rows.last.last - assert_equal str, value + assert_equal str, value + end end def test_table_alias_length @@ -191,44 +207,50 @@ module ActiveRecord end def test_exec_no_binds - result = @connection.exec_query('SELECT id, data FROM ex') - assert_equal 0, result.rows.length - assert_equal 2, result.columns.length - assert_equal %w{ id data }, result.columns - - string = @connection.quote('foo') - @connection.exec_query("INSERT INTO ex (id, data) VALUES (1, #{string})") - result = @connection.exec_query('SELECT id, data FROM ex') - assert_equal 1, result.rows.length - assert_equal 2, result.columns.length - - assert_equal [['1', 'foo']], result.rows + with_example_table do + result = @connection.exec_query('SELECT id, data FROM ex') + assert_equal 0, result.rows.length + assert_equal 2, result.columns.length + assert_equal %w{ id data }, result.columns + + string = @connection.quote('foo') + @connection.exec_query("INSERT INTO ex (id, data) VALUES (1, #{string})") + result = @connection.exec_query('SELECT id, data FROM ex') + assert_equal 1, result.rows.length + assert_equal 2, result.columns.length + + assert_equal [['1', 'foo']], result.rows + end end def test_exec_with_binds - string = @connection.quote('foo') - @connection.exec_query("INSERT INTO ex (id, data) VALUES (1, #{string})") - result = @connection.exec_query( - 'SELECT id, data FROM ex WHERE id = $1', nil, [[nil, 1]]) + with_example_table do + string = @connection.quote('foo') + @connection.exec_query("INSERT INTO ex (id, data) VALUES (1, #{string})") + result = @connection.exec_query( + 'SELECT id, data FROM ex WHERE id = $1', nil, [[nil, 1]]) - assert_equal 1, result.rows.length - assert_equal 2, result.columns.length + assert_equal 1, result.rows.length + assert_equal 2, result.columns.length - assert_equal [['1', 'foo']], result.rows + assert_equal [['1', 'foo']], result.rows + end end def test_exec_typecasts_bind_vals - string = @connection.quote('foo') - @connection.exec_query("INSERT INTO ex (id, data) VALUES (1, #{string})") + with_example_table do + string = @connection.quote('foo') + @connection.exec_query("INSERT INTO ex (id, data) VALUES (1, #{string})") - column = @connection.columns('ex').find { |col| col.name == 'id' } - result = @connection.exec_query( - 'SELECT id, data FROM ex WHERE id = $1', nil, [[column, '1-fuu']]) + column = @connection.columns('ex').find { |col| col.name == 'id' } + result = @connection.exec_query( + 'SELECT id, data FROM ex WHERE id = $1', nil, [[column, '1-fuu']]) - assert_equal 1, result.rows.length - assert_equal 2, result.columns.length + assert_equal 1, result.rows.length + assert_equal 2, result.columns.length - assert_equal [['1', 'foo']], result.rows + assert_equal [['1', 'foo']], result.rows + end end def test_substitute_at @@ -240,9 +262,11 @@ module ActiveRecord end def test_partial_index - @connection.add_index 'ex', %w{ id number }, :name => 'partial', :where => "number > 100" - index = @connection.indexes('ex').find { |idx| idx.name == 'partial' } - assert_equal "(number > 100)", index.where + with_example_table do + @connection.add_index 'ex', %w{ id number }, :name => 'partial', :where => "number > 100" + index = @connection.indexes('ex').find { |idx| idx.name == 'partial' } + assert_equal "(number > 100)", index.where + end end def test_columns_for_distinct_zero_orders @@ -300,6 +324,14 @@ module ActiveRecord ctx.exec_insert(sql, 'SQL', binds) end + def with_example_table(definition = nil) + definition ||= 'id serial primary key, number integer, data character varying(255)' + @connection.exec_query("create table ex(#{definition})") + yield + ensure + @connection.exec_query('drop table if exists ex') + end + def connection_without_insert_returning ActiveRecord::Base.postgresql_connection(ActiveRecord::Base.configurations['arunit'].merge(:insert_returning => false)) end diff --git a/activerecord/test/cases/adapters/sqlite3/copy_table_test.rb b/activerecord/test/cases/adapters/sqlite3/copy_table_test.rb index e78cb88562..b478db749d 100644 --- a/activerecord/test/cases/adapters/sqlite3/copy_table_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/copy_table_test.rb @@ -1,7 +1,7 @@ require "cases/helper" class CopyTableTest < ActiveRecord::TestCase - fixtures :customers, :companies, :comments, :binaries + fixtures :customers def setup @connection = ActiveRecord::Base.connection diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index 02834edf7b..73cb739b2b 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -33,9 +33,8 @@ module ActiveRecord end end - def test_connect_with_url - skip "can't establish new connection when using memory db" if in_memory_db? - begin + unless in_memory_db? + def test_connect_with_url original_connection = ActiveRecord::Base.remove_connection tf = Tempfile.open 'whatever' url = "sqlite3://#{tf.path}" @@ -46,11 +45,8 @@ module ActiveRecord tf.unlink ActiveRecord::Base.establish_connection(original_connection) end - end - def test_connect_memory_with_url - skip "can't establish new connection when using memory db" if in_memory_db? - begin + def test_connect_memory_with_url original_connection = ActiveRecord::Base.remove_connection url = "sqlite3:///:memory:" ActiveRecord::Base.establish_connection(url) diff --git a/activerecord/test/cases/associations/association_scope_test.rb b/activerecord/test/cases/associations/association_scope_test.rb index d38648202e..c78b036f53 100644 --- a/activerecord/test/cases/associations/association_scope_test.rb +++ b/activerecord/test/cases/associations/association_scope_test.rb @@ -6,8 +6,9 @@ module ActiveRecord module Associations class AssociationScopeTest < ActiveRecord::TestCase test 'does not duplicate conditions' do - association_scope = AssociationScope.new(Author.new.association(:welcome_posts)) - wheres = association_scope.scope.where_values.map(&:right) + scope = AssociationScope.scope(Author.new.association(:welcome_posts), + Author.connection) + wheres = scope.where_values.map(&:right) assert_equal wheres.uniq, wheres end end diff --git a/activerecord/test/cases/associations/eager_singularization_test.rb b/activerecord/test/cases/associations/eager_singularization_test.rb index 669569a774..b12bc355e8 100644 --- a/activerecord/test/cases/associations/eager_singularization_test.rb +++ b/activerecord/test/cases/associations/eager_singularization_test.rb @@ -1,6 +1,7 @@ require "cases/helper" +if ActiveRecord::Base.connection.supports_migrations? class EagerSingularizationTest < ActiveRecord::TestCase class Virus < ActiveRecord::Base belongs_to :octopus @@ -50,8 +51,6 @@ class EagerSingularizationTest < ActiveRecord::TestCase end def setup - skip 'Does not support migrations' unless connection.supports_migrations? - connection.create_table :viri do |t| t.column :octopus_id, :integer t.column :species, :string @@ -146,3 +145,4 @@ class EagerSingularizationTest < ActiveRecord::TestCase end end end +end diff --git a/activerecord/test/cases/invertible_migration_test.rb b/activerecord/test/cases/invertible_migration_test.rb index 428145d00b..debacf815c 100644 --- a/activerecord/test/cases/invertible_migration_test.rb +++ b/activerecord/test/cases/invertible_migration_test.rb @@ -106,6 +106,22 @@ module ActiveRecord end end + class RevertNamedIndexMigration1 < SilentMigration + def change + create_table("horses") do |t| + t.column :content, :string + t.column :remind_at, :datetime + end + add_index :horses, :content + end + end + + class RevertNamedIndexMigration2 < SilentMigration + def change + add_index :horses, :content, name: "horses_index_named" + end + end + def teardown %w[horses new_horses].each do |table| if ActiveRecord::Base.connection.table_exists?(table) @@ -255,5 +271,17 @@ module ActiveRecord ActiveRecord::Base.table_name_prefix = ActiveRecord::Base.table_name_suffix = '' end + def test_migrate_revert_add_index_with_name + RevertNamedIndexMigration1.new.migrate(:up) + RevertNamedIndexMigration2.new.migrate(:up) + RevertNamedIndexMigration2.new.migrate(:down) + + connection = ActiveRecord::Base.connection + assert connection.index_exists?(:horses, :content), + "index on content should exist" + assert !connection.index_exists?(:horses, :content, name: "horses_index_named"), + "horses_index_named index should not exist" + end + end end diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb index 35b656ee43..a925cf4c05 100644 --- a/activerecord/test/cases/migration/command_recorder_test.rb +++ b/activerecord/test/cases/migration/command_recorder_test.rb @@ -174,13 +174,13 @@ module ActiveRecord end def test_invert_add_index - remove = @recorder.inverse_of :add_index, [:table, [:one, :two], options: true] - assert_equal [:remove_index, [:table, {column: [:one, :two], options: true}]], remove + remove = @recorder.inverse_of :add_index, [:table, [:one, :two]] + assert_equal [:remove_index, [:table, {column: [:one, :two]}]], remove end def test_invert_add_index_with_name remove = @recorder.inverse_of :add_index, [:table, [:one, :two], name: "new_index"] - assert_equal [:remove_index, [:table, {column: [:one, :two], name: "new_index"}]], remove + assert_equal [:remove_index, [:table, {name: "new_index"}]], remove end def test_invert_add_index_with_no_options diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index e390d37871..8718110c36 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -151,11 +151,14 @@ class RelationTest < ActiveRecord::TestCase assert_equal relation.to_a, Comment.select('a.*').from(relation, :a).to_a end - def test_finding_with_subquery_without_select - relation = Topic.where(:approved => true) - assert_equal relation.to_a, Topic.from(relation).to_a + def test_finding_with_subquery_without_select_does_not_change_the_select + relation = Topic.where(approved: true) + assert_raises(ActiveRecord::StatementInvalid) do + Topic.from(relation).to_a + end end + def test_finding_with_conditions assert_equal ["David"], Author.where(:name => 'David').map(&:name) assert_equal ['Mary'], Author.where(["name = ?", 'Mary']).map(&:name) diff --git a/guides/source/4_1_release_notes.md b/guides/source/4_1_release_notes.md index 90e6b2fcbc..8fcfc71351 100644 --- a/guides/source/4_1_release_notes.md +++ b/guides/source/4_1_release_notes.md @@ -346,10 +346,8 @@ for detailed changes. params "deep munging" that was used to address security vulnerability CVE-2013-0155. ([Pull Request](https://github.com/rails/rails/pull/13188)) -* Added `:serializer` option for `config.session_store :cookie_store`. This - changes default serializer when using - `:cookie_store`. ([Pull Request](https://github.com/rails/rails/pull/13692)) - +* New config option `config.action_dispatch.cookies_serializer` for specifying + a serializer for the signed and encrypted cookie jars. (Pull Requests [1](https://github.com/rails/rails/pull/13692), [2](https://github.com/rails/rails/pull/13945) / [More Details](upgrading_ruby_on_rails.html#cookies-serializer)) Action Mailer ------------- diff --git a/guides/source/action_controller_overview.md b/guides/source/action_controller_overview.md index 9eaf03dd82..222d86afe9 100644 --- a/guides/source/action_controller_overview.md +++ b/guides/source/action_controller_overview.md @@ -381,22 +381,6 @@ You can also pass a `:domain` key and specify the domain name for the cookie: YourApp::Application.config.session_store :cookie_store, key: '_your_app_session', domain: ".example.com" ``` -You can pass `:serializer` key to specify serializer for serializing session: - -```ruby -YourApp::Application.config.session_store :cookie_store, key: '_your_app_session', serializer: :json -``` - -The default serializer for new application is `:json`. For compatibility with -old applications `:marshal` is used when `serializer` option is not specified. - -It is also possible to pass a custom serializer class with `load` and `dump` -public methods defined: - -```ruby -YourApp::Application.config.session_store :cookie_store, key: '_your_app_session', serializer: MyCustomSerializer -``` - Rails sets up (for the CookieStore) a secret key used for signing the session data. This can be changed in `config/initializers/secret_token.rb` ```ruby @@ -588,6 +572,38 @@ end Note that while for session values you set the key to `nil`, to delete a cookie value you should use `cookies.delete(:key)`. +Rails also provides a signed cookie jar and an encrypted cookie jar for storing +sensitive data. The signed cookie jar appends a cryptographic signature on the +cookie values to protect their integrity. The encrypted cookie jar encrypts the +values in addition to signing them, so that they cannot be read by the end user. +Refer to the [API documentation](http://api.rubyonrails.org/classes/ActionDispatch/Cookies.html) +for more details. + +These special cookie jars use a serializer to serialize the assigned values into +strings and deserializes them into Ruby objects on read. + +You can specify what serializer to use: + +```ruby +Rails.application.config.action_dispatch.cookies_serializer = :json +``` + +The default serializer for new applications is `:json`. For compatibility with +old applications with existing cookies, `:marshal` is used when `serializer` +option is not specified. + +You may also set this option to `:hybrid`, in which case Rails would transparently +deserialize existing (`Marshal`-serialized) cookies on read and re-write them in +the `JSON` format. This is useful for migrating existing applications to the +`:json` serializer. + +It is also possible to pass a custom serializer that responds to `load` and +`dump`: + +```ruby +Rails.application.config.action_dispatch.cookies_serializer = MyCustomSerializer +``` + Rendering XML and JSON data --------------------------- diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md index 59dfefd22f..2ad09f599b 100644 --- a/guides/source/active_support_core_extensions.md +++ b/guides/source/active_support_core_extensions.md @@ -1403,6 +1403,8 @@ The third argument, `indent_empty_lines`, is a flag that says whether empty line The `indent!` method performs indentation in-place. +NOTE: Defined in `active_support/core_ext/string/indent.rb`. + ### Access #### `at(position)` diff --git a/guides/source/asset_pipeline.md b/guides/source/asset_pipeline.md index 9ae3fbb0b5..fa2e57ff92 100644 --- a/guides/source/asset_pipeline.md +++ b/guides/source/asset_pipeline.md @@ -1018,7 +1018,8 @@ The X-Sendfile header is a directive to the web server to ignore the response from the application, and instead serve a specified file from disk. This option is off by default, but can be enabled if your server supports it. When enabled, this passes responsibility for serving the file to the web server, which is -faster. +faster. Have a look at [send_file](http://api.rubyonrails.org/classes/ActionController/DataStreaming.html#method-i-send_file) +on how to use this feature. Apache and nginx support this option, which can be enabled in `config/environments/production.rb`: @@ -1033,6 +1034,10 @@ option, take care to paste this configuration option only into `production.rb` and any other environments you define with production behavior (not `application.rb`). +TIP: For further details have a look at the docs of your production web server: +- [Apache](https://tn123.org/mod_xsendfile/) +- [Nginx](http://wiki.nginx.org/XSendfile) + Assets Cache Store ------------------ diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index 5264f82b4b..53d2a9b55b 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -1119,7 +1119,11 @@ The `method: :patch` option tells Rails that we want this form to be submitted via the `PATCH` HTTP method which is the HTTP method you're expected to use to **update** resources according to the REST protocol. -TIP: By default forms built with the _form_for_ helper are sent via `POST`. +The first parameter of the `form_tag` can be an object, say, `@article` which would +cause the helper to fill in the form with the fields of the object. Passing in a +symbol (`:article`) with the same name as the instance variable (`@article`) also +automagically leads to the same behavior. This is what is happening here. More details +can be found in [form_for documentation](http://api.rubyonrails.org/classes/ActionView/Helpers/FormHelper.html#method-i-form_for). Next we need to create the `update` action in `app/controllers/articles_controller.rb`: @@ -1390,7 +1394,9 @@ class CreateComments < ActiveRecord::Migration create_table :comments do |t| t.string :commenter t.text :body - t.references :article, index: true + + # this line adds an integer column called `article_id`. + t.references :article, index: true t.timestamps end diff --git a/guides/source/security.md b/guides/source/security.md index 70fb066b64..ece431dae7 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -549,7 +549,7 @@ Injection is very tricky, because the same code or parameter can be malicious in ### Whitelists versus Blacklists -NOTE: _When sanitizing, protecting or verifying something, whitelists over blacklists._ +NOTE: _When sanitizing, protecting or verifying something, prefer whitelists over blacklists._ A blacklist can be a list of bad e-mail addresses, non-public actions or bad HTML tags. This is opposed to a whitelist which lists the good e-mail addresses, public actions, good HTML tags and so on. Although sometimes it is not possible to create a whitelist (in a SPAM filter, for example), _prefer to use whitelist approaches_: diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index 2055452935..8aae3bbc1a 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -98,6 +98,19 @@ If your test helper contains a call to is now done automatically when you `require 'test_help'`, although leaving this line in your helper is not harmful in any way. +### Cookies serializer + +Applications created before Rails 4.1 uses `Marshal` to serialize cookie values into +the signed and encrypted cookie jars. If you want to use the new `JSON`-based format +in your application, you can add an initializer file with the following content: + + ```ruby + Rails.application.config.cookies_serializer :hybrid + ``` + +This would transparently migrate your existing `Marshal`-serialized cookies into the +new `JSON`-based format. + ### Changes in JSON handling There are a few major changes related to JSON handling in Rails 4.1. diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index a57d56f4aa..bade9ef543 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,7 @@ +* Do not crash when `config/secrets.yml` is empty. + + *Yves Senn* + * Set `dump_schema_after_migration` config values in production. Set `config.active_record.dump_schema_after_migration` as false diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 36432e56ba..e37347b576 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -206,7 +206,7 @@ module Rails "action_dispatch.signed_cookie_salt" => config.action_dispatch.signed_cookie_salt, "action_dispatch.encrypted_cookie_salt" => config.action_dispatch.encrypted_cookie_salt, "action_dispatch.encrypted_signed_cookie_salt" => config.action_dispatch.encrypted_signed_cookie_salt, - "action_dispatch.session_serializer" => config.session_options[:serializer] + "action_dispatch.cookies_serializer" => config.action_dispatch.cookies_serializer }) end end @@ -308,7 +308,8 @@ module Rails yaml = config.paths["config/secrets"].first if File.exist?(yaml) require "erb" - env_secrets = YAML.load(ERB.new(IO.read(yaml)).result)[Rails.env] + all_secrets = YAML.load(ERB.new(IO.read(yaml)).result) || {} + env_secrets = all_secrets[Rails.env] secrets.merge!(env_secrets.symbolize_keys) if env_secrets end diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 815894144a..f1f79d8378 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -14,7 +14,6 @@ module Rails DATABASES.concat(JDBC_DATABASES) attr_accessor :rails_template - attr_accessor :app_template add_shebang_option! argument :app_path, type: :string @@ -27,9 +26,6 @@ module Rails class_option :template, type: :string, aliases: '-m', desc: "Path to some #{name} template (can be a filesystem path or URL)" - class_option :app_template, type: :string, aliases: '-n', - desc: "Path to some #{name} template (can be a filesystem path or URL)" - class_option :skip_gemfile, type: :boolean, default: false, desc: "Don't create a Gemfile" @@ -126,10 +122,6 @@ module Rails }.curry[@gem_filter] end - def remove_gem(name) - add_gem_entry_filter { |gem| gem.name != name } - end - def builder @builder ||= begin builder_class = get_builder_class @@ -149,92 +141,21 @@ module Rails FileUtils.cd(destination_root) unless options[:pretend] end - class TemplateRecorder < ::BasicObject # :nodoc: - attr_reader :gems - - def initialize(target) - @target = target - # unfortunately, instance eval has access to these ivars - @app_const = target.send :app_const if target.respond_to?(:app_const, true) - @app_const_base = target.send :app_const_base if target.respond_to?(:app_const_base, true) - @app_name = target.send :app_name if target.respond_to?(:app_name, true) - @commands = [] - @gems = [] - end - - def gemfile_entry(*args) - @target.send :gemfile_entry, *args - end - - def add_gem_entry_filter(*args, &block) - @target.send :add_gem_entry_filter, *args, &block - end - - def remove_gem(*args, &block) - @target.send :remove_gem, *args, &block - end - - def method_missing(name, *args, &block) - @commands << [name, args, block] - end - - def respond_to_missing?(method, priv = false) - super || @target.respond_to?(method, priv) - end - - def replay! - @commands.each do |name, args, block| - @target.send name, *args, &block - end - end - end - def apply_rails_template - @recorder = TemplateRecorder.new self - - apply(rails_template, target: self) if rails_template - apply(app_template, target: @recorder) if app_template + apply rails_template if rails_template rescue Thor::Error, LoadError, Errno::ENOENT => e raise Error, "The template [#{rails_template}] could not be loaded. Error: #{e}" end - def replay_template - @recorder.replay! if @recorder - end - - def apply(path, config={}) - verbose = config.fetch(:verbose, true) - target = config.fetch(:target, self) - is_uri = path =~ /^https?\:\/\// - path = find_in_source_paths(path) unless is_uri - - say_status :apply, path, verbose - shell.padding += 1 if verbose - - if is_uri - contents = open(path, "Accept" => "application/x-thor-template") {|io| io.read } - else - contents = open(path) {|io| io.read } - end - - target.instance_eval(contents, path) - shell.padding -= 1 if verbose - end - def set_default_accessors! self.destination_root = File.expand_path(app_path, destination_root) - self.rails_template = expand_template options[:template] - self.app_template = expand_template options[:app_template] - end - - def expand_template(name) - case name - when /^https?:\/\// - name - when String - File.expand_path(name, Dir.pwd) - else - name + self.rails_template = case options[:template] + when /^https?:\/\// + options[:template] + when String + File.expand_path(options[:template], Dir.pwd) + else + options[:template] end end diff --git a/railties/lib/rails/generators/erb/mailer/templates/view.html.erb b/railties/lib/rails/generators/erb/mailer/templates/view.html.erb index 8bb7c2b768..b5045671b3 100644 --- a/railties/lib/rails/generators/erb/mailer/templates/view.html.erb +++ b/railties/lib/rails/generators/erb/mailer/templates/view.html.erb @@ -1,5 +1,5 @@ <h1><%= class_name %>#<%= @action %></h1> <p> - <%%= @greeting %>, find me in app/views/<%= @path %> + <%%= @greeting %>, find me in <%= @path %> </p> diff --git a/railties/lib/rails/generators/erb/mailer/templates/view.text.erb b/railties/lib/rails/generators/erb/mailer/templates/view.text.erb index 6d597256a6..342285df19 100644 --- a/railties/lib/rails/generators/erb/mailer/templates/view.text.erb +++ b/railties/lib/rails/generators/erb/mailer/templates/view.text.erb @@ -1,3 +1,3 @@ <%= class_name %>#<%= @action %> -<%%= @greeting %>, find me in app/views/<%= @path %> +<%%= @greeting %>, find me in <%= @path %> diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index d2eca5b2fb..83cb1dc0d5 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -166,7 +166,6 @@ module Rails end public_task :set_default_accessors! - public_task :apply_rails_template public_task :create_root def create_root_files @@ -236,8 +235,7 @@ module Rails build(:leftovers) end - public_task :run_bundle - public_task :replay_template + public_task :apply_rails_template, :run_bundle public_task :generate_spring_binstubs protected diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/cookies_serializer.rb b/railties/lib/rails/generators/rails/app/templates/config/initializers/cookies_serializer.rb new file mode 100644 index 0000000000..7a06a89f0f --- /dev/null +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/cookies_serializer.rb @@ -0,0 +1,3 @@ +# Be sure to restart your server when you modify this file. + +Rails.application.config.action_dispatch.cookies_serializer = :json
\ No newline at end of file diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/session_store.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/session_store.rb.tt index 097fcb4bb0..2bb9b82c61 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/session_store.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/session_store.rb.tt @@ -1,3 +1,3 @@ # Be sure to restart your server when you modify this file. -Rails.application.config.session_store :cookie_store, key: <%= "'_#{app_name}_session'" %>, serializer: :json +Rails.application.config.session_store :cookie_store, key: <%= "'_#{app_name}_session'" %> diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index dbe1e37d8e..f6f529b80a 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -185,7 +185,6 @@ task default: :test end public_task :set_default_accessors! - public_task :apply_rails_template public_task :create_root def create_root_files @@ -242,6 +241,7 @@ task default: :test build(:leftovers) end + public_task :apply_rails_template, :run_bundle def name @name ||= begin @@ -255,9 +255,6 @@ task default: :test end end - public_task :run_bundle - public_task :replay_template - protected def app_templates_dir diff --git a/railties/lib/rails/tasks/framework.rake b/railties/lib/rails/tasks/framework.rake index 94e8f83e86..e669315934 100644 --- a/railties/lib/rails/tasks/framework.rake +++ b/railties/lib/rails/tasks/framework.rake @@ -10,7 +10,7 @@ namespace :rails do require 'rails/generators' require 'rails/generators/rails/app/app_generator' generator = Rails::Generators::AppGenerator.new [Rails.root], {}, destination_root: Rails.root - generator.send :apply, template, verbose: false + generator.apply template, verbose: false end namespace :templates do diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index b814479540..b2d0e7e202 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -336,6 +336,14 @@ module ApplicationTests assert_equal 'myamazonsecretaccesskey', app.secrets.aws_secret_access_key end + test "blank config/secrets.yml does not crash the loading process" do + app_file 'config/secrets.yml', <<-YAML + YAML + require "#{app_path}/config/environment" + + assert_nil app.secrets.not_defined + end + test "protect from forgery is the default in a new app" do make_basic_app diff --git a/railties/test/application/rake/templates_test.rb b/railties/test/application/rake/templates_test.rb deleted file mode 100644 index 1fca80debd..0000000000 --- a/railties/test/application/rake/templates_test.rb +++ /dev/null @@ -1,32 +0,0 @@ -require "isolation/abstract_unit" - -module ApplicationTests - module RakeTests - class TemplatesTest < ActiveSupport::TestCase - include ActiveSupport::Testing::Isolation - - def setup - build_app - require "rails/all" - super - end - - def teardown - super - teardown_app - end - - def test_rake_template - Dir.chdir(app_path) do - cmd = "bundle exec rake rails:template LOCATION=foo" - r,w = IO.pipe - Process.waitpid Process.spawn(cmd, out: w, err: w) - w.close - assert_match(/Could not find.*foo/, r.read) - r.close - end - end - end - end -end - diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 700935fd8d..5811379e35 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -163,73 +163,6 @@ class AppGeneratorTest < Rails::Generators::TestCase end end - def test_arbitrary_code - output = Tempfile.open('my_template') do |template| - template.puts 'puts "You are using Rails version #{Rails::VERSION::STRING}."' - template.close - run_generator([destination_root, "-m", template.path]) - end - assert_match 'You are using', output - end - - def test_add_gemfile_entry - Tempfile.open('my_template') do |template| - template.puts 'gemfile_entry "tenderlove"' - template.flush - template.close - run_generator([destination_root, "-n", template.path]) - assert_file "Gemfile", /tenderlove/ - end - end - - def test_add_skip_entry - Tempfile.open 'my_template' do |template| - template.puts 'add_gem_entry_filter { |gem| gem.name != "jbuilder" }' - template.close - - run_generator([destination_root, "-n", template.path]) - assert_file "Gemfile" do |contents| - assert_no_match 'jbuilder', contents - end - end - end - - def test_remove_gem - Tempfile.open 'my_template' do |template| - template.puts 'remove_gem "jbuilder"' - template.close - - run_generator([destination_root, "-n", template.path]) - assert_file "Gemfile" do |contents| - assert_no_match 'jbuilder', contents - end - end - end - - def test_skip_turbolinks_when_it_is_not_on_gemfile - Tempfile.open 'my_template' do |template| - template.puts 'add_gem_entry_filter { |gem| gem.name != "turbolinks" }' - template.flush - - run_generator([destination_root, "-n", template.path]) - assert_file "Gemfile" do |contents| - assert_no_match 'turbolinks', contents - end - - assert_file "app/views/layouts/application.html.erb" do |contents| - assert_no_match 'turbolinks', contents - end - - assert_file "app/views/layouts/application.html.erb" do |contents| - assert_no_match('data-turbolinks-track', contents) - end - - assert_file "app/assets/javascripts/application.js" do |contents| - assert_no_match 'turbolinks', contents - end - end - end - def test_config_another_database run_generator([destination_root, "-d", "mysql"]) assert_file "config/database.yml", /mysql/ @@ -433,7 +366,7 @@ class AppGeneratorTest < Rails::Generators::TestCase def test_new_hash_style run_generator [destination_root] assert_file "config/initializers/session_store.rb" do |file| - assert_match(/config.session_store :cookie_store, key: '_.+_session', serializer: :json/, file) + assert_match(/config.session_store :cookie_store, key: '_.+_session'/, file) end end diff --git a/railties/test/generators/generator_test.rb b/railties/test/generators/generator_test.rb index 94d2c1bf50..7871399dd7 100644 --- a/railties/test/generators/generator_test.rb +++ b/railties/test/generators/generator_test.rb @@ -1,7 +1,6 @@ require 'active_support/test_case' require 'active_support/testing/autorun' require 'rails/generators/app_base' -require 'rails/generators/rails/app/app_generator' module Rails module Generators diff --git a/railties/test/generators/mailer_generator_test.rb b/railties/test/generators/mailer_generator_test.rb index d209801f60..25649881eb 100644 --- a/railties/test/generators/mailer_generator_test.rb +++ b/railties/test/generators/mailer_generator_test.rb @@ -69,12 +69,12 @@ class MailerGeneratorTest < Rails::Generators::TestCase def test_invokes_default_text_template_engine run_generator assert_file "app/views/notifier/foo.text.erb" do |view| - assert_match(%r(app/views/notifier/foo\.text\.erb), view) + assert_match(%r(\sapp/views/notifier/foo\.text\.erb), view) assert_match(/<%= @greeting %>/, view) end assert_file "app/views/notifier/bar.text.erb" do |view| - assert_match(%r(app/views/notifier/bar\.text\.erb), view) + assert_match(%r(\sapp/views/notifier/bar\.text\.erb), view) assert_match(/<%= @greeting %>/, view) end end @@ -82,12 +82,12 @@ class MailerGeneratorTest < Rails::Generators::TestCase def test_invokes_default_html_template_engine run_generator assert_file "app/views/notifier/foo.html.erb" do |view| - assert_match(%r(app/views/notifier/foo\.html\.erb), view) + assert_match(%r(\sapp/views/notifier/foo\.html\.erb), view) assert_match(/<%= @greeting %>/, view) end assert_file "app/views/notifier/bar.html.erb" do |view| - assert_match(%r(app/views/notifier/bar\.html\.erb), view) + assert_match(%r(\sapp/views/notifier/bar\.html\.erb), view) assert_match(/<%= @greeting %>/, view) end end |