From fe6340038462c482b3e1ae98a89e0752fe710516 Mon Sep 17 00:00:00 2001 From: Suan-Aik Yeo Date: Wed, 7 Oct 2015 10:03:06 -0400 Subject: [PATCH] Patch rack-accept to support all valid RFC6838 characters in media types --- Appraisals | 1 + CHANGELOG.md | 1 + README.md | 3 --- gemfiles/rack_1.5.2.gemfile | 14 ++++++------- gemfiles/rails_3.gemfile | 15 +++++++------- gemfiles/rails_4.gemfile | 14 ++++++------- lib/grape/middleware/versioner/header.rb | 1 + .../versioner/parse_media_type_patch.rb | 20 +++++++++++++++++++ .../grape/middleware/versioner/header_spec.rb | 14 ++++++------- 9 files changed, 52 insertions(+), 31 deletions(-) create mode 100644 lib/grape/middleware/versioner/parse_media_type_patch.rb diff --git a/Appraisals b/Appraisals index 351ea8fa21..695d50bc76 100644 --- a/Appraisals +++ b/Appraisals @@ -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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 691001ba31..34c36baf15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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). diff --git a/README.md b/README.md index e57f437947..f0afd435a1 100644 --- a/README.md +++ b/README.md @@ -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. diff --git a/gemfiles/rack_1.5.2.gemfile b/gemfiles/rack_1.5.2.gemfile index 0ef881bcbc..a269f61505 100644 --- a/gemfiles/rack_1.5.2.gemfile +++ b/gemfiles/rack_1.5.2.gemfile @@ -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 => "../" diff --git a/gemfiles/rails_3.gemfile b/gemfiles/rails_3.gemfile index 63d48aa432..fb164a00ad 100644 --- a/gemfiles/rails_3.gemfile +++ b/gemfiles/rails_3.gemfile @@ -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 => "../" diff --git a/gemfiles/rails_4.gemfile b/gemfiles/rails_4.gemfile index 69e4d1532a..12e4f2e37e 100644 --- a/gemfiles/rails_4.gemfile +++ b/gemfiles/rails_4.gemfile @@ -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 => "../" diff --git a/lib/grape/middleware/versioner/header.rb b/lib/grape/middleware/versioner/header.rb index c41c7976e9..fa3ac907e4 100644 --- a/lib/grape/middleware/versioner/header.rb +++ b/lib/grape/middleware/versioner/header.rb @@ -1,4 +1,5 @@ require 'grape/middleware/base' +require 'grape/middleware/versioner/parse_media_type_patch' module Grape module Middleware diff --git a/lib/grape/middleware/versioner/parse_media_type_patch.rb b/lib/grape/middleware/versioner/parse_media_type_patch.rb new file mode 100644 index 0000000000..cf0c987ef1 --- /dev/null +++ b/lib/grape/middleware/versioner/parse_media_type_patch.rb @@ -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 diff --git a/spec/grape/middleware/versioner/header_spec.rb b/spec/grape/middleware/versioner/header_spec.rb index a2733b3067..2df31aef15 100644 --- a/spec/grape/middleware/versioner/header_spec.rb +++ b/spec/grape/middleware/versioner/header_spec.rb @@ -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 @@ -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 @@ -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