Skip to content

Commit

Permalink
Merge pull request #1179 from suan/really_support_rfc6838
Browse files Browse the repository at this point in the history
Patch rack-accept to support all valid RFC6838 characters in media types
  • Loading branch information
dblock committed Oct 9, 2015
2 parents a7e4b2c + fe63400 commit 0924100
Show file tree
Hide file tree
Showing 9 changed files with 52 additions and 31 deletions.
1 change: 1 addition & 0 deletions Appraisals
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
appraise "rails-3" do
gem "rails", "3.2.19"
gem "rack-cache", "<= 1.2" # Pin as next rack-cache version (1.3) removes Ruby1.9 support
end

appraise "rails-4" do
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

* Your contribution here.

* [#1179](https://github.com/ruby-grape/grape/pull/1179): Allow all RFC6838 valid characters in header vendor - [@suan](https://github.com/suan).
* [#1170](https://github.com/ruby-grape/grape/pull/1170): Allow dashes and periods in header vendor - [@suan](https://github.com/suan).
* [#1167](https://github.com/ruby-grape/grape/pull/1167): Convenience wrapper `type: File` for validating multipart file parameters - [@dslh](https://github.com/dslh).
* [#1167](https://github.com/ruby-grape/grape/pull/1167): Refactor and extend coercion and type validation system - [@dslh](https://github.com/dslh).
Expand Down
3 changes: 0 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -339,9 +339,6 @@ vnd.vendor-and-or-resource-v1234+format
```

Basically all tokens between the final `-` and the `+` will be interpreted as the version.
Grape also only supports alphanumerics, periods, and dashes in the vendor/resource/version parts
of the media type, even though [the appropriate RFC](http://tools.ietf.org/html/rfc6838#section-4.2)
technically allows far more characters.

Using this versioning strategy, clients should pass the desired version in the HTTP `Accept` head.

Expand Down
14 changes: 7 additions & 7 deletions gemfiles/rack_1.5.2.gemfile
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
# This file was generated by Appraisal

source 'https://rubygems.org'
source "https://rubygems.org"

gem 'rack', '1.5.2'
gem "rack", "1.5.2"

group :development, :test do
gem 'rubocop', '~> 0.31.0'
gem 'guard'
gem 'guard-rspec'
gem 'guard-rubocop'
gem "rubocop", "0.33.0"
gem "guard"
gem "guard-rspec"
gem "guard-rubocop"
end

gemspec :path => '../'
gemspec :path => "../"
15 changes: 8 additions & 7 deletions gemfiles/rails_3.gemfile
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
# This file was generated by Appraisal

source 'https://rubygems.org'
source "https://rubygems.org"

gem 'rails', '3.2.19'
gem "rails", "3.2.19"
gem "rack-cache", "<= 1.2"

group :development, :test do
gem 'rubocop', '~> 0.31.0'
gem 'guard'
gem 'guard-rspec'
gem 'guard-rubocop'
gem "rubocop", "0.33.0"
gem "guard"
gem "guard-rspec"
gem "guard-rubocop"
end

gemspec :path => '../'
gemspec :path => "../"
14 changes: 7 additions & 7 deletions gemfiles/rails_4.gemfile
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
# This file was generated by Appraisal

source 'https://rubygems.org'
source "https://rubygems.org"

gem 'rails', '4.1.6'
gem "rails", "4.1.6"

group :development, :test do
gem 'rubocop', '~> 0.31.0'
gem 'guard'
gem 'guard-rspec'
gem 'guard-rubocop'
gem "rubocop", "0.33.0"
gem "guard"
gem "guard-rspec"
gem "guard-rubocop"
end

gemspec :path => '../'
gemspec :path => "../"
1 change: 1 addition & 0 deletions lib/grape/middleware/versioner/header.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'grape/middleware/base'
require 'grape/middleware/versioner/parse_media_type_patch'

module Grape
module Middleware
Expand Down
20 changes: 20 additions & 0 deletions lib/grape/middleware/versioner/parse_media_type_patch.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
module Rack
module Accept
module Header
class << self
# Corrected version of https://github.com/mjackson/rack-accept/blob/master/lib/rack/accept/header.rb#L40-L44
def parse_media_type(media_type)
# see http://tools.ietf.org/html/rfc6838#section-4.2 for allowed characters in media type names
m = media_type.to_s.match(%r{^([a-z*]+)\/([a-z0-9*\&\^\-_#\$!.+]+)(?:;([a-z0-9=;]+))?$})
m ? [m[1], m[2], m[3] || ''] : []
end
end
end

class MediaType
def parse_media_type(media_type)
Header.parse_media_type(media_type)
end
end
end
end
14 changes: 7 additions & 7 deletions spec/grape/middleware/versioner/header_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,8 @@

let(:v1_app) {
Class.new(Grape::API) do
version 'v1', using: :header, vendor: 'test.a-cool-resource'
content_type :v1_test, 'application/vnd.test.a-cool-resource-v1+json'
version 'v1', using: :header, vendor: 'test.a-cool_resource', cascade: false, strict: true
content_type :v1_test, 'application/vnd.test.a-cool_resource-v1+json'
formatter :v1_test, ->(object, _) { object }
format :v1_test

Expand All @@ -276,8 +276,8 @@

let(:v2_app) {
Class.new(Grape::API) do
version 'v2', using: :header, vendor: 'test.a-cool-resource'
content_type :v2_test, 'application/vnd.test.a-cool-resource-v2+json'
version 'v2', using: :header, vendor: 'test.a-cool_resource', strict: true
content_type :v2_test, 'application/vnd.test.a-cool_resource-v2+json'
formatter :v2_test, ->(object, _) { object }
format :v2_test

Expand All @@ -290,20 +290,20 @@
}

def app
subject.mount v1_app
subject.mount v2_app
subject.mount v1_app
subject
end

context 'with header versioned endpoints and a rescue_all block defined' do
it 'responds correctly to a v1 request' do
versioned_get '/users/hello', 'v1', using: :header, vendor: 'test.a-cool-resource'
versioned_get '/users/hello', 'v1', using: :header, vendor: 'test.a-cool_resource'
expect(last_response.body).to eq('one')
expect(last_response.body).not_to include('API vendor or version not found')
end

it 'responds correctly to a v2 request' do
versioned_get '/users/hello', 'v2', using: :header, vendor: 'test.a-cool-resource'
versioned_get '/users/hello', 'v2', using: :header, vendor: 'test.a-cool_resource'
expect(last_response.body).to eq('two')
expect(last_response.body).not_to include('API vendor or version not found')
end
Expand Down

0 comments on commit 0924100

Please sign in to comment.