merged

Gemfile.rails_version

@@ -1,6 +1,7 @@
 # For testing against different releases of Rails.  This is in a
 # separate fragment so that it can be sourced from the test
 # application's Gemfile in addition to the main development Gemfile.
+
 if ENV['RAILS_VERSION']
   case ENV['RAILS_VERSION']
   when /3.0$/
@@ -13,7 +14,14 @@ if ENV['RAILS_VERSION']
     gem 'rails', '~> 3.2.0'
     # A JS runtime is required for Rails 3.1+
     gem 'therubyracer', '~> 0.10.2'
+  when /4.0$/
+    gem 'rails', '~> 4.0.0'
+    # A JS runtime is required for Rails 4.0+
+    gem 'therubyracer', platforms: :ruby  
+    gem 'protected_attributes', :git => 'git://github.com/rails/protected_attributes.git'
   else
     fail "Unknown Rails version #{ENV['RAILS_VERSION']}"
   end
+else 
+  fail "ENV['RAILS_VERSION'] not set"
 end

Rakefile

@@ -41,7 +41,11 @@ namespace :testbed do
   desc 'Generate a minimal surveyor-using rails app'
   task :generate do
     Tempfile.open('surveyor_Rakefile') do |f|
-      f.write("application \"config.time_zone='Rome'\"");f.flush
+      f.write("application \"config.time_zone='Rome'\"\n")
+      # SMELL add support for protected_attributes
+      f.write("application \"config.active_record.whitelist_attributes = true\"\n")
+      f.write("application \"config.active_record.mass_assignment_sanitizer = :strict\"\n")
+      f.flush
       sh "bundle exec rails new testbed --skip-bundle -m #{f.path}" # don't run bundle install until the Gemfile modifications
     end
     chdir('testbed') do

lib/surveyor.rb

@@ -1,5 +1,5 @@
 module Surveyor
-  require 'surveyor/engine' if defined?(Rails) && Rails::VERSION::MAJOR == 3
+  require 'surveyor/engine' if defined?(Rails) && Rails::VERSION::MAJOR >= 3
   autoload :VERSION, 'surveyor/version'
   autoload :ParserError, 'surveyor/parser'
 end

lib/surveyor/models/answer_methods.rb

@@ -24,8 +24,9 @@ def self.included(base)
           @@validations_already_included = true
         end
 
-        # Whitelisting attributes
-        base.send :attr_accessible, :question, :question_id, :text, :short_text, :help_text, :weight, :response_class, :reference_identifier, :data_export_identifier, :common_namespace, :common_identifier, :display_order, :is_exclusive, :display_length, :custom_class, :custom_renderer, :default_value, :display_type, :input_mask, :input_mask_placeholder
+          # Whitelisting attributes
+          base.send :attr_accessible, :question, :question_id, :text, :short_text, :help_text, :weight, :response_class, :reference_identifier, :data_export_identifier, :common_namespace, :common_identifier, :display_order, :is_exclusive, :display_length, :custom_class, :custom_renderer, :default_value, :display_type, :input_mask, :input_mask_placeholder
+
       end
 
       # Instance Methods

lib/surveyor/models/dependency_methods.rb

@@ -11,7 +11,7 @@ def self.included(base)
         unless @@validations_already_included
           # Validations
           base.send :validates_presence_of, :rule
-          base.send :validates_format_of, :rule, :with => /^(?:and|or|\)|\(|[A-Z]|\s)+$/ #TODO properly formed parenthesis etc.
+          base.send :validates_format_of, :rule, :with => /^(?:and|or|\)|\(|[A-Z]|\s)+$/, :multiline => true #TODO properly formed parenthesis etc. # SMELL with :multiline => true Rails reports a security risk
           base.send :validates_numericality_of, :question_id, :if => Proc.new { |d| d.question_group_id.nil? }
           base.send :validates_numericality_of, :question_group_id, :if => Proc.new { |d| d.question_id.nil? }
 

lib/surveyor/models/validation_condition_methods.rb

@@ -46,7 +46,8 @@ def is_valid?(response)
           !(response.as(klass) == compare_to.as(klass))
         when "=~"
           return false if compare_to != self
-          !(response.as(klass).to_s =~ Regexp.new(self.regexp || "")).nil?
+          # SMELL Eval regex!!
+          !(response.as(klass).to_s =~ Regexp.new(eval(self.regexp) || "")).nil?
         else
           false
         end

lib/surveyor/models/validation_methods.rb

@@ -12,7 +12,7 @@ def self.included(base)
         unless @@validations_already_included
           # Validations
           base.send :validates_presence_of, :rule
-          base.send :validates_format_of, :rule, :with => /^(?:and|or|\)|\(|[A-Z]|\s)+$/
+          base.send :validates_format_of, :rule, :with => /^(?:and|or|\)|\(|[A-Z]|\s)+$/, :multiline => true # SMELL with :multiline => true Rails reports a security risk
           # this causes issues with building and saving
           # base.send :validates_numericality_of, :answer_id
 

spec/controllers/surveyor_controller_spec.rb

@@ -6,10 +6,10 @@
     @routes = Surveyor::Engine.routes
   end
 
-  let!(:survey)           { Factory(:survey, :title => "Alphabet", :access_code => "alpha", :survey_version => 0)}
-  let!(:survey_beta)      { Factory(:survey, :title => "Alphabet", :access_code => "alpha", :survey_version => 1)}
-  let!(:response_set)      { Factory(:response_set, :survey => survey, :access_code => "pdq")}
-  let!(:response_set_beta) { Factory(:response_set, :survey => survey_beta, :access_code => "rst")}
+  let!(:survey)           { FactoryGirl.create(:survey, :title => "Alphabet", :access_code => "alpha", :survey_version => 0)}
+  let!(:survey_beta)      { FactoryGirl.create(:survey, :title => "Alphabet", :access_code => "alpha", :survey_version => 1)}
+  let!(:response_set)      { FactoryGirl.create(:response_set, :survey => survey, :access_code => "pdq")}
+  let!(:response_set_beta) { FactoryGirl.create(:response_set, :survey => survey_beta, :access_code => "rst")}
   before { ResponseSet.stub!(:create).and_return(response_set) }
 
   # match '/', :to                                     => 'surveyor#new', :as    => 'available_surveys', :via => :get
@@ -114,7 +114,7 @@ def do_get(params = {})
 
   context "#edit" do
     def do_get(params = {})
-      survey.sections = [Factory(:survey_section, :survey => survey)]
+      survey.sections = [FactoryGirl.create(:survey_section, :survey => survey)]
       get :edit, {:survey_code => "alpha", :response_set_code => "pdq"}.merge(params)
     end
     it "renders edit" do
@@ -132,13 +132,13 @@ def do_get(params = {})
       response.should redirect_to(available_surveys_path)
     end
     it "assigns dependents if javascript not enabled" do
-      controller.stub!(:get_unanswered_dependencies_minus_section_questions).and_return([Factory(:question)])
+      controller.stub!(:get_unanswered_dependencies_minus_section_questions).and_return([FactoryGirl.create(:question)])
       session[:surveyor_javascript].should be_nil
       do_get
       assigns[:dependents].should_not be_empty
     end
     it "does not assign dependents if javascript is enabled" do
-      controller.stub!(:get_unanswered_dependencies_minus_section_questions).and_return([Factory(:question)])
+      controller.stub!(:get_unanswered_dependencies_minus_section_questions).and_return([FactoryGirl.create(:question)])
       session[:surveyor_javascript] = "enabled"
       do_get
       assigns[:dependents].should be_empty
@@ -149,7 +149,7 @@ def do_get(params = {})
       assigns[:survey].should == survey
     end
     it "assigns later survey_version" do
-      survey_beta.sections = [Factory(:survey_section, :survey => survey_beta)]
+      survey_beta.sections = [FactoryGirl.create(:survey_section, :survey => survey_beta)]
       do_get :response_set_code => "rst"
       assigns[:survey].should == survey_beta
       assigns[:response_set].should == response_set_beta
@@ -184,14 +184,14 @@ def do_get(params = {})
       end
       context "with update exceptions" do
         it 'retries the update on a constraint violation' do
-          response_set.should_receive(:update_from_ui_hash).ordered.with(responses_ui_hash).and_raise(ActiveRecord::StatementInvalid)
+ response_set.should_receive(:update_from_ui_hash).ordered.with(responses_ui_hash).and_raise(ActiveRecord::StatementInvalid.new('statement invalid'))
           response_set.should_receive(:update_from_ui_hash).ordered.with(responses_ui_hash)
 
           expect { do_put(:r => responses_ui_hash) }.to_not raise_error
         end
 
         it 'only retries three times' do
-          response_set.should_receive(:update_from_ui_hash).exactly(3).times.with(responses_ui_hash).and_raise(ActiveRecord::StatementInvalid)
+          response_set.should_receive(:update_from_ui_hash).exactly(3).times.with(responses_ui_hash).and_raise(ActiveRecord::StatementInvalid.new('statement invalid'))
 
           expect { do_put(:r => responses_ui_hash) }.to raise_error(ActiveRecord::StatementInvalid)
         end