From: Tom Hughes Date: Sat, 29 Feb 2020 15:15:02 +0000 (+0000) Subject: Rework set_default_request_format ast set_request_formats X-Git-Tag: live~2757 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/aaf9d15d5679f97447be0a09790aa63fef0b9022 Rework set_default_request_format ast set_request_formats Instead of trying to work out the exact format, just figure out the list of allowed formats and let rails do the rest. --- diff --git a/app/controllers/api/map_controller.rb b/app/controllers/api/map_controller.rb index 113554c72..1b5150537 100644 --- a/app/controllers/api/map_controller.rb +++ b/app/controllers/api/map_controller.rb @@ -5,7 +5,7 @@ module Api before_action :check_api_readable around_action :api_call_handle_error, :api_call_timeout - before_action :set_default_request_format + before_action :set_request_formats # This is probably the most common call of all. It is used for getting the # OSM data for a specified bounding box, usually for editing. First the diff --git a/app/controllers/api/nodes_controller.rb b/app/controllers/api/nodes_controller.rb index 336aebc91..9204d96c0 100644 --- a/app/controllers/api/nodes_controller.rb +++ b/app/controllers/api/nodes_controller.rb @@ -13,7 +13,7 @@ module Api before_action :check_api_readable, :except => [:create, :update, :delete] around_action :api_call_handle_error, :api_call_timeout - before_action :set_default_request_format, :except => [:create, :update, :delete] + before_action :set_request_formats, :except => [:create, :update, :delete] # Create a node from XML. def create diff --git a/app/controllers/api/old_controller.rb b/app/controllers/api/old_controller.rb index f672e7c49..f8e42476f 100644 --- a/app/controllers/api/old_controller.rb +++ b/app/controllers/api/old_controller.rb @@ -16,7 +16,7 @@ module Api before_action :lookup_old_element, :except => [:history] before_action :lookup_old_element_versions, :only => [:history] - before_action :set_default_request_format, :except => [:redact] + before_action :set_request_formats, :except => [:redact] def history # the .where() method used in the lookup_old_element_versions diff --git a/app/controllers/api/relations_controller.rb b/app/controllers/api/relations_controller.rb index 329c5e29c..28e4a026b 100644 --- a/app/controllers/api/relations_controller.rb +++ b/app/controllers/api/relations_controller.rb @@ -11,7 +11,7 @@ module Api before_action :check_api_readable, :except => [:create, :update, :delete] around_action :api_call_handle_error, :api_call_timeout - before_action :set_default_request_format, :except => [:create, :update, :delete] + before_action :set_request_formats, :except => [:create, :update, :delete] def create assert_method :put diff --git a/app/controllers/api/ways_controller.rb b/app/controllers/api/ways_controller.rb index a7c876710..3b58f208b 100644 --- a/app/controllers/api/ways_controller.rb +++ b/app/controllers/api/ways_controller.rb @@ -11,7 +11,7 @@ module Api before_action :check_api_readable, :except => [:create, :update, :delete] around_action :api_call_handle_error, :api_call_timeout - before_action :set_default_request_format, :except => [:create, :update, :delete] + before_action :set_request_formats, :except => [:create, :update, :delete] def create assert_method :put diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 64514a432..478810dfc 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -4,52 +4,45 @@ class ApiController < ApplicationController private ## - # Set default request format to xml unless a client requests a specific format, - # which can be done via (a) URL suffix and/or (b) HTTP Accept header, where - # the URL suffix always takes precedence over the Accept header. - def set_default_request_format + # Set allowed request formats if no explicit format has been + # requested via a URL suffix. Allowed formats are taken from + # any HTTP Accept header with XML as the default. + def set_request_formats unless params[:format] accept_header = request.headers["HTTP_ACCEPT"] - if accept_header.nil? - # e.g. unit tests don't set an Accept: header by default, force XML in this case - request.format = "xml" - return - end - - req_mimetypes = [] - - # Some clients (JOSM) send Accept headers which cannot be parsed by Rails, example: *; q=.2 - # To be fair, JOSM's Accept header doesn't adhere to RFC 7231, section 5.3.1, et al. either - # As a workaround for backwards compatibility, we're assuming XML format - begin - req_mimetypes = Mime::Type.parse(accept_header) - rescue Mime::Type::InvalidMimeType - request.format = "xml" - return - end - - # req_mimetypes contains all Accept header MIME types with descending priority - req_mimetypes.each do |mime| - if mime.symbol == :xml - request.format = "xml" - break - end - - if mime.symbol == :json - request.format = "json" - break - end - # Any format, not explicitly requesting XML or JSON -> assume XML as default - if mime == "*/*" - request.format = "xml" - break + if accept_header + # Some clients (such asJOSM) send Accept headers which cannot be + # parse by Rails, for example: + # + # Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2 + # + # where both "*" and ".2" as a quality do not adhere to the syntax + # described in RFC 7231, section 5.3.1, etc. + # + # As a workaround, and for back compatibility, default to XML format. + mimetypes = begin + Mime::Type.parse(accept_header) + rescue Mime::Type::InvalidMimeType + Array(Mime[:xml]) + end + + # Allow XML and JSON formats, and treat an all formats wildcard + # as XML for backwards compatibility - all other formats are discarded + # which will result in a 406 Not Acceptable response being sent + formats = mimetypes.map do |mime| + if mime.symbol == :xml then :xml + elsif mime.symbol == :json then :json + elsif mime == "*/*" then :xml + end end - - # In case the client requests some other format besides XML, JSON and */*, - # we deliberately don't set request.format. The framework will return an - # ActionController::UnknownFormat error to the client later on in this case. + else + # Default to XML if no accept header was sent - this includes + # the unit tests which don't set one by default + formats = Array(:xml) end + + request.formats = formats.compact end end