Merge pull request ruby-grape#144 from rodzyn/accept_header_strict

Accept header strict

Author: Daniel Doubrovkine (dB.)

lib/grape/middleware/versioner/header.rb

@@ -18,16 +18,16 @@ module Versioner
       #    env['api.version]  => 'v1'
       #    env['api.format]   => 'format'
       #
-      # If version does not match this route, then a 404 is throw with
+      # If version does not match this route, then a 406 is throw with
       # X-Cascade header to alert Rack::Mount to attempt the next matched
       # route.
       class Header < Base
         def before
           accept = env['HTTP_ACCEPT'] || ""
 
           if options[:version_options] && options[:version_options].keys.include?(:strict) && options[:version_options][:strict]
-            if (accept.nil? || accept.empty?) && options[:versions] && options[:version_options][:using] == :header
-              throw :error, :status => 404, :headers => {'X-Cascade' => 'pass'}, :message => "404 API Version Not Found"
+            if (is_accept_header_valid?(accept))  && options[:version_options][:using] == :header
+              throw :error, :status => 406, :headers => {'X-Cascade' => 'pass'}, :message => "406 API Version Not Found"
             end
           end
           accept.strip.scan(/^(.+?)\/(.+?)$/) do |type, subtype|
@@ -39,7 +39,7 @@ def before
               is_vendored_match = is_vendored ? options[:version_options][:vendor] == vendor : true
 
               if (options[:versions] && !options[:versions].include?(version)) || !is_vendored_match
-                throw :error, :status => 404, :headers => {'X-Cascade' => 'pass'}, :message => "404 API Version Not Found"
+                throw :error, :status => 406, :headers => {'X-Cascade' => 'pass'}, :message => "406 API Version Not Found"
               end
 
               env['api.version'] = version
@@ -48,6 +48,11 @@ def before
             end
           end
         end
+
+        protected
+        def is_accept_header_valid?(header)
+          (header.strip =~ /application\/vnd\.(.+?)-(.+?)\+(.+?)/).nil?
+        end
       end
     end
   end

spec/grape/middleware/versioner/header_spec.rb

@@ -69,15 +69,46 @@
       subject.call({}).first.should == 200
     end
 
-    it 'should return a 404 when no header is set but strict header based versioning is used' do
-      @options = {
-        :versions => ['v1'],
-        :version_options => {:using => :header, :strict => true}
-      }
-      expect {
-        env = subject.call('HTTP_ACCEPT' => '').last
-      }.to throw_symbol(:error, :status => 404, :headers => {'X-Cascade' => 'pass'}, :message => "404 API Version Not Found")
+    context 'when strict header versioning is used' do
+      it 'should return a 406 when no header' do
+        @options = {
+          :versions => ['v1'],
+          :version_options => {:using => :header, :strict => true}
+        }
+        expect {
+          env = subject.call('HTTP_ACCEPT' => '').last
+        }.to throw_symbol(
+          :error,
+          :status => 406,
+          :headers => {'X-Cascade' => 'pass'},
+          :message => "406 API Version Not Found"
+        )
+      end
+
+      it 'should return a 406 when incorrect header format is used' do
+        @options = {
+          :versions => ['v1'],
+          :version_options => {:using => :header, :strict => true}
+        }
+        expect {
+          env = subject.call('HTTP_ACCEPT' => '*/*').last
+        }.to throw_symbol(
+          :error,
+          :status => 406,
+          :headers => {'X-Cascade' => 'pass'},
+          :message => "406 API Version Not Found"
+        )
+      end
+
+      it 'should return a 200 when proper header is set' do
+        @options = {
+          :versions => ['v1'],
+          :version_options => {:using => :header, :strict => true}
+        }
+        subject.call('HTTP_ACCEPT' => 'application/vnd.testing-v1+json').first.should == 200
+      end
     end
+
   end
 
   context 'vendors' do
@@ -96,7 +127,7 @@
     it 'should not match with an incorrect vendor' do
       expect {
         env = subject.call('HTTP_ACCEPT' => 'application/vnd.othervendor-v1+json').last
-      }.to throw_symbol(:error, :status => 404, :headers => {'X-Cascade' => 'pass'}, :message => "404 API Version Not Found")
+      }.to throw_symbol(:error, :status => 406, :headers => {'X-Cascade' => 'pass'}, :message => "406 API Version Not Found")
     end
   end
 
@@ -108,10 +139,10 @@
       }
     end
 
-    it 'should throw 404 error with X-Cascade header set to pass' do
+    it 'should throw 406 error with X-Cascade header set to pass' do
       expect {
         env = subject.call('HTTP_ACCEPT' => accept).last
-      }.to throw_symbol(:error, :status => 404, :headers => {'X-Cascade' => 'pass'}, :message => "404 API Version Not Found")
+      }.to throw_symbol(:error, :status => 406, :headers => {'X-Cascade' => 'pass'}, :message => "406 API Version Not Found")
     end
   end
 end