Add validation of string site settings with regex, and min and max lengths

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}"

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
 

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

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

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]

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

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

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