From 3eb65885d1677de353b9f7bbeb3b6d1c10a2408e Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Wed, 18 Jun 2014 10:49:21 -0400 Subject: [PATCH] Add validation of string site settings with regex, and min and max lengths --- config/locales/server.en.yml | 5 + config/site_settings.yml | 28 +++-- lib/site_setting_extension.rb | 14 +-- lib/validators/email_setting_validator.rb | 2 +- lib/validators/integer_setting_validator.rb | 2 +- lib/validators/string_setting_validator.rb | 38 +++++++ lib/validators/username_setting_validator.rb | 2 +- .../string_setting_validator_spec.rb | 100 ++++++++++++++++++ 8 files changed, 167 insertions(+), 24 deletions(-) create mode 100644 lib/validators/string_setting_validator.rb create mode 100644 spec/components/validators/string_setting_validator_spec.rb diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 330720e6c2c..e33219821d0 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -922,6 +922,11 @@ en: invalid_integer_min: "Value must be %{min} or greater." invalid_integer_max: "Value cannot be higher than %{max}." invalid_integer: "Value must be an integer." + regex_mismatch: "Value doesn't match the required format." + invalid_string: "Invalid value." + invalid_string_min_max: "Must be between %{min} and %{max} characters." + invalid_string_min: "Must be at least %{min} characters." + invalid_string_max: "Must be no more than %{max} characters." notification_types: mentioned: "%{display_username} mentioned you in %{link}" diff --git a/config/site_settings.yml b/config/site_settings.yml index c1438266d28..7b2473e22c5 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1,13 +1,21 @@ -# Possible types: +# Available options: # -# email - Must be a valid email address -# username - Must match the username of an existing user -# integer - Must be an integer. Can also provide max and min options to limit the range of values. +# default - The default value of the setting. +# client - Set to true if the javascript should have access to this setting's value. +# refresh - Set to true if clients should refresh when the setting is changed. +# min - For a string setting, the minimum length. For an integer setting, the minimum value. +# max - For a string setting, the maximum length. For an integer setting, the maximum value. +# regex - A regex that the value must match. +# enum - The setting has a fixed set of allowed values, and only one can be chosen. +# Set to the class name that defines the set. +# type: email - Must be a valid email address. +# type: username - Must match the username of an existing user. required: title: client: true default: 'Discourse' - site_description: '' + site_description: + default: '' contact_email: default: '' type: email @@ -45,7 +53,6 @@ basic: suggested_topics: client: true default: 5 - type: integer min: 0 limit_suggested_to_category: client: false @@ -125,32 +132,26 @@ basic: relative_date_duration: client: true default: 30 - type: integer min: 0 topics_per_period_in_top_summary: default: 20 - type: integer min: 1 topics_per_period_in_top_page: default: 50 - type: integer min: 1 category_featured_topics: client: true default: 3 - type: integer min: 1 fixed_category_positions: client: true default: false topics_per_page: default: 30 - type: integer min: 1 posts_per_page: client: true default: 20 - type: integer min: 1 enable_badges: client: true @@ -172,18 +173,15 @@ users: min_username_length: client: true default: 3 - type: integer min: 1 max_username_length: client: true default: 20 - type: integer min: 8 max: 60 min_password_length: client: true default: 8 - type: integer min: 1 block_common_passwords: true diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index 205d4e13027..c09c2d71834 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -82,8 +82,9 @@ module SiteSettingExtension if opts[:refresh] refresh_settings << name end - if validator_type = opts[:type] - validators[name] = {class: validator_for(validator_type), opts: opts} + + if validator_type = validator_for(opts[:type] || get_data_type(name, defaults[name])) + validators[name] = {class: validator_type, opts: opts} end current[name] = current_value @@ -242,7 +243,7 @@ module SiteSettingExtension if v = validators[name] validator = v[:class].new(v[:opts]) unless validator.valid_value?(val) - raise Discourse::InvalidParameters.new(validator.error_message(val)) + raise Discourse::InvalidParameters.new(validator.error_message) end end @@ -327,9 +328,10 @@ module SiteSettingExtension def validator_for(type_name) @validator_mapping ||= { - 'email' => EmailSettingValidator, - 'username' => UsernameSettingValidator, - 'integer' => IntegerSettingValidator + 'email' => EmailSettingValidator, + 'username' => UsernameSettingValidator, + types[:fixnum] => IntegerSettingValidator, + types[:string] => StringSettingValidator } @validator_mapping[type_name] end diff --git a/lib/validators/email_setting_validator.rb b/lib/validators/email_setting_validator.rb index d35f7279fc7..e9fb583167b 100644 --- a/lib/validators/email_setting_validator.rb +++ b/lib/validators/email_setting_validator.rb @@ -7,7 +7,7 @@ class EmailSettingValidator !val.present? || !!(EmailValidator.email_regex =~ val) end - def error_message(val) + def error_message I18n.t('site_settings.errors.invalid_email') end end diff --git a/lib/validators/integer_setting_validator.rb b/lib/validators/integer_setting_validator.rb index f3259246c85..2c92f3c20b2 100644 --- a/lib/validators/integer_setting_validator.rb +++ b/lib/validators/integer_setting_validator.rb @@ -10,7 +10,7 @@ class IntegerSettingValidator true end - def error_message(val) + def error_message if @opts[:min] && @opts[:max] I18n.t('site_settings.errors.invalid_integer_min_max', {min: @opts[:min], max: @opts[:max]}) elsif @opts[:min] diff --git a/lib/validators/string_setting_validator.rb b/lib/validators/string_setting_validator.rb new file mode 100644 index 00000000000..4f7e9a054bb --- /dev/null +++ b/lib/validators/string_setting_validator.rb @@ -0,0 +1,38 @@ +class StringSettingValidator + def initialize(opts={}) + @opts = opts + @regex = Regexp.new(opts[:regex]) if opts[:regex] + end + + def valid_value?(val) + return true if !val.present? + + if (@opts[:min] and @opts[:min].to_i > val.length) || (@opts[:max] and @opts[:max].to_i < val.length) + @length_fail = true + return false + end + + if @regex and !(val =~ @regex) + @regex_fail = true + return false + end + + true + end + + def error_message + if @regex_fail + I18n.t('site_settings.errors.regex_mismatch') + elsif @length_fail + if @opts[:min] && @opts[:max] + I18n.t('site_settings.errors.invalid_string_min_max', {min: @opts[:min], max: @opts[:max]}) + elsif @opts[:min] + I18n.t('site_settings.errors.invalid_string_min', {min: @opts[:min]}) + else + I18n.t('site_settings.errors.invalid_string_max', {max: @opts[:max]}) + end + else + I18n.t('site_settings.errors.invalid_string') + end + end +end diff --git a/lib/validators/username_setting_validator.rb b/lib/validators/username_setting_validator.rb index 67509ebbba9..3dbc75fba1d 100644 --- a/lib/validators/username_setting_validator.rb +++ b/lib/validators/username_setting_validator.rb @@ -7,7 +7,7 @@ class UsernameSettingValidator !val.present? || User.where(username: val).exists? end - def error_message(val) + def error_message I18n.t('site_settings.errors.invalid_username') end end diff --git a/spec/components/validators/string_setting_validator_spec.rb b/spec/components/validators/string_setting_validator_spec.rb new file mode 100644 index 00000000000..b241428e079 --- /dev/null +++ b/spec/components/validators/string_setting_validator_spec.rb @@ -0,0 +1,100 @@ +require 'spec_helper' + +describe StringSettingValidator do + + describe '#valid_value?' do + shared_examples "for all StringSettingValidator opts" do + it "returns true for blank values" do + validator.valid_value?('').should == true + validator.valid_value?(nil).should == true + end + end + + context 'with a regex' do + subject(:validator) { described_class.new(regex: 'bacon') } + + include_examples "for all StringSettingValidator opts" + + it "returns true if value matches the regex" do + validator.valid_value?('The bacon is delicious').should == true + end + + it "returns false if the value doesn't match the regex" do + validator.valid_value?('The vegetables are delicious').should == false + end + + it "test some other regexes" do + v = described_class.new(regex: '^(chocolate|banana)$') + v.valid_value?('chocolate').should == true + v.valid_value?('chocolates').should == false + + v = described_class.new(regex: '^[\w]+$') + v.valid_value?('the_file').should == true + v.valid_value?('the_file.bat').should == false + end + end + + context 'with min' do + subject(:validator) { described_class.new(min: 2) } + + include_examples "for all StringSettingValidator opts" + + it "returns true if length is ok" do + validator.valid_value?('ok').should == true + validator.valid_value?('yep long enough').should == true + end + + it "returns false if too short" do + validator.valid_value?('x').should == false + end + end + + context 'with max' do + subject(:validator) { described_class.new(max: 5) } + + include_examples "for all StringSettingValidator opts" + + it "returns true if length is ok" do + validator.valid_value?('Z').should == true + validator.valid_value?('abcde').should == true + end + + it "returns false if too long" do + validator.valid_value?('banana').should == false + end + end + + context 'combinations of options' do + it "min and regex" do + v = described_class.new(regex: '^[\w]+$', min: 3) + v.valid_value?('chocolate').should == true + v.valid_value?('hi').should == false + v.valid_value?('game.exe').should == false + end + + it "max and regex" do + v = described_class.new(regex: '^[\w]+$', max: 5) + v.valid_value?('chocolate').should == false + v.valid_value?('a_b_c').should == true + v.valid_value?('a b c').should == false + end + + it "min and max" do + v = described_class.new(min: 3, max: 5) + v.valid_value?('chocolate').should == false + v.valid_value?('a').should == false + v.valid_value?('a b c').should == true + v.valid_value?('a b').should == true + end + + it "min, max, and regex" do + v = described_class.new(min: 3, max: 12, regex: 'bacon') + v.valid_value?('go bacon!').should == true + v.valid_value?('sprinkle bacon on your cereal').should == false + v.valid_value?('ba').should == false + end + end + + end + +end