Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with multipart/form-data format #418

Open
Odebe opened this issue Feb 16, 2023 · 1 comment
Open

Issue with multipart/form-data format #418

Odebe opened this issue Feb 16, 2023 · 1 comment

Comments

@Odebe
Copy link

Odebe commented Feb 16, 2023

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
Copy link
Author

Odebe commented Jul 4, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant