improve quoted parameters in mime types

actionpack/CHANGELOG.md

@@ -1,3 +1,11 @@
+*   The `Mime::Type` now supports handling types with parameters and correctly handles quotes.
+    When parsing the accept header, the parameters before the q-parameter are kept and if a matching mime-type exists it is used.
+    To keep the current functionality, a fallback is created to look for the media-type without the parameters.
+
+    This change allows for custom MIME-types that are more complex like `application/vnd.api+json; profile="https://jsonapi.org/profiles/ethanresnick/cursor-pagination/" ext="https://jsonapi.org/ext/atomic"` for the [JSON API](https://jsonapi.org/).
+
+    *Nicolas Erni*
+
 *   The url_for helpers now support a new option called `path_params`.
     This is very useful in situations where you only want to add a required param that is part of the route's URL but for other route not append an extraneous query param.
 

actionpack/lib/action_dispatch/http/mime_type.rb

@@ -152,14 +152,18 @@ def self.find_item_by_name(array, name)
 
     class << self
       TRAILING_STAR_REGEXP = /^(text|application)\/\*/
-      PARAMETER_SEPARATOR_REGEXP = /;\s*\w+="?\w+"?/
+      # all media-type parameters need to be before the q-parameter
+      # https://www.rfc-editor.org/rfc/rfc7231#section-5.3.2
+      PARAMETER_SEPARATOR_REGEXP = /\s*;\s*q="?/
+      ACCEPT_HEADER_REGEXP = /[^,\s"](?:[^,"]|"[^"]*")*/
 
       def register_callback(&block)
         @register_callbacks << block
       end
 
       def lookup(string)
-        LOOKUP[string] || Type.new(string)
+        # fallback to the media-type without parameters if it was not found
+        LOOKUP[string] || LOOKUP[string.split(";", 2)[0].rstrip] || Type.new(string)
       end
 
       def lookup_by_extension(extension)
@@ -195,7 +199,7 @@ def parse(accept_header)
           parse_trailing_star(accept_header) || Array(Mime::Type.lookup(accept_header))
         else
           list, index = [], 0
-          accept_header.split(",").each do |header|
+          accept_header.scan(ACCEPT_HEADER_REGEXP).each do |header|
             params, q = header.split(PARAMETER_SEPARATOR_REGEXP)
 
             next unless params
@@ -244,7 +248,7 @@ def unregister(symbol)
     attr_reader :hash
 
     MIME_NAME = "[a-zA-Z0-9][a-zA-Z0-9#{Regexp.escape('!#$&-^_.+')}]{0,126}"
-    MIME_PARAMETER_VALUE = "#{Regexp.escape('"')}?#{MIME_NAME}#{Regexp.escape('"')}?"
+    MIME_PARAMETER_VALUE = "(?:#{MIME_NAME}|\"[^\"\r\\\\]*\")"
     MIME_PARAMETER = "\s*;\s*#{MIME_NAME}(?:=#{MIME_PARAMETER_VALUE})?"
     MIME_REGEXP = /\A(?:\*\/\*|#{MIME_NAME}\/(?:\*|#{MIME_NAME})(?>#{MIME_PARAMETER})*\s*)\z/
 

actionpack/test/controller/integration_test.rb

@@ -1100,6 +1100,10 @@ def foos_json
     def foos_wibble
       render plain: "ok"
     end
+
+    def foos_json_api
+      render plain: "ok"
+    end
   end
 
   def test_standard_json_encoding_works
@@ -1170,6 +1174,26 @@ def test_registering_custom_encoder
     Mime::Type.unregister :wibble
   end
 
+  def test_registering_custom_encoder_including_parameters
+    accept_header = 'application/vnd.api+json; profile="https://jsonapi.org/profiles/ethanresnick/cursor-pagination/"; ext="https://jsonapi.org/ext/atomic"'
+    Mime::Type.register accept_header, :json_api
+
+    ActionDispatch::IntegrationTest.register_encoder(:json_api,
+      param_encoder: -> params { params })
+
+    post_to_foos as: :json_api do
+      assert_response :success
+      assert_equal "/foos_json_api", request.path
+      assert_equal "application/vnd.api+json", request.media_type
+      assert_equal accept_header, request.accepts.first.to_s
+      assert_equal :json_api, request.format.ref
+      assert_equal Hash.new, request.request_parameters # Unregistered MIME Type can't be parsed.
+      assert_equal "ok", response.parsed_body
+    end
+  ensure
+    Mime::Type.unregister :json_api
+  end
+
   def test_parsed_body_without_as_option
     with_routing do |routes|
       routes.draw do

actionpack/test/dispatch/mime_type_test.rb

@@ -80,6 +80,18 @@ class MimeTypeTest < ActiveSupport::TestCase
     assert_equal expect, Mime::Type.parse(accept)
   end
 
+  test "parse arbitrary media type parameters with comma" do
+    accept = 'multipart/form-data; boundary="simple, boundary"'
+    expect = [Mime[:multipart_form]]
+    assert_equal expect, Mime::Type.parse(accept)
+  end
+
+  test "parse arbitrary media type parameters with comma and additional media type" do
+    accept = 'multipart/form-data; boundary="simple, boundary", text/xml'
+    expect = [Mime[:multipart_form], Mime[:xml]]
+    assert_equal expect, Mime::Type.parse(accept)
+  end
+
   # Accept header send with user HTTP_USER_AGENT: Sunrise/0.42j (Windows XP)
   test "parse broken acceptlines" do
     accept = "text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/*,,*/*;q=0.5"
@@ -111,6 +123,15 @@ class MimeTypeTest < ActiveSupport::TestCase
     Mime::Type.unregister(:foobar)
   end
 
+  test "custom type with url parameter" do
+    accept = 'application/vnd.api+json; profile="https://jsonapi.org/profiles/example"'
+    type = Mime::Type.register(accept, :example_api)
+    assert_equal type, Mime[:example_api]
+    assert_equal [type], Mime::Type.parse(accept)
+  ensure
+    Mime::Type.unregister(:example_api)
+  end
+
   test "register callbacks" do
     registered_mimes = []
     Mime::Type.register_callback do |mime|