Skip to content

Commit

Permalink
allow dashes and other RFC6838-compliant characters in header vendor
Browse files Browse the repository at this point in the history
  • Loading branch information
suan authored and Suan-Aik Yeo committed Oct 5, 2015
1 parent 05d697c commit 8dac028
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 12 deletions.
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.

* [#1170](https://github.com/ruby-grape/grape/pull/1170): Allow dashes, periods, and other RFC6838-compliant characters 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).
* [#1163](https://github.com/ruby-grape/grape/pull/1163): First-class `JSON` parameter type - [@dslh](https://github.com/dslh).
Expand Down
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,17 @@ Using this versioning strategy, clients should pass the desired version in the U
version 'v1', using: :header, vendor: 'twitter'
```

Currently, Grape only supports versioned media types in the following format:

```
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.

curl -H Accept:application/vnd.twitter-v1+json http://localhost:9292/statuses/public_timeline
Expand Down
22 changes: 15 additions & 7 deletions lib/grape/middleware/versioner/header.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ module Versioner
# application/vnd.:vendor-:version+:format
#
# Example: For request header
# Accept: application/vnd.mycompany-v1+json
# Accept: application/vnd.mycompany.a-cool-resource-v1+json
#
# The following rack env variables are set:
#
# env['api.type'] => 'application'
# env['api.subtype'] => 'vnd.mycompany-v1+json'
# env['api.vendor] => 'mycompany'
# env['api.subtype'] => 'vnd.mycompany.a-cool-resource-v1+json'
# env['api.vendor] => 'mycompany.a-cool-resource'
# env['api.version] => 'v1'
# env['api.format] => 'json'
#
Expand All @@ -23,7 +23,10 @@ module Versioner
# route.
class Header < Base
VENDOR_VERSION_HEADER_REGEX =
/\Avnd\.([a-z0-9*.]+)(?:-([a-z0-9*\-.]+))?(?:\+([a-z0-9*\-.+]+))?\z/
/\Avnd\.([a-z0-9.\-_!#\$&\^]+?)(?:-([a-z0-9*.]+))?(?:\+([a-z0-9*\-.]+))?\z/

HAS_VENDOR_REGEX = /\Avnd\.[a-z0-9.\-_!#\$&\^]+/
HAS_VERSION_REGEX = /\Avnd\.([a-z0-9.\-_!#\$&\^]+?)(?:-([a-z0-9*.]+))+/

def before
strict_header_checks if strict?
Expand Down Expand Up @@ -122,7 +125,7 @@ def headers_contain_wrong_vendor?

def headers_contain_wrong_version?
header.values.all? do |header_value|
version?(header_value)
version?(header_value) && !versions.include?(request_version(header_value))
end
end

Expand Down Expand Up @@ -169,19 +172,24 @@ def error_headers
# @return [Boolean] whether the content type sets a vendor
def vendor?(media_type)
_, subtype = Rack::Accept::Header.parse_media_type(media_type)
subtype[/\Avnd\.[a-z0-9*.]+/]
subtype[HAS_VENDOR_REGEX]
end

def request_vendor(media_type)
_, subtype = Rack::Accept::Header.parse_media_type(media_type)
subtype.match(VENDOR_VERSION_HEADER_REGEX)[1]
end

def request_version(media_type)
_, subtype = Rack::Accept::Header.parse_media_type(media_type)
subtype.match(VENDOR_VERSION_HEADER_REGEX)[2]
end

# @param [String] media_type a content type
# @return [Boolean] whether the content type sets an API version
def version?(media_type)
_, subtype = Rack::Accept::Header.parse_media_type(media_type)
subtype[/\Avnd\.[a-z0-9*.]+-[a-z0-9*\-.]+/]
subtype[HAS_VERSION_REGEX]
end
end
end
Expand Down
18 changes: 13 additions & 5 deletions spec/grape/middleware/versioner/header_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@
end
end

context 'when there are multiple versions specified with rescue_from :all' do
context 'when there are multiple versions with complex vendor specified with rescue_from :all' do
subject {
Class.new(Grape::API) do
rescue_from :all
Expand All @@ -261,7 +261,11 @@

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

resources :users do
get :hello do
'one'
Expand All @@ -272,7 +276,11 @@

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

resources :users do
get :hello do
'two'
Expand All @@ -289,13 +297,13 @@ def app

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'
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'
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 8dac028

Please sign in to comment.