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 simply uses a blank report until validation,
when it then properly fails. 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 |    7 ++++++-
 app/models/report.rb                  |    9 ++++++---
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/app/controllers/reports_controller.rb 
b/app/controllers/reports_controller.rb
index 1a48456..068ff1a 100644
--- a/app/controllers/reports_controller.rb
+++ b/app/controllers/reports_controller.rb
@@ -10,7 +10,12 @@ class ReportsController < InheritedResources::Base
       return
     end
 
-    create!
+    create! do |success,failure|
+      failure.html do
+        @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..46e6728 100644
--- a/app/models/report.rb
+++ b/app/models/report.rb
@@ -2,11 +2,11 @@ class Report < ActiveRecord::Base
   def self.per_page; 20 end # Pagination
   belongs_to :node
 
+  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
 
@@ -24,7 +24,8 @@ class Report < ActiveRecord::Base
   end
 
   def report
-    rep = read_attribute(:report)
+    # Returning a blank report will let us fail gracefully for a bad report
+    rep = read_attribute(:report) rescue Puppet::Transaction::Report.new
     rep.extend(ReportExtensions) unless rep.nil? or rep.is_a?(ReportExtensions)
     rep
   end
@@ -81,6 +82,8 @@ class Report < ActiveRecord::Base
   end
 
   def report_contains_metrics
-    report.metrics.present?
+    valid = report.metrics.present?
+    errors.add_to_base("The report is in an invalid format") unless valid
+    valid
   end
 end
-- 
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.

Reply via email to