While validation was already present, deserializing the report during the before_validation step of assigning certain parameters was failing, before the validation even got the chance to protect anything. This patch adds an earlier before_validation step which verifies that the report is actually a Puppet::Transaction::Report and declares it invalid if not. It also logs errors and will return 406 Not Acceptable for an invalid report.
Signed-off-by: Nick Lewis <[email protected]> --- app/controllers/reports_controller.rb | 8 +++++++- app/models/report.rb | 16 ++++++++++++++-- spec/controllers/reports_controller_spec.rb | 10 ++++++++++ spec/models/report_spec.rb | 17 ++++++++++++++++- 4 files changed, 47 insertions(+), 4 deletions(-) diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 1a48456..68991f4 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -10,7 +10,13 @@ class ReportsController < InheritedResources::Base return end - create! + create! do |success,failure| + failure.html do + Rails.logger.debug "WARNING! ReportsController#create failed:" + @report.errors.full_messages.each { |msg| Rails.logger.debug msg } + render :status => 406 + end + end end private diff --git a/app/models/report.rb b/app/models/report.rb index 04c81b8..c08e3e7 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -2,11 +2,12 @@ class Report < ActiveRecord::Base def self.per_page; 20 end # Pagination belongs_to :node + before_validation :ensure_valid_format + before_validation :process_report validate :report_contains_metrics validates_presence_of :host validates_presence_of :time validates_uniqueness_of :host, :scope => :time, :allow_nil => true - before_validation :process_report after_save :update_node after_destroy :replace_last_report @@ -54,6 +55,15 @@ class Report < ActiveRecord::Base private + def ensure_valid_format + begin + report + rescue ActiveRecord::SerializationTypeMismatch + errors.add_to_base("The report is in an invalid format") + false + end + end + def process_report set_attributes assign_to_node @@ -81,6 +91,8 @@ class Report < ActiveRecord::Base end def report_contains_metrics - report.metrics.present? + has_metrics = report.metrics.present? + errors.add_to_base("The report contains no metrics") unless has_metrics + has_metrics end end diff --git a/spec/controllers/reports_controller_spec.rb b/spec/controllers/reports_controller_spec.rb index 6de85bd..5f3771b 100644 --- a/spec/controllers/reports_controller_spec.rb +++ b/spec/controllers/reports_controller_spec.rb @@ -51,6 +51,16 @@ describe ReportsController do it { should == "406" } end + + describe "with a POST with invalid report data, the response code" do + before :each do + post(:create, :report => "foo bar baz bad data invalid") + end + + subject { response.code } + + it { should == "406" } + end end def post_with_body(action, body, headers) diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb index 3de7d09..a2c9b51 100644 --- a/spec/models/report_spec.rb +++ b/spec/models/report_spec.rb @@ -32,8 +32,23 @@ describe Report do Report.find(report.id).should_not be_success end - it "is not created if a report for the same host exists with the same time" do + it "should properly create a valid report" do + + end + + it "should consider a blank report to be invalid" do + Report.create(:report => '').should_not be_valid + end + it "should consider a report in incorrect format to be invalid" do + Report.create(:report => 'foo bar baz bad data invalid').should_not be_valid + end + + it "should consider a report in correct format to be valid" do + report_from_yaml.should be_valid + end + + it "is not created if a report for the same host exists with the same time" do Report.create(:report => @report_yaml) lambda { Report.create(:report => @report_yaml) -- 1.7.3 -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
