From 5a0edc0cdd35cb41f708c868da6db080c568db35 Mon Sep 17 00:00:00 2001 | |
From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= | |
<rafaelmfranca@gmail.com> | |
Date: Thu, 3 Apr 2014 17:31:54 -0300 | |
Subject: [PATCH] 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 <b>error</b>') | |
--- | |
lib/simple_form/components/errors.rb | 8 +++++- | |
lib/simple_form/inputs/base.rb | 2 +- | |
test/form_builder/error_test.rb | 56 +++++++++++++++++++++++++++++++----- | |
test/form_builder/general_test.rb | 4 +-- | |
4 files changed, 59 insertions(+), 11 deletions(-) | |
diff --git a/lib/simple_form/components/errors.rb b/lib/simple_form/components/errors.rb | |
index aad4833..d60a48d 100644 | |
--- a/lib/simple_form/components/errors.rb | |
+++ b/lib/simple_form/components/errors.rb | |
@@ -12,7 +12,9 @@ module SimpleForm | |
protected | |
def error_text | |
- "#{html_escape(options[:error_prefix])} #{errors.send(error_method)}".lstrip.html_safe | |
+ text = has_error_in_options? ? options[:error] : errors.send(error_method) | |
+ | |
+ "#{html_escape(options[:error_prefix])} #{html_escape(text)}".lstrip.html_safe | |
end | |
def error_method | |
@@ -30,6 +32,10 @@ module SimpleForm | |
def errors_on_association | |
reflection ? object.errors[reflection.name] : [] | |
end | |
+ | |
+ def has_error_in_options? | |
+ options[:error] && !options[:error].nil? | |
+ end | |
end | |
end | |
end | |
diff --git a/lib/simple_form/inputs/base.rb b/lib/simple_form/inputs/base.rb | |
index b3f4b0d..465405b 100644 | |
--- a/lib/simple_form/inputs/base.rb | |
+++ b/lib/simple_form/inputs/base.rb | |
@@ -45,7 +45,7 @@ module SimpleForm | |
end | |
# Always enabled. | |
- enable :hint | |
+ enable :hint, :error | |
# Usually disabled, needs to be enabled explicitly passing true as option. | |
disable :maxlength, :placeholder, :pattern, :min_max | |
diff --git a/test/form_builder/error_test.rb b/test/form_builder/error_test.rb | |
index 06a024a..d0e4dba 100644 | |
--- a/test/form_builder/error_test.rb | |
+++ b/test/form_builder/error_test.rb | |
@@ -14,6 +14,10 @@ class ErrorTest < ActionView::TestCase | |
end | |
end | |
+ def with_custom_error_for(object, *args) | |
+ with_form_for(object, *args) | |
+ end | |
+ | |
test 'error should not generate content for attribute without errors' do | |
with_error_for @user, :active | |
assert_no_select 'span.error' | |
@@ -32,7 +36,7 @@ class ErrorTest < ActionView::TestCase | |
test 'error should generate messages for attribute with single error' do | |
with_error_for @user, :name | |
- assert_select 'span.error', "can't be blank" | |
+ assert_select 'span.error', "can't be blank" | |
end | |
test 'error should generate messages for attribute with one error when using first' do | |
@@ -82,12 +86,21 @@ class ErrorTest < ActionView::TestCase | |
test 'error should escape error prefix text' do | |
with_error_for @user, :name, error_prefix: '<b>Name</b>' | |
- assert_select 'span.error', "<b>Name</b> can't be blank" | |
+ assert_select 'span.error', "<b>Name</b> can't be blank" | |
+ end | |
+ | |
+ test 'error escapes error text' do | |
+ @user.errors.merge!(action: ['must not contain <b>markup</b>']) | |
+ | |
+ with_error_for @user, :action | |
+ | |
+ assert_select 'span.error' | |
+ assert_no_select 'span.error b', 'markup' | |
end | |
test 'error should generate an error message with raw HTML tags' do | |
with_error_for @user, :name, error_prefix: '<b>Name</b>'.html_safe | |
- assert_select 'span.error', "Name can't be blank" | |
+ assert_select 'span.error', "Name can't be blank" | |
assert_select 'span.error b', "Name" | |
end | |
@@ -95,7 +108,7 @@ class ErrorTest < ActionView::TestCase | |
test 'full error should generate an 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! can't be blank" | |
end | |
test 'full error should generate an full error tag with a clean HTML' do | |
@@ -105,13 +118,13 @@ class ErrorTest < ActionView::TestCase | |
test 'full error should allow 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 can't be blank" | |
end | |
test 'full error should 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! can't be blank" | |
assert_equal({ id: 'name_error' }, options) | |
end | |
@@ -120,7 +133,36 @@ 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', "can't be blank" | |
end | |
end | |
+ | |
+ # CUSTOM ERRORS | |
+ | |
+ test 'input with custom error works' do | |
+ with_custom_error_for(@user, :name, error: "Super User Name! can't be blank") | |
+ | |
+ assert_select 'span.error', "Super User Name! can't be blank" | |
+ end | |
+ | |
+ test 'input with custom error does not generate the error if there is no error on the attribute' do | |
+ error_text = "Super User Active! can't be blank" | |
+ with_form_for @user, :active, error: error_text | |
+ | |
+ assert_no_select 'span.error' | |
+ end | |
+ | |
+ test 'input with custom error escapes the error text' do | |
+ with_form_for @user, :name, error: 'error must not contain <b>markup</b>' | |
+ | |
+ 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 <b>markup</b>'.html_safe | |
+ | |
+ assert_select 'span.error' | |
+ assert_select 'span.error b', 'markup' | |
+ end | |
end | |
diff --git a/test/form_builder/general_test.rb b/test/form_builder/general_test.rb | |
index 223732a..f3a0b48 100644 | |
--- a/test/form_builder/general_test.rb | |
+++ b/test/form_builder/general_test.rb | |
@@ -302,7 +302,7 @@ class FormBuilderTest < ActionView::TestCase | |
test 'builder should generate errors for attribute with errors' do | |
with_form_for @user, :name | |
- assert_select 'span.error', "can't be blank" | |
+ assert_select 'span.error', "can't be blank" | |
end | |
test 'builder should be able to disable showing errors for a input' do | |
@@ -312,7 +312,7 @@ class FormBuilderTest < ActionView::TestCase | |
test 'builder should pass 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', "can't be blank" | |
end | |
test 'placeholder should not be generated when set to false' do | |
-- | |
2.0.2 | |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment