Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
From ce27a82d6fd29c555f414dde4254c70e524b6108 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 3fb2cda..5b4226c 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 1a028bb..df84bba 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&#x27;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', "&lt;b&gt;Name&lt;/b&gt; can't be blank"
+ assert_select 'span.error', "&lt;b&gt;Name&lt;/b&gt; can&#x27;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&#x27;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&#x27;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&#x27;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&#x27;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&#x27;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&#x27;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 0b2eea7..419c827 100644
--- a/test/form_builder/general_test.rb
+++ b/test/form_builder/general_test.rb
@@ -283,7 +283,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&#x27;t be blank"
end
test 'builder should be able to disable showing errors for a input' do
@@ -293,7 +293,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&#x27;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
You can’t perform that action at this time.