From 460e443694fc9d9f83f22e6e288e8a39aae9ed10 Mon Sep 17 00:00:00 2001 From: Suan-Aik Yeo Date: Fri, 2 Oct 2015 18:14:15 -0400 Subject: [PATCH] allow dashes and other RFC6838-compliant characters in header vendor --- CHANGELOG.md | 1 + README.md | 11 ++++++++++ lib/grape/middleware/versioner/header.rb | 22 +++++++++++++------ .../grape/middleware/versioner/header_spec.rb | 18 ++++++++++----- 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39eeed90d..691001ba3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * Your contribution here. +* [#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). * [#1163](https://github.com/ruby-grape/grape/pull/1163): First-class `JSON` parameter type - [@dslh](https://github.com/dslh). diff --git a/README.md b/README.md index cf610ba89..41e4489f7 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/lib/grape/middleware/versioner/header.rb b/lib/grape/middleware/versioner/header.rb index c3ce0d318..c41c7976e 100644 --- a/lib/grape/middleware/versioner/header.rb +++ b/lib/grape/middleware/versioner/header.rb @@ -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' # @@ -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? @@ -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 @@ -169,7 +172,7 @@ 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) @@ -177,11 +180,16 @@ def request_vendor(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 diff --git a/spec/grape/middleware/versioner/header_spec.rb b/spec/grape/middleware/versioner/header_spec.rb index c0c5e824e..a2733b306 100644 --- a/spec/grape/middleware/versioner/header_spec.rb +++ b/spec/grape/middleware/versioner/header_spec.rb @@ -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 @@ -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' @@ -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' @@ -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