From 7d0ba39309c28591b24fa3e5fdb73a70d37ccb9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Thu, 20 Nov 2014 15:22:59 -0200 Subject: [PATCH 1/3] Always escape error messages Before, if your error message contained HTML tags, they were marked as safe. Some error messages may contain user input so this would lead a XSS vulnerability. Error messages are now always escaped. If users need to mark them as safe they will need to use the explicit `:error` option: f.input :name, error: raw('My error') --- lib/simple_form/components/errors.rb | 6 ++-- test/form_builder/error_test.rb | 51 ++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/lib/simple_form/components/errors.rb b/lib/simple_form/components/errors.rb index 55f783f2..459281e0 100644 --- a/lib/simple_form/components/errors.rb +++ b/lib/simple_form/components/errors.rb @@ -18,13 +18,11 @@ module SimpleForm def error_text text = has_custom_error? ? options[:error] : errors.send(error_method) - "#{html_escape(options[:error_prefix])} #{text}".lstrip.html_safe + "#{html_escape(options[:error_prefix])} #{html_escape(text)}".lstrip.html_safe end def full_error_text - text = has_custom_error? ? options[:error] : full_errors.send(error_method) - - text.html_safe + has_custom_error? ? options[:error] : full_errors.send(error_method) end def error_method diff --git a/test/form_builder/error_test.rb b/test/form_builder/error_test.rb index c6f4c89b..b0cf4c13 100644 --- a/test/form_builder/error_test.rb +++ b/test/form_builder/error_test.rb @@ -85,6 +85,16 @@ class ErrorTest < ActionView::TestCase assert_no_select 'span.error b' end + test 'error escapes error text' do + @user.errors.add(:action, 'must not contain markup') + + with_error_for @user, :action + + assert_select 'span.error' + assert_no_select 'span.error b', 'markup' + end + + test 'error generates an error message with raw HTML tags' do with_error_for @user, :name, error_prefix: 'Name'.html_safe assert_select 'span.error', "Name can't be blank" @@ -115,6 +125,15 @@ class ErrorTest < ActionView::TestCase assert_equal({ id: 'name_error' }, options) end + test 'full error escapes error text' do + @user.errors.add(:action, 'must not contain markup') + + with_full_error_for @user, :action + + assert_select 'span.error' + assert_no_select 'span.error b', 'markup' + end + # CUSTOM WRAPPERS test 'error with custom wrappers works' do @@ -185,6 +204,38 @@ class ErrorTest < ActionView::TestCase end end + test 'input with custom error escapes the error text' do + with_form_for @user, :name, error: 'error must not contain markup' + + assert_select 'span.error' + assert_no_select 'span.error b', 'markup' + end + + test 'input with custom error does not escape the error text if it is safe' do + with_form_for @user, :name, error: 'error must contain markup'.html_safe + + assert_select 'span.error' + assert_select 'span.error b', 'markup' + end + + test 'input with custom error escapes the error text using full_error component' do + swap_wrapper :default, self.custom_wrapper_with_full_error do + with_form_for @user, :name, error: 'error must not contain markup' + + assert_select 'span.error' + assert_no_select 'span.error b', 'markup' + end + end + + test 'input with custom error does not escape the error text if it is safe using full_error component' do + swap_wrapper :default, self.custom_wrapper_with_full_error do + with_form_for @user, :name, error: 'error must contain markup'.html_safe + + assert_select 'span.error' + assert_select 'span.error b', 'markup' + end + end + test 'input with custom error when using full_error component does not generate the error if there is no error on the attribute' do swap_wrapper :default, self.custom_wrapper_with_full_error do with_form_for @user, :active, error: "Super User Active! can't be blank" From 542bc0d6ef7a3bb372dbdb5a50bfc6657389a632 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 25 Nov 2014 15:52:47 -0200 Subject: [PATCH 2/3] Fix tests for all supported Rails versions --- test/form_builder/error_test.rb | 20 ++++++++++---------- test/form_builder/general_test.rb | 4 ++-- test/form_builder/wrapper_test.rb | 2 +- test/support/models.rb | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/test/form_builder/error_test.rb b/test/form_builder/error_test.rb index b0cf4c13..a54bc5f4 100644 --- a/test/form_builder/error_test.rb +++ b/test/form_builder/error_test.rb @@ -32,7 +32,7 @@ class ErrorTest < ActionView::TestCase test 'error generates messages for attribute with single error' do with_error_for @user, :name - assert_select 'span.error', "can't be blank" + assert_select 'span.error', "cannot be blank" end test 'error generates messages for attribute with one error when using first' do @@ -97,7 +97,7 @@ class ErrorTest < ActionView::TestCase test 'error generates an error message with raw HTML tags' do with_error_for @user, :name, error_prefix: 'Name'.html_safe - assert_select 'span.error', "Name can't be blank" + assert_select 'span.error', "Name cannot be blank" assert_select 'span.error b', "Name" end @@ -105,7 +105,7 @@ class ErrorTest < ActionView::TestCase test 'full error generates a full error tag for the attribute' do with_full_error_for @user, :name - assert_select 'span.error', "Super User Name! can't be blank" + assert_select 'span.error', "Super User Name! cannot be blank" end test 'full error generates a full error tag with a clean HTML' do @@ -115,13 +115,13 @@ class ErrorTest < ActionView::TestCase test 'full error allows passing options to full error tag' do with_full_error_for @user, :name, id: 'name_error', error_prefix: "Your name" - assert_select 'span.error#name_error', "Your name can't be blank" + assert_select 'span.error#name_error', "Your name cannot be blank" end test 'full error does not modify the options hash' do options = { id: 'name_error' } with_full_error_for @user, :name, options - assert_select 'span.error#name_error', "Super User Name! can't be blank" + assert_select 'span.error#name_error', "Super User Name! cannot be blank" assert_equal({ id: 'name_error' }, options) end @@ -139,7 +139,7 @@ class ErrorTest < ActionView::TestCase test 'error with custom wrappers works' do swap_wrapper do with_error_for @user, :name - assert_select 'span.omg_error', "can't be blank" + assert_select 'span.omg_error', "cannot be blank" end end @@ -177,7 +177,7 @@ class ErrorTest < ActionView::TestCase # CUSTOM ERRORS test 'input with custom error works' do - error_text = "Super User Name! can't be blank" + error_text = "Super User Name! cannot be blank" with_form_for @user, :name, error: error_text assert_select 'span.error', error_text @@ -186,18 +186,18 @@ class ErrorTest < ActionView::TestCase test 'input with error option as true does not use custom error' do with_form_for @user, :name, error: true - assert_select 'span.error', "can't be blank" + assert_select 'span.error', "cannot be blank" end test 'input with custom error does not generate the error if there is no error on the attribute' do - with_form_for @user, :active, error: "Super User Active! can't be blank" + with_form_for @user, :active, error: "Super User Active! cannot be blank" assert_no_select 'span.error' end test 'input with custom error works when using full_error component' do swap_wrapper :default, self.custom_wrapper_with_full_error do - error_text = "Super User Name! can't be blank" + error_text = "Super User Name! cannot be blank" with_form_for @user, :name, error: error_text assert_select 'span.error', error_text diff --git a/test/form_builder/general_test.rb b/test/form_builder/general_test.rb index 06b11f29..70fb1735 100644 --- a/test/form_builder/general_test.rb +++ b/test/form_builder/general_test.rb @@ -327,7 +327,7 @@ class FormBuilderTest < ActionView::TestCase test 'builder generates errors for attribute with errors' do with_form_for @user, :name - assert_select 'span.error', "can't be blank" + assert_select 'span.error', "cannot be blank" end test 'builder is able to disable showing errors for an input' do @@ -337,7 +337,7 @@ class FormBuilderTest < ActionView::TestCase test 'builder passes options to errors' do with_form_for @user, :name, error_html: { id: "cool" } - assert_select 'span.error#cool', "can't be blank" + assert_select 'span.error#cool', "cannot be blank" end test 'placeholder does not be generated when set to false' do diff --git a/test/form_builder/wrapper_test.rb b/test/form_builder/wrapper_test.rb index e1922338..5f771534 100644 --- a/test/form_builder/wrapper_test.rb +++ b/test/form_builder/wrapper_test.rb @@ -139,7 +139,7 @@ class WrapperTest < ActionView::TestCase test 'custom wrappers can have full error message on attributes' do swap_wrapper :default, self.custom_wrapper_with_full_error do with_form_for @user, :name - assert_select 'span.error', "Name can't be blank" + assert_select 'span.error', "Name cannot be blank" end end diff --git a/test/support/models.rb b/test/support/models.rb index ed762df1..28124ecf 100644 --- a/test/support/models.rb +++ b/test/support/models.rb @@ -175,7 +175,7 @@ class User def errors @errors ||= begin errors = ActiveModel::Errors.new(self) - errors.add(:name, "can't be blank") + errors.add(:name, "cannot be blank") errors.add(:description, 'must be longer than 15 characters') errors.add(:age, 'is not a number') errors.add(:age, 'must be greater than 18') From 803389b593bdcaba1cf5017b70b0a17c6979a192 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 25 Nov 2014 15:53:40 -0200 Subject: [PATCH 3/3] Prepare for 3.1.0 release --- CHANGELOG.md | 12 +++--------- Gemfile.lock | 2 +- lib/simple_form/version.rb | 2 +- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54923780..6d267b9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,15 +1,7 @@ -## master (unreleased) +## 3.1.0 ### enhancements * Update foundation generator to version 5. [@jorge-d](https://github.com/jorge-d) - -### bug fix - * Fix `full_error` when the attribute is an association. [@mvdamme](https://github.com/jorge-d) - * Fix suppport to `:namespace` and `:index` options for nested check boxes and radio buttons when the attribute is an association. - -## 3.1.0.rc2 - -### enhancements * Add mapping to `uuid` columns. * Add custom namespaces for custom inputs feature. [@vala](https://github.com/vala) * Add `:unless_blank` option to the wrapper API. [@IanVaughan](https://github.com/IanVaughan) @@ -39,6 +31,8 @@ * The default form class can now be overridden with `html: { :class }`. [@rmm5t](https://github.com/rmm5t) ### bug fix + * Fix `full_error` when the attribute is an association. [@mvdamme](https://github.com/jorge-d) + * Fix suppport to `:namespace` and `:index` options for nested check boxes and radio buttons when the attribute is an association. * Collection input that uses automatic collection translation properly sets checked values. Closes [#971](https://github.com/plataformatec/simple_form/issues/971) [@nashby](https://github.com/nashby) * Collection input generates `required` attribute if it has `prompt` option. [@nashby](https://github.com/nashby) diff --git a/Gemfile.lock b/Gemfile.lock index 6ccfb75f..8762d7e0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -43,7 +43,7 @@ GIT PATH remote: . specs: - simple_form (3.1.0.rc2) + simple_form (3.1.0) actionpack (~> 4.0) activemodel (~> 4.0) diff --git a/lib/simple_form/version.rb b/lib/simple_form/version.rb index 95913206..54f9b1b0 100644 --- a/lib/simple_form/version.rb +++ b/lib/simple_form/version.rb @@ -1,3 +1,3 @@ module SimpleForm - VERSION = "3.1.0.rc2".freeze + VERSION = "3.1.0".freeze end