controller icon indicating copy to clipboard operation
controller copied to clipboard

Issue with multipart/form-data format

Open Odebe opened this issue 2 years ago • 1 comments

Hi, I run into an issue with request formats. Seems like action compares whole content type against config media types when enforcing content type. In result action can't accept multipart request due boundary definition in requests's content type.

Tested on 2.0.1 and 2.0.2. How to reproduce:

require 'hanami-controller'

class BaseAction < Hanami::Action
  config.formats.add :form_data, 'multipart/form-data'
  config.formats.add :hardcoded_boundary, 'multipart/form-data; boundary=AaB03x'

  def handle(req, res); end
end

# 415 Unsupported Media Type
class CaseOne < BaseAction
  config.format :form_data
end

# 200
class CaseTwo < BaseAction
  config.format :hardcoded_boundary
end

# request sample from https://github.com/rack/rack/blob/main/test/spec_multipart.rb
data = <<-REQUEST
--AaB03x
content-disposition: form-data; name="submit-name"

Larry
--AaB03x
content-disposition: form-data; name="submit-name-with-content"
content-type: text/plain

Berry
--AaB03x
content-disposition: form-data; name="files"; filename="file1.txt"
content-type: text/plain

contents
--AaB03x--
REQUEST

env = {
  "CONTENT_TYPE"   => 'multipart/form-data; boundary=AaB03x',
  "CONTENT_LENGTH" => data.bytesize.to_s,
  input:              StringIO.new(data)
}

puts "Hanami::Controller::VERSION #{Hanami::Controller::VERSION}"

[
  CaseOne,
  CaseTwo,
].each do |action|
  puts "#{action.name}"

  response = action.new.call(env)

  puts ">> request.content_type: #{response.request.content_type}"
  puts ">> request.media_type: #{response.request.media_type}"

  puts "<< response.status: #{response.status}"

  puts ''
end

# Hanami::Controller::VERSION 2.0.1
# CaseOne
# >> request.content_type: multipart/form-data; boundary=AaB03x
# >> request.media_type: multipart/form-data
# << response.status: 415
#
# CaseTwo
# >> request.content_type: multipart/form-data; boundary=AaB03x
# >> request.media_type: multipart/form-data
# << response.status: 200
#

My suggestion is to use request.media_type in[ this part.] (https://github.com/hanami/controller/blob/main/lib/hanami/action/mime.rb#L225) I've monkey patched this part and it works just fine in my case. But smells bad.

But this can lead to the following issues:

  1. More objects allocation (due to https://github.com/rack/rack/blob/2-2-stable/lib/rack/media_type.rb#L18)
  2. It can break cases when charset defined as part of content-type in action config.

Or maybe I just don't get how to configure formats.

Odebe avatar Feb 16 '23 16:02 Odebe

Any thoughts on this? I'm wondering if it's a bug at all or not.

Odebe avatar Jul 04 '23 20:07 Odebe