From c1c45a3f2ca3123ccb8ff005b568ab34a69b0e0d Mon Sep 17 00:00:00 2001
From: Matthew Ford <matt@new-bamboo.co.uk>
Date: Thu, 8 May 2008 22:45:30 +0100
Subject: [PATCH] spec and code for catching invalid filter options

---
 lib/merb-core/controller/abstract_controller.rb |   10 ++++++++--
 spec/public/abstract_controller/filter_spec.rb  |    9 +++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/lib/merb-core/controller/abstract_controller.rb b/lib/merb-core/controller/abstract_controller.rb
index d12406d..7c4fc91 100644
--- a/lib/merb-core/controller/abstract_controller.rb
+++ b/lib/merb-core/controller/abstract_controller.rb
@@ -72,6 +72,8 @@ class Merb::AbstractController
   include Merb::InlineTemplates

   class_inheritable_accessor :_before_filters, :_after_filters, :_layout, :_template_root
+
+  FilterOptions = [:only, :exclude, :if, :unless, :with]

   # ==== Returns
   # String:: The controller name in path form, e.g. "admin/items".
@@ -435,8 +437,8 @@ class Merb::AbstractController
   #
   # ==== Raises
   # ArgumentError::
-  #   Both :only and :exclude, or :if and :unless given, or filter is not a
-  #   Symbol, String or Proc.
+  #   Both :only and :exclude, or :if and :unless given, if filter is not a
+  #   Symbol, String or Proc, or if an unknown option is passed.
   def self.add_filter(filters, filter, opts={})
     raise(ArgumentError,
       "You can specify either :only or :exclude but
@@ -445,6 +447,10 @@ class Merb::AbstractController
      raise(ArgumentError,
        "You can specify either :if or :unless but
         not both at the same time for the same filter.") if opts.key?(:if) && opts.key?(:unless)
+
+      opts.each_key {|key| raise(ArgumentError,
+          "You can only specify known filter options, #{key} is invalid.") unless FilterOptions.include?(key)
+          }

     opts = normalize_filters!(opts)

diff --git a/spec/public/abstract_controller/filter_spec.rb b/spec/public/abstract_controller/filter_spec.rb
index 7835c2f..2a19ecc 100644
--- a/spec/public/abstract_controller/filter_spec.rb
+++ b/spec/public/abstract_controller/filter_spec.rb
@@ -74,6 +74,15 @@ describe Merb::AbstractController, " should support before and after filters" do
     end }.should raise_error(ArgumentError, /either :if or :unless/)
   end

+  it "should throw an error if an unknown option is passed to a filter" do
+    running { Merb::Test::Fixtures::Abstract.class_eval do
+
+      class TestErrorFilter < Merb::Test::Fixtures::Abstract::Testing
+        before :foo, :except => :index
+      end
+    end }.should raise_error(ArgumentError, /known filter options/)
+  end
+
   it "should throw an error" do
     running { dispatch_should_make_body("TestConditionalFilterWithNoProcOrSymbol", "") }.should raise_error(ArgumentError, /a Symbol or a Proc/)
   end
--
1.5.4.5