From 3836866b4a65c80973df417bc653057f92b5f736 Mon Sep 17 00:00:00 2001
From: Alex Hornung <alex@alexhornung.com>
Date: Fri, 3 Jan 2014 10:51:56 +0000
Subject: [PATCH] Fix for #464: gracefully handle invalid version headers

 * Catch the exception(s) thrown by rack-accept and return a 406 if one
   is caught.

 * Add a spec to cover this case.
---
 CHANGELOG.md                             |  1 +
 README.md                                |  4 ++++
 lib/grape/middleware/versioner/header.rb |  6 +++++-
 spec/grape/endpoint_spec.rb              | 19 +++++++++++++++++++
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 65adf0bfb7..a79c5b43e3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -25,6 +25,7 @@ Next Release
 * [#503](/~https://github.com/intridea/grape/pull/503): Calling declared(params) from child namespace fails to include parent namespace defined params - [@myitcv](/~https://github.com/myitcv).
 * [#512](/~https://github.com/intridea/grape/pull/512): Don't create `Grape::Request` multiple times - [@dblock](/~https://github.com/dblock).
 * [#538](/~https://github.com/intridea/grape/pull/538): Fixed default values for grouped params - [@dm1try](/~https://github.com/dm1try).
+* [#549](/~https://github.com/intridea/grape/pull/549): Fixed handling of invalid version headers to return 406 if a header cannot be parsed - [@bwalex](/~https://github.com/bwalex).
 
 
 0.6.1
diff --git a/README.md b/README.md
index 7ea79f1222..192bc1cf3b 100644
--- a/README.md
+++ b/README.md
@@ -231,6 +231,10 @@ supplied. This behavior is similar to routing in Rails. To circumvent this defau
 one could use the `:strict` option. When this option is set to `true`, a `406 Not Acceptable` error
 is returned when no correct `Accept` header is supplied.
 
+When an invalid `Accept` header is supplied, a `406 Not Acceptable` error is returned if the `:cascade`
+option is set to `false`. Otherwise a `404 Not Found` error is returned by Rack if no other route
+matches.
+
 ### Accept-Version Header
 
 ```ruby
diff --git a/lib/grape/middleware/versioner/header.rb b/lib/grape/middleware/versioner/header.rb
index 57124b2f33..231148f6c8 100644
--- a/lib/grape/middleware/versioner/header.rb
+++ b/lib/grape/middleware/versioner/header.rb
@@ -23,7 +23,11 @@ module Versioner
       # route.
       class Header < Base
         def before
-          header = Rack::Accept::MediaType.new env['HTTP_ACCEPT']
+          begin
+            header = Rack::Accept::MediaType.new env['HTTP_ACCEPT']
+          rescue RuntimeError => e
+            throw :error, status: 406, headers: error_headers, message: e.message
+          end
 
           if strict?
             # If no Accept header:
diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb
index faac50bb20..8fab2e2885 100644
--- a/spec/grape/endpoint_spec.rb
+++ b/spec/grape/endpoint_spec.rb
@@ -714,4 +714,23 @@ def memoized
     end
   end
 
+  context 'version headers' do
+    before do
+      # NOTE: a 404 is returned instead of the 406 if cascade: false is not set.
+      subject.version 'v1', using: :header, vendor: 'ohanapi', cascade: false
+      subject.get '/test' do
+        "Hello!"
+      end
+    end
+
+    it 'result in a 406 response if they are invalid' do
+      get '/test', {}, 'HTTP_ACCEPT' => 'application/vnd.ohanapi.v1+json'
+      last_response.status.should == 406
+    end
+
+    it 'result in a 406 response if they cannot be parsed by rack-accept' do
+      get '/test', {}, 'HTTP_ACCEPT' => 'application/vnd.ohanapi.v1+json; version=1'
+      last_response.status.should == 406
+    end
+  end
 end