diff options
18 files changed, 174 insertions, 32 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 465d19e50d..d275569cf6 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,7 @@ ## Rails 3.2.3 (unreleased) ## +* Add `config.action_view.embed_authenticity_token_in_remote_forms` (defaults to true) which allows to set if authenticity token will be included by default in remote forms. If you change it to false, you can still force authenticity token by passing `:authenticity_token => true` in form options *Piotr Sarnacki* + * Do not include the authenticity token in forms where remote: true as ajax forms use the meta-tag value *DHH* * Turn off verbose mode of rack-cache, we still have X-Rack-Cache to diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 2267f3c185..252fe1ed03 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -656,15 +656,16 @@ module ActionView # 'Accept <a href="/terms">Terms</a>.'.html_safe # end def label(object_name, method, content_or_options = nil, options = nil, &block) + options ||= {} + content_is_options = content_or_options.is_a?(Hash) if content_is_options || block_given? - options = content_or_options if content_is_options + options.merge!(content_or_options) if content_is_options text = nil else text = content_or_options end - options ||= {} InstanceTag.new(object_name, method, self, options.delete(:object)).to_label_tag(text, options, &block) end @@ -1072,7 +1073,7 @@ module ActionView options["cols"], options["rows"] = size.split("x") if size.respond_to?(:split) end - content_tag("textarea", "\n#{options.delete('value') || value_before_type_cast(object)}", options) + content_tag("textarea", options.delete('value') || value_before_type_cast(object), options) end def to_check_box_tag(options = {}, checked_value = "1", unchecked_value = "0") diff --git a/actionpack/lib/action_view/helpers/form_tag_helper.rb b/actionpack/lib/action_view/helpers/form_tag_helper.rb index 4ce878f26a..066b98d4a2 100644 --- a/actionpack/lib/action_view/helpers/form_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/form_tag_helper.rb @@ -2,6 +2,7 @@ require 'cgi' require 'action_view/helpers/tag_helper' require 'active_support/core_ext/object/blank' require 'active_support/core_ext/string/output_safety' +require 'active_support/core_ext/module/attribute_accessors' module ActionView # = Action View Form Tag Helpers @@ -17,6 +18,9 @@ module ActionView include UrlHelper include TextHelper + mattr_accessor :embed_authenticity_token_in_remote_forms + self.embed_authenticity_token_in_remote_forms = true + # Starts a form tag that points the action to an url configured with <tt>url_for_options</tt> just like # ActionController::Base#url_for. The method for the form defaults to POST. # @@ -27,9 +31,11 @@ module ActionView # is added to simulate the verb over post. # * <tt>:authenticity_token</tt> - Authenticity token to use in the form. Use only if you need to # pass custom authenticity token string, or to not add authenticity_token field at all - # (by passing <tt>false</tt>). If this is a remote form, the authenticity_token will by default - # not be included as the ajax handler will get it from the meta-tag (but you can force it to be - # rendered anyway in that case by passing <tt>true</tt>). + # (by passing <tt>false</tt>). Remote forms may omit the embedded authenticity token + # by setting <tt>config.action_view.embed_authenticity_token_in_remote_forms = false</tt>. + # This is helpful when you're fragment-caching the form. Remote forms get the + # authenticity from the <tt>meta</tt> tag, so embedding is unnecessary unless you + # support browsers without JavaScript. # * A list of parameters to feed to the URL the form will be posted to. # * <tt>:remote</tt> - If set to true, will allow the Unobtrusive JavaScript drivers to control the # submit behavior. By default this behavior is an ajax submit. @@ -611,16 +617,18 @@ module ActionView # responsibility of the caller to escape all the values. html_options["action"] = url_for(url_for_options) html_options["accept-charset"] = "UTF-8" - + html_options["data-remote"] = true if html_options.delete("remote") - if html_options["data-remote"] && html_options["authenticity_token"] == true + if html_options["data-remote"] && + !embed_authenticity_token_in_remote_forms && + html_options["authenticity_token"].blank? + # The authenticity token is taken from the meta tag in this case + html_options["authenticity_token"] = false + elsif html_options["authenticity_token"] == true # Include the default authenticity_token, which is only generated when its set to nil, # but we needed the true value to override the default of no authenticity_token on data-remote. html_options["authenticity_token"] = nil - elsif html_options["data-remote"] - # The authenticity token is taken from the meta tag in this case - html_options["authenticity_token"] = false end end end diff --git a/actionpack/lib/action_view/helpers/tag_helper.rb b/actionpack/lib/action_view/helpers/tag_helper.rb index 8c33ef09fa..2b0dd96435 100644 --- a/actionpack/lib/action_view/helpers/tag_helper.rb +++ b/actionpack/lib/action_view/helpers/tag_helper.rb @@ -17,6 +17,10 @@ module ActionView autofocus novalidate formnovalidate open pubdate).to_set BOOLEAN_ATTRIBUTES.merge(BOOLEAN_ATTRIBUTES.map {|attribute| attribute.to_sym }) + PRE_CONTENT_STRINGS = { + :textarea => "\n" + } + # Returns an empty HTML tag of type +name+ which by default is XHTML # compliant. Set +open+ to true to create an open tag compatible # with HTML 4.0 and below. Add HTML attributes by passing an attributes @@ -125,7 +129,7 @@ module ActionView def content_tag_string(name, content, options, escape = true) tag_options = tag_options(options, escape) if options - "<#{name}#{tag_options}>#{escape ? ERB::Util.h(content) : content}</#{name}>".html_safe + "<#{name}#{tag_options}>#{PRE_CONTENT_STRINGS[name.to_sym]}#{escape ? ERB::Util.h(content) : content}</#{name}>".html_safe end def tag_options(options, escape = true) diff --git a/actionpack/lib/action_view/railtie.rb b/actionpack/lib/action_view/railtie.rb index 80391d72cc..5086fdc6a3 100644 --- a/actionpack/lib/action_view/railtie.rb +++ b/actionpack/lib/action_view/railtie.rb @@ -7,6 +7,14 @@ module ActionView config.action_view = ActiveSupport::OrderedOptions.new config.action_view.stylesheet_expansions = {} config.action_view.javascript_expansions = { :defaults => %w(jquery jquery_ujs) } + config.action_view.embed_authenticity_token_in_remote_forms = true + + initializer "action_view.embed_authenticity_token_in_remote_forms" do |app| + ActiveSupport.on_load(:action_view) do + ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms = + app.config.action_view.delete(:embed_authenticity_token_in_remote_forms) + end + end initializer "action_view.cache_asset_ids" do |app| unless app.config.cache_classes diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index b8b14f3a24..144003a59b 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -45,6 +45,14 @@ module RequestForgeryProtectionActions render :inline => "<%= form_for(:some_resource, :remote => true, :authenticity_token => true ) {} %>" end + def form_for_with_token + render :inline => "<%= form_for(:some_resource, :authenticity_token => true ) {} %>" + end + + def form_for_remote_with_external_token + render :inline => "<%= form_for(:some_resource, :remote => true, :authenticity_token => 'external_token') {} %>" + end + def rescue_action(e) raise e end end @@ -111,11 +119,48 @@ module RequestForgeryProtectionTests assert_select 'form>div>input[name=?][value=?]', 'custom_authenticity_token', @token end - def test_should_render_form_without_token_tag_if_remote + def test_should_render_form_with_token_tag_if_remote + assert_not_blocked do + get :form_for_remote + end + assert_match(/authenticity_token/, response.body) + end + + def test_should_render_form_without_token_tag_if_remote_and_embedding_token_is_off + ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms = false assert_not_blocked do get :form_for_remote end assert_no_match(/authenticity_token/, response.body) + ensure + ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms = true + end + + def test_should_render_form_with_token_tag_if_remote_and_embedding_token_is_off_but_true_option_passed + ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms = false + assert_not_blocked do + get :form_for_remote_with_token + end + assert_match(/authenticity_token/, response.body) + ensure + ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms = true + end + + def test_should_render_form_with_token_tag_if_remote_and_external_authenticity_token_requested_and_embedding_is_off + ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms = false + assert_not_blocked do + get :form_for_remote_with_external_token + end + assert_select 'form>div>input[name=?][value=?]', 'custom_authenticity_token', 'external_token' + ensure + ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms = true + end + + def test_should_render_form_with_token_tag_if_remote_and_external_authenticity_token_requested + assert_not_blocked do + get :form_for_remote_with_external_token + end + assert_select 'form>div>input[name=?][value=?]', 'custom_authenticity_token', 'external_token' end def test_should_render_form_with_token_tag_if_remote_and_authenticity_token_requested @@ -125,6 +170,13 @@ module RequestForgeryProtectionTests assert_select 'form>div>input[name=?][value=?]', 'custom_authenticity_token', @token end + def test_should_render_form_with_token_tag_with_authenticity_token_requested + assert_not_blocked do + get :form_for_with_token + end + assert_select 'form>div>input[name=?][value=?]', 'custom_authenticity_token', @token + end + def test_should_allow_get assert_not_blocked { get :index } end @@ -270,10 +322,6 @@ class FreeCookieControllerTest < ActionController::TestCase end end - - - - class CustomAuthenticityParamControllerTest < ActionController::TestCase def setup ActionController::Base.request_forgery_protection_token = :custom_token_name diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index f9940ead58..7eeede7087 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -943,6 +943,41 @@ class FormHelperTest < ActionView::TestCase assert_dom_equal expected, output_buffer end + def test_form_for_label_error_wrapping + form_for(@post) do |f| + concat f.label(:author_name, :class => 'label') + concat f.text_field(:author_name) + concat f.submit('Create post') + end + + expected = whole_form("/posts/123", "edit_post_123" , "edit_post", :method => "put") do + "<div class='field_with_errors'><label for='post_author_name' class='label'>Author name</label></div>" + + "<div class='field_with_errors'><input name='post[author_name]' size='30' type='text' id='post_author_name' value='' /></div>" + + "<input name='commit' type='submit' value='Create post' />" + end + + assert_dom_equal expected, output_buffer + end + + + def test_form_for_label_error_wrapping_without_conventional_instance_variable + post = remove_instance_variable :@post + + form_for(post) do |f| + concat f.label(:author_name, :class => 'label') + concat f.text_field(:author_name) + concat f.submit('Create post') + end + + expected = whole_form("/posts/123", "edit_post_123" , "edit_post", :method => "put") do + "<div class='field_with_errors'><label for='post_author_name' class='label'>Author name</label></div>" + + "<div class='field_with_errors'><input name='post[author_name]' size='30' type='text' id='post_author_name' value='' /></div>" + + "<input name='commit' type='submit' value='Create post' />" + end + + assert_dom_equal expected, output_buffer + end + def test_form_for_with_namespace form_for(@post, :namespace => 'namespace') do |f| concat f.text_field(:title) diff --git a/actionpack/test/template/form_tag_helper_test.rb b/actionpack/test/template/form_tag_helper_test.rb index 233907d07a..dcc6dd0411 100644 --- a/actionpack/test/template/form_tag_helper_test.rb +++ b/actionpack/test/template/form_tag_helper_test.rb @@ -216,19 +216,19 @@ class FormTagHelperTest < ActionView::TestCase def test_text_area_tag_size_string actual = text_area_tag "body", "hello world", "size" => "20x40" - expected = %(<textarea cols="20" id="body" name="body" rows="40">hello world</textarea>) + expected = %(<textarea cols="20" id="body" name="body" rows="40">\nhello world</textarea>) assert_dom_equal expected, actual end def test_text_area_tag_size_symbol actual = text_area_tag "body", "hello world", :size => "20x40" - expected = %(<textarea cols="20" id="body" name="body" rows="40">hello world</textarea>) + expected = %(<textarea cols="20" id="body" name="body" rows="40">\nhello world</textarea>) assert_dom_equal expected, actual end def test_text_area_tag_should_disregard_size_if_its_given_as_an_integer actual = text_area_tag "body", "hello world", :size => 20 - expected = %(<textarea id="body" name="body">hello world</textarea>) + expected = %(<textarea id="body" name="body">\nhello world</textarea>) assert_dom_equal expected, actual end @@ -239,19 +239,19 @@ class FormTagHelperTest < ActionView::TestCase def test_text_area_tag_escape_content actual = text_area_tag "body", "<b>hello world</b>", :size => "20x40" - expected = %(<textarea cols="20" id="body" name="body" rows="40"><b>hello world</b></textarea>) + expected = %(<textarea cols="20" id="body" name="body" rows="40">\n<b>hello world</b></textarea>) assert_dom_equal expected, actual end def test_text_area_tag_unescaped_content actual = text_area_tag "body", "<b>hello world</b>", :size => "20x40", :escape => false - expected = %(<textarea cols="20" id="body" name="body" rows="40"><b>hello world</b></textarea>) + expected = %(<textarea cols="20" id="body" name="body" rows="40">\n<b>hello world</b></textarea>) assert_dom_equal expected, actual end def test_text_area_tag_unescaped_nil_content actual = text_area_tag "body", nil, :escape => false - expected = %(<textarea id="body" name="body"></textarea>) + expected = %(<textarea id="body" name="body">\n</textarea>) assert_dom_equal expected, actual end diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 2a7a77777d..06b66b5195 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -64,6 +64,7 @@ module ActiveRecord return if attribute_methods_generated? superclass.define_attribute_methods unless self == base_class super(column_names) + column_names.each { |name| define_external_attribute_method(name) } @attribute_methods_generated = true end end diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 1548114580..9b3724777a 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -71,22 +71,25 @@ module ActiveRecord generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1 def __temp__ - #{internal_attribute_access_code(attr_name, cast_code)} + #{internal_attribute_access_code(attr_name, attribute_cast_code(attr_name))} end alias_method '#{attr_name}', :__temp__ undef_method :__temp__ STR + end + + private + def define_external_attribute_method(attr_name) generated_external_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1 def __temp__(v, attributes, attributes_cache, attr_name) - #{external_attribute_access_code(attr_name, cast_code)} + #{external_attribute_access_code(attr_name, attribute_cast_code(attr_name))} end alias_method '#{attr_name}', :__temp__ undef_method :__temp__ STR end - private def cacheable_column?(column) attribute_types_cached_by_default.include?(column.type) end diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index 17cf34cdf6..c9b2edbd8c 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -43,6 +43,7 @@ module ActiveRecord end time = time.in_time_zone rescue nil if time write_attribute(:#{attr_name}, original_time) + #{attr_name}_will_change! @attributes_cache["#{attr_name}"] = time end EOV diff --git a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb index 9ca63a209e..b9b5ec7956 100644 --- a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb +++ b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb @@ -13,9 +13,9 @@ class <%= migration_class_name %> < ActiveRecord::Migration <% attributes.each do |attribute| -%> <%- if migration_action -%> <%= migration_action %>_column :<%= table_name %>, :<%= attribute.name %><% if migration_action == 'add' %>, :<%= attribute.type %><%= attribute.inject_options %><% end %> - <% if attribute.has_index? && migration_action == 'add' %> + <%- if attribute.has_index? && migration_action == 'add' -%> add_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %> - <% end -%> + <%- end -%> <%- end -%> <%- end -%> end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index acb9a44305..5e5325b570 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -620,6 +620,20 @@ class AttributeMethodsTest < ActiveRecord::TestCase end end + def test_time_zone_aware_attribute_saved + old_default, ActiveRecord::Base.default_timezone = ActiveRecord::Base.default_timezone, :utc + + in_time_zone 1 do + record = @target.create(:written_on => '2012-02-20 10:00') + + record.written_on = '2012-02-20 09:00' + record.save + assert_equal Time.zone.local(2012, 02, 20, 9), record.reload.written_on + end + ensure + ActiveRecord::Base.default_timezone = old_default + end + def test_setting_time_zone_aware_attribute_to_blank_string_returns_nil in_time_zone "Pacific Time (US & Canada)" do record = @target.new diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index aeb44da2b2..300a3636e3 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -61,7 +61,11 @@ end class Weird < ActiveRecord::Base; end -class Boolean < ActiveRecord::Base; end +class Boolean < ActiveRecord::Base + def has_fun + super + end +end class LintTest < ActiveRecord::TestCase include ActiveModel::Lint::Tests @@ -930,6 +934,16 @@ class BasicsTest < ActiveRecord::TestCase assert b_true.value? end + def test_boolean_without_questionmark + b_true = Boolean.create({ "value" => true }) + true_id = b_true.id + + subclass = Class.new(Boolean).find true_id + superclass = Boolean.find true_id + + assert_equal superclass.read_attribute(:has_fun), subclass.read_attribute(:has_fun) + end + def test_boolean_cast_from_string b_blank = Boolean.create({ "value" => "" }) blank_id = b_blank.id diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index fb4ea4223d..7028b7fd1b 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -90,6 +90,7 @@ ActiveRecord::Schema.define do create_table :booleans, :force => true do |t| t.boolean :value + t.boolean :has_fun, :null => false, :default => false end create_table :bulbs, :force => true do |t| diff --git a/railties/guides/source/configuring.textile b/railties/guides/source/configuring.textile index 11dbba0e3d..60034cdda5 100644 --- a/railties/guides/source/configuring.textile +++ b/railties/guides/source/configuring.textile @@ -375,6 +375,8 @@ And can reference in the view with the following code: * +config.action_view.cache_asset_ids+ With the cache enabled, the asset tag helper methods will make fewer expensive file system calls (the default implementation checks the file system timestamp). However this prevents you from modifying any asset files while the server is running. +* +config.action_view.embed_authenticity_token_in_remote_forms+ This is by default set to true. If you set it to false, authenticity_token will not be added to forms with +:remote => true+ by default. You can force +authenticity_token+ to be added to such remote form by passing +:authenticity_token => true+ option. + h4. Configuring Action Mailer There are a number of settings available on +config.action_mailer+: diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index a942c11518..2843ac8c1a 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -139,13 +139,13 @@ module Rails <<-GEMFILE.strip_heredoc gem 'rails', :path => '#{Rails::Generators::RAILS_DEV_PATH}' gem 'journey', :git => 'git://github.com/rails/journey.git' - gem 'arel', :git => 'git://github.com/rails/arel.git' + gem 'arel', :git => 'git://github.com/rails/arel.git', :branch => '3-0-stable' GEMFILE elsif options.edge? <<-GEMFILE.strip_heredoc gem 'rails', :git => 'git://github.com/rails/rails.git', :branch => '3-2-stable' gem 'journey', :git => 'git://github.com/rails/journey.git' - gem 'arel', :git => 'git://github.com/rails/arel.git' + gem 'arel', :git => 'git://github.com/rails/arel.git', :branch => '3-0-stable' GEMFILE else <<-GEMFILE.strip_heredoc diff --git a/railties/test/generators/shared_generator_tests.rb b/railties/test/generators/shared_generator_tests.rb index b262951b88..c072aaab26 100644 --- a/railties/test/generators/shared_generator_tests.rb +++ b/railties/test/generators/shared_generator_tests.rb @@ -127,7 +127,7 @@ module SharedGeneratorTests quietly { generator.invoke_all } assert_file 'Gemfile', %r{^gem\s+["']rails["'],\s+:git\s+=>\s+["']#{Regexp.escape("git://github.com/rails/rails.git")}["'],\s+:branch\s+=>\s+["']3-2-stable["']$} assert_file 'Gemfile', %r{^gem\s+["']journey["'],\s+:git\s+=>\s+["']#{Regexp.escape("git://github.com/rails/journey.git")}["']$} - assert_file 'Gemfile', %r{^gem\s+["']arel["'],\s+:git\s+=>\s+["']#{Regexp.escape("git://github.com/rails/arel.git")}["']$} + assert_file 'Gemfile', %r{^gem\s+["']arel["'],\s+:git\s+=>\s+["']#{Regexp.escape("git://github.com/rails/arel.git")}["'].*$} end def test_skip_gemfile |